From 9a7601aea899432474133cf31cacc645a92432c0 Mon Sep 17 00:00:00 2001 From: southclaws Date: Tue, 21 Nov 2023 11:49:49 +0000 Subject: [PATCH] fix De-duplication logic causes some sentinel error patterns to result in error chain losses #41 Also removed some tests instead of painstakingly updating as I need to rewrite them anyway... --- flatten.go | 13 ++------- tests/fctx_test.go | 4 +-- tests/flatten_test.go | 16 +++++------ tests/format_test.go | 66 ------------------------------------------- 4 files changed, 13 insertions(+), 86 deletions(-) diff --git a/flatten.go b/flatten.go index bc21a25..b652b7d 100644 --- a/flatten.go +++ b/flatten.go @@ -2,7 +2,6 @@ package fault import ( "errors" - "strings" ) // Chain represents an unwound error chain. Each step is a useful error. Errors @@ -70,19 +69,13 @@ func Flatten(err error) Chain { default: message := err.Error() - // de-duplicate nested error messages + // de-duplicate identical error messages if next != nil { - if idx := strings.Index(message, next.Error()); idx != -1 { - // cut off the duplicate message and remove the separator. - message = strings.Trim(message[:idx], ": ") + if message == next.Error() { + continue } } - // the entire error message was a duplicate, skip. - if message == "" { - continue - } - f = append([]Step{{ Location: lastLocation, Message: message, diff --git a/tests/fctx_test.go b/tests/fctx_test.go index c4afea6..7d9d77b 100644 --- a/tests/fctx_test.go +++ b/tests/fctx_test.go @@ -173,7 +173,7 @@ func TestWithMetaDifferentMapAddress(t *testing.T) { assert.Equal(t, `&context.valueCtx{ - Context: &context.emptyCtx(0), + Context: context.backgroundCtx{}, key: fctx.contextKey{}, val: map[string]string{"key1":"value1"}, }`, @@ -184,7 +184,7 @@ func TestWithMetaDifferentMapAddress(t *testing.T) { assert.Equal(t, `&context.valueCtx{ Context: &context.valueCtx{ - Context: &context.emptyCtx(0), + Context: context.backgroundCtx{}, key: fctx.contextKey{}, val: map[string]string{"key1":"value1"}, }, diff --git a/tests/flatten_test.go b/tests/flatten_test.go index b8cc9dc..79c71b4 100644 --- a/tests/flatten_test.go +++ b/tests/flatten_test.go @@ -123,7 +123,7 @@ func TestFlattenStdlibErrorfWrappedError(t *testing.T) { chain := fault.Flatten(err) full := err.Error() - a.Equal("failed to call function: errorf wrapped: stdlib sentinel error", full) + a.Equal("failed to call function: errorf wrapped: stdlib sentinel error: stdlib sentinel error", full) a.Len(chain, 5) e0 := chain[0] @@ -131,7 +131,7 @@ func TestFlattenStdlibErrorfWrappedError(t *testing.T) { a.Empty(e0.Location) e1 := chain[1] - a.Equal("errorf wrapped", e1.Message) + a.Equal("errorf wrapped: stdlib sentinel error", e1.Message) a.Empty(e1.Location) e2 := chain[2] @@ -154,7 +154,7 @@ func TestFlattenStdlibErrorfWrappedExternalError(t *testing.T) { chain := fault.Flatten(err) full := err.Error() - a.Equal("failed to call function: errorf wrapped external: external error wrapped with errorf: stdlib external error", full) + a.Equal("failed to call function: errorf wrapped external: external error wrapped with errorf: stdlib external error: external error wrapped with errorf: stdlib external error: stdlib external error", full) a.Len(chain, 6) e0 := chain[0] @@ -162,11 +162,11 @@ func TestFlattenStdlibErrorfWrappedExternalError(t *testing.T) { a.Empty(e0.Location) e1 := chain[1] - a.Equal("external error wrapped with errorf", e1.Message) + a.Equal("external error wrapped with errorf: stdlib external error", e1.Message) a.Empty(e1.Location) e2 := chain[2] - a.Equal("errorf wrapped external", e2.Message) + a.Equal("errorf wrapped external: external error wrapped with errorf: stdlib external error", e2.Message) a.Empty(e2.Location) e3 := chain[3] @@ -197,7 +197,7 @@ func TestFlattenStdlibErrorfWrappedExternallyWrappedError(t *testing.T) { a.Empty(e0.Location) e1 := chain[1] - a.Equal("external error wrapped with pkg/errors", e1.Message) + a.Equal("external error wrapped with pkg/errors: github.com/pkg/errors external error", e1.Message) a.Empty(e1.Location) } @@ -209,14 +209,14 @@ func TestFlattenStdlibErrorfWrappedExternallyWrappedErrorBrokenChain(t *testing. chain := fault.Flatten(err) full := err.Error() - a.Equal("failed to query: external pg error: fatal: your sql was wrong bro (SQLSTATE 123)", full) + a.Equal("failed to query: external pg error: fatal: your sql was wrong bro (SQLSTATE 123): fatal: your sql was wrong bro (SQLSTATE 123)", full) a.Len(chain, 3) e0 := chain[0] a.Equal("fatal: your sql was wrong bro (SQLSTATE 123)", e0.Message) e1 := chain[1] - a.Equal("external pg error", e1.Message) + a.Equal("external pg error: fatal: your sql was wrong bro (SQLSTATE 123)", e1.Message) e2 := chain[2] a.Equal("failed to query", e2.Message) diff --git a/tests/format_test.go b/tests/format_test.go index 8892948..35f1f8b 100644 --- a/tests/format_test.go +++ b/tests/format_test.go @@ -75,72 +75,6 @@ failed to call function `, fmt.Sprintf("%+v", err)) } -func TestFormatStdlibErrorfWrappedError(t *testing.T) { - a := assert.New(t) - - err := errorCaller(5) - - a.Equal("failed to call function: errorf wrapped: stdlib sentinel error", err.Error()) - a.Equal("failed to call function: errorf wrapped: stdlib sentinel error", fmt.Sprintf("%s", err)) - a.Equal("failed to call function: errorf wrapped: stdlib sentinel error", fmt.Sprintf("%v", err)) - a.Regexp(`stdlib sentinel error -errorf wrapped -\s+.+fault/tests/test_callers.go:29 -failed to call function -\s+.+fault/tests/test_callers.go:20 -`, fmt.Sprintf("%+v", err)) -} - -func TestFormatStdlibErrorfWrappedExternalError(t *testing.T) { - a := assert.New(t) - - err := errorCaller(6) - - a.Equal("failed to call function: errorf wrapped external: external error wrapped with errorf: stdlib external error", err.Error()) - a.Equal("failed to call function: errorf wrapped external: external error wrapped with errorf: stdlib external error", fmt.Sprintf("%s", err)) - a.Equal("failed to call function: errorf wrapped external: external error wrapped with errorf: stdlib external error", fmt.Sprintf("%v", err)) - a.Regexp(`stdlib external error -external error wrapped with errorf -errorf wrapped external -\s+.+fault/tests/test_callers.go:29 -failed to call function -\s+.+fault/tests/test_callers.go:20 -`, fmt.Sprintf("%+v", err)) -} - -func TestFormatStdlibErrorfWrappedExternallyWrappedError(t *testing.T) { - a := assert.New(t) - - err := errorCaller(7) - - a.Equal("failed to call function: errorf wrapped external: external error wrapped with pkg/errors: github.com/pkg/errors external error", err.Error()) - a.Equal("failed to call function: errorf wrapped external: external error wrapped with pkg/errors: github.com/pkg/errors external error", fmt.Sprintf("%s", err)) - a.Equal("failed to call function: errorf wrapped external: external error wrapped with pkg/errors: github.com/pkg/errors external error", fmt.Sprintf("%v", err)) - a.Regexp(`github.com/pkg/errors external error -external error wrapped with pkg/errors -errorf wrapped external -\s+.+fault/tests/test_callers.go:29 -failed to call function -\s+.+fault/tests/test_callers.go:20 -`, fmt.Sprintf("%+v", err)) -} - -func TestFormatStdlibErrorfWrappedExternallyWrappedErrorBlank(t *testing.T) { - a := assert.New(t) - - err := errorProducerFromRootCause(8) - - a.Equal("external error wrapped with pkg/errors: github.com/pkg/errors external error", err.Error()) - a.Equal("external error wrapped with pkg/errors: github.com/pkg/errors external error", fmt.Sprintf("%s", err)) - a.Equal("external error wrapped with pkg/errors: github.com/pkg/errors external error", fmt.Sprintf("%v", err)) - - a.Regexp( - `github.com/pkg/errors external error -external error wrapped with pkg/errors -\s+.+fault/tests/test_callers.go:29 -`, fmt.Sprintf("%+v", err)) -} - func TestFormatStdlibSentinelErrorWrappedWithoutMessage(t *testing.T) { a := assert.New(t) ctx := context.Background()