From f32d463816bc8a7934b28fb2559e6df0a5f1d78d Mon Sep 17 00:00:00 2001 From: Suhail Patel Date: Wed, 28 Sep 2022 11:00:59 +0100 Subject: [PATCH 1/3] Add cruft we don't want to the gitignore list --- .gitignore | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.gitignore b/.gitignore index e43b0f9..1485e10 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1,6 @@ .DS_Store +.AppleDouble +.LSOverride +.idea +.vscode +coverage.out From 160095be57c603739e2c86249245bb808790ecc4 Mon Sep 17 00:00:00 2001 From: Suhail Patel Date: Wed, 28 Sep 2022 11:01:09 +0100 Subject: [PATCH 2/3] Add a failing test where we lose context when calling Wrap This commit showcases a failing test where calling Wrap on a terror causes us to lose the underlying context In this case, we would expect the error to be - not_found.foo: added context: failed to find foo But instead we get - not_found.foo: added context So when chaining these Wrap and Augment calls, we lose the context of the original error --- errors_test.go | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/errors_test.go b/errors_test.go index 9d5d534..7e26c87 100644 --- a/errors_test.go +++ b/errors_test.go @@ -276,6 +276,15 @@ func TestAugmentTerror(t *testing.T) { assert.Equal(t, base, terr.cause) } +func TestAugmentTerrorWithWrap(t *testing.T) { + base := NotFound("foo", "failed to find foo", map[string]string{"base": "meta"}) + augmentedErr := Augment(base, "added context", map[string]string{"new": "meta"}) + assert.Equal(t, "not_found.foo: added context: failed to find foo", augmentedErr.Error()) + + wrappedErr := Wrap(augmentedErr, map[string]string{"wrap": "meta"}) + assert.Equal(t, "not_found.foo: added context: failed to find foo", wrappedErr.Error()) +} + func TestAugmentNil(t *testing.T) { assert.Nil(t, Augment(nil, "added context", map[string]string{ "new": "meta", From 33980aba1af54cd83012a9ba7a36e580563d35fb Mon Sep 17 00:00:00 2001 From: Suhail Patel Date: Wed, 28 Sep 2022 11:38:10 +0100 Subject: [PATCH 3/3] Propagate err cause across wrapped errors This allows us to maintain downstream error reasons when calling Wrap --- errors.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/errors.go b/errors.go index 449658b..df4236c 100644 --- a/errors.go +++ b/errors.go @@ -4,14 +4,17 @@ // user defined error parameters. // // Terrors can be used to wrap any object that satisfies the error interface: +// // terr := terrors.Wrap(err, map[string]string{"context": "my_context"}) // // Terrors can be instantiated directly: -// err := terrors.New("not_found", "object not found", map[string]string{ +// +// err := terrors.New("not_found", "object not found", map[string]string{ // "context": "my_context" // }) // // Terrors offers built-in functions for instantiating Errors with common codes: +// // err := terrors.NotFound("config_file", "config file not found", map[string]string{ // "context": my_context // }) @@ -208,6 +211,7 @@ func addParams(err *Error, params map[string]string) *Error { Params: copiedParams, StackFrames: err.StackFrames, IsRetryable: err.IsRetryable, + cause: err.cause, } } @@ -237,7 +241,9 @@ func (p *Error) PrefixMatches(prefixParts ...string) bool { // `PrefixMatches`, so if you were previously matching against a part of the string returned from error.Error() that // is _not_ the prefix, then this will be a breaking change. In this case you should update the string to match the // prefix. If this is not possible, you can match against the entire error string explicitly, for example: -// strings.Contains(err.Error(), "context deadline exceeded") +// +// strings.Contains(err.Error(), "context deadline exceeded") +// // But we consider this bad practice and is part of the motivation for deprecating Matches in the first place. func Matches(err error, match string) bool { if terr, ok := Wrap(err, nil).(*Error); ok {