From 1be749f6269c170e93d68f509ecb95a726c9a349 Mon Sep 17 00:00:00 2001 From: Mathieu Durand <1435391+matdurand@users.noreply.github.com> Date: Wed, 31 May 2023 20:10:28 -0400 Subject: [PATCH] fix: issue 28 wrong location of some errors --- .gitignore | 2 +- fault.go | 11 ++++++ flatten.go | 10 ++++- tests/flatten_test.go | 92 ++++++++++++++++++++++++++++++++----------- tests/format_test.go | 4 ++ 5 files changed, 94 insertions(+), 25 deletions(-) diff --git a/.gitignore b/.gitignore index e410a29..9f11b75 100644 --- a/.gitignore +++ b/.gitignore @@ -1 +1 @@ - .idea \ No newline at end of file +.idea/ diff --git a/fault.go b/fault.go index f1ea76c..3102dc0 100644 --- a/fault.go +++ b/fault.go @@ -21,6 +21,17 @@ func Wrap(err error, w ...Wrapper) error { return nil } + // the passed err might already have a location if it's a fault.New error, or it might not if it's another type of + // error like one from the standard library. Wrapping it in a container with an empty location ensures that the + // location will be reset when we flatten the error chain. If the error is a fault.New error, it will itself be + // wrapped in a container which will have a location + if _, ok := err.(*container); !ok { + err = &container{ + cause: err, + location: "", + } + } + for _, fn := range w { err = fn(err) } diff --git a/flatten.go b/flatten.go index 2b63908..bc21a25 100644 --- a/flatten.go +++ b/flatten.go @@ -49,6 +49,14 @@ func Flatten(err error) Chain { // exist to contain other errors that actually contain information, // store the container's recorded location for usage with the next item. case *container: + if _, ok := next.(*container); ok && unwrapped.location != "" { + // Having 2 containers back to back can happen if we're using .Wrap without using any wrappers. In that + // case, we add a Step to avoid losing the location whe the wrapping occurred + f = append([]Step{{ + Location: unwrapped.location, + Message: "", + }}, f...) + } lastLocation = unwrapped.location case *fundamental: @@ -79,8 +87,6 @@ func Flatten(err error) Chain { Location: lastLocation, Message: message, }}, f...) - - lastLocation = "" } } diff --git a/tests/flatten_test.go b/tests/flatten_test.go index 358e556..b8cc9dc 100644 --- a/tests/flatten_test.go +++ b/tests/flatten_test.go @@ -16,15 +16,23 @@ func TestFlattenStdlibSentinelError(t *testing.T) { full := err.Error() a.Equal("failed to call function: stdlib sentinel error", full) - a.Len(chain, 2) + a.Len(chain, 4) e0 := chain[0] a.Equal("stdlib sentinel error", e0.Message) - a.Contains(e0.Location, "test_callers.go:29") + a.Empty(e0.Location) e1 := chain[1] - a.Equal("failed to call function", e1.Message) - a.Contains(e1.Location, "test_callers.go:20") + a.Empty(e1.Message) + a.Contains(e1.Location, "test_callers.go:29") + + e2 := chain[2] + a.Equal("failed to call function", e2.Message) + a.Contains(e2.Location, "test_callers.go:20") + + e3 := chain[3] + a.Empty(e3.Message) + a.Contains(e3.Location, "test_callers.go:11") } func TestFlattenFaultSentinelError(t *testing.T) { @@ -35,15 +43,23 @@ func TestFlattenFaultSentinelError(t *testing.T) { full := err.Error() a.Equal("failed to call function: fault sentinel error", full) - a.Len(chain, 2) + a.Len(chain, 4) e0 := chain[0] a.Equal("fault sentinel error", e0.Message) a.Contains(e0.Location, "root.go:15") e1 := chain[1] - a.Equal("failed to call function", e1.Message) - a.Contains(e1.Location, "test_callers.go:20") + a.Empty(e1.Message) + a.Contains(e1.Location, "test_callers.go:29") + + e2 := chain[2] + a.Equal("failed to call function", e2.Message) + a.Contains(e2.Location, "test_callers.go:20") + + e3 := chain[3] + a.Empty(e3.Message) + a.Contains(e3.Location, "test_callers.go:11") } func TestFlattenStdlibInlineError(t *testing.T) { @@ -54,15 +70,23 @@ func TestFlattenStdlibInlineError(t *testing.T) { full := err.Error() a.Equal("failed to call function: stdlib root cause error", full) - a.Len(chain, 2) + a.Len(chain, 4) e0 := chain[0] a.Equal("stdlib root cause error", e0.Message) - a.Contains(e0.Location, "test_callers.go:29") + a.Empty(e0.Location) e1 := chain[1] - a.Equal("failed to call function", e1.Message) - a.Contains(e1.Location, "test_callers.go:20") + a.Empty(e1.Message) + a.Contains(e1.Location, "test_callers.go:29") + + e2 := chain[2] + a.Equal("failed to call function", e2.Message) + a.Contains(e2.Location, "test_callers.go:20") + + e3 := chain[3] + a.Empty(e3.Message) + a.Contains(e3.Location, "test_callers.go:11") } func TestFlattenFaultInlineError(t *testing.T) { @@ -73,15 +97,23 @@ func TestFlattenFaultInlineError(t *testing.T) { full := err.Error() a.Equal("failed to call function: fault root cause error", full) - a.Len(chain, 2) + a.Len(chain, 4) e0 := chain[0] a.Equal("fault root cause error", e0.Message) a.Contains(e0.Location, "root.go:28") e1 := chain[1] - a.Equal("failed to call function", e1.Message) - a.Contains(e1.Location, "test_callers.go:20") + a.Empty(e1.Message) + a.Contains(e1.Location, "test_callers.go:29") + + e2 := chain[2] + a.Equal("failed to call function", e2.Message) + a.Contains(e2.Location, "test_callers.go:20") + + e3 := chain[3] + a.Empty(e3.Message) + a.Contains(e3.Location, "test_callers.go:11") } func TestFlattenStdlibErrorfWrappedError(t *testing.T) { @@ -92,7 +124,7 @@ func TestFlattenStdlibErrorfWrappedError(t *testing.T) { full := err.Error() a.Equal("failed to call function: errorf wrapped: stdlib sentinel error", full) - a.Len(chain, 3) + a.Len(chain, 5) e0 := chain[0] a.Equal("stdlib sentinel error", e0.Message) @@ -100,11 +132,19 @@ func TestFlattenStdlibErrorfWrappedError(t *testing.T) { e1 := chain[1] a.Equal("errorf wrapped", e1.Message) - a.Contains(e1.Location, "test_callers.go:29") + a.Empty(e1.Location) e2 := chain[2] - a.Equal("failed to call function", e2.Message) - a.Contains(e2.Location, "test_callers.go:20") + a.Empty(e2.Message) + a.Contains(e2.Location, "test_callers.go:29") + + e3 := chain[3] + a.Equal("failed to call function", e3.Message) + a.Contains(e3.Location, "test_callers.go:20") + + e4 := chain[4] + a.Empty(e4.Message) + a.Contains(e4.Location, "test_callers.go:11") } func TestFlattenStdlibErrorfWrappedExternalError(t *testing.T) { @@ -115,7 +155,7 @@ func TestFlattenStdlibErrorfWrappedExternalError(t *testing.T) { full := err.Error() a.Equal("failed to call function: errorf wrapped external: external error wrapped with errorf: stdlib external error", full) - a.Len(chain, 4) + a.Len(chain, 6) e0 := chain[0] a.Equal("stdlib external error", e0.Message) @@ -127,11 +167,19 @@ func TestFlattenStdlibErrorfWrappedExternalError(t *testing.T) { e2 := chain[2] a.Equal("errorf wrapped external", e2.Message) - a.Contains(e2.Location, "test_callers.go:29") + a.Empty(e2.Location) e3 := chain[3] - a.Equal("failed to call function", e3.Message) - a.Contains(e3.Location, "test_callers.go:20") + a.Empty(e3.Message) + a.Contains(e3.Location, "test_callers.go:29") + + e4 := chain[4] + a.Equal("failed to call function", e4.Message) + a.Contains(e4.Location, "test_callers.go:20") + + e5 := chain[5] + a.Empty(e5.Message) + a.Contains(e5.Location, "test_callers.go:11") } func TestFlattenStdlibErrorfWrappedExternallyWrappedError(t *testing.T) { diff --git a/tests/format_test.go b/tests/format_test.go index 0fb2575..1cec242 100644 --- a/tests/format_test.go +++ b/tests/format_test.go @@ -36,8 +36,10 @@ func TestFormatFaultSentinelError(t *testing.T) { a.Equal("failed to call function: fault sentinel error", fmt.Sprintf("%v", err)) a.Regexp(`fault sentinel error \s+.+fault/tests/root.go:15 +\s+.+fault/tests/test_callers.go:29 failed to call function \s+.+fault/tests/test_callers.go:20 +\s+.+fault/tests/test_callers.go:11 `, fmt.Sprintf("%+v", err)) } @@ -66,8 +68,10 @@ func TestFormatFaultInlineError(t *testing.T) { a.Equal("failed to call function: fault root cause error", fmt.Sprintf("%v", err)) a.Regexp(`fault root cause error \s+.+fault/tests/root.go:28 +\s+.+fault/tests/test_callers.go:29 failed to call function \s+.+fault/tests/test_callers.go:20 +\s+.+fault/tests/test_callers.go:11 `, fmt.Sprintf("%+v", err)) }