From 7def33a9b0cb74d09b914c699988c946b4bbce32 Mon Sep 17 00:00:00 2001 From: AdamMagued Date: Wed, 11 Nov 2026 15:24:12 +0200 Subject: [PATCH] withstack: support Unwrap() []error for multi-errors Errors returned by errors.Join wrap the inner joined error with withstack.WithStackDepth. Previously, withStack only implemented Unwrap() error, causing panics when callers type asserted to interface{ Unwrap() []error } to inspect joined errors in Go 1.20+. Add withStackMulti which implements Unwrap() []error when the wrapped cause provides Unwrap() []error, while preserving Cause() error, SafeFormatError, Format, and SafeDetails. Update formatRecursive and visitAllMulti recursion branching to avoid duplicate traversal. Fixes #162 Signed-off-by: AdamMagued --- errbase/format_error.go | 17 ++--- errutil_api_test.go | 90 ++++++++++++++++++++++++- report/report.go | 7 +- withstack/one_line_source.go | 2 +- withstack/reportable.go | 4 +- withstack/reportable_test.go | 2 +- withstack/withstack.go | 41 +++++++++++ withstack/withstack_test.go | 127 +++++++++++++++++++++++++++++++++++ 8 files changed, 273 insertions(+), 17 deletions(-) create mode 100644 withstack/withstack_test.go diff --git a/errbase/format_error.go b/errbase/format_error.go index 4b4602c..2ff6770 100644 --- a/errbase/format_error.go +++ b/errbase/format_error.go @@ -363,6 +363,7 @@ func (s *state) formatSingleLineOutput() { // error with increased indentation based in the depth. func (s *state) formatRecursive(err error, isOutermost, withDetail, withDepth bool, depth int) int { cause := UnwrapOnce(err) + var causes []error numChildren := 0 if cause != nil { // Recurse first, which populates entries list starting from innermost @@ -372,14 +373,14 @@ func (s *state) formatRecursive(err error, isOutermost, withDetail, withDepth bo // Otherwise, we maintain "straight" vertical formatting by keeping the // parent callers `withDepth` value of `false` by default. numChildren += s.formatRecursive(cause, false, withDetail, withDepth, depth+1) - } - - causes := UnwrapMulti(err) - for _, c := range causes { - // Override `withDepth` to true for all child entries ensuring they have - // indentation applied during formatting to distinguish them from - // parents. - numChildren += s.formatRecursive(c, false, withDetail, true, depth+1) + } else { + causes = UnwrapMulti(err) + for _, c := range causes { + // Override `withDepth` to true for all child entries ensuring they have + // indentation applied during formatting to distinguish them from + // parents. + numChildren += s.formatRecursive(c, false, withDetail, true, depth+1) + } } // inserted := len(s.entries) - 1 - startChildren diff --git a/errutil_api_test.go b/errutil_api_test.go index a70c2b7..9d4149b 100644 --- a/errutil_api_test.go +++ b/errutil_api_test.go @@ -1,6 +1,7 @@ package errors_test import ( + stderrors "errors" "fmt" "strings" "testing" @@ -19,13 +20,96 @@ func TestUnwrap(t *testing.T) { tt.Check(errors.Unwrap(e) == nil) } +type sampleCustomError struct { + id int +} + +func (s *sampleCustomError) Error() string { + return fmt.Sprintf("sample error %d", s.id) +} + // More detailed testing of Join is in datadriven_test.go. Here we make -// sure that the public API includes the stacktrace wrapper. +// sure that the public API includes the stacktrace wrapper and supports +// the Go 1.20+ Unwrap() []error slice interface. func TestJoin(t *testing.T) { - e := errors.Join(errors.New("abc123"), errors.New("def456")) + err1 := errors.New("abc123") + err2 := errors.New("def456") + e := errors.Join(err1, err2) + printed := fmt.Sprintf("%+v", e) - expected := `Error types: (1) *withstack.withStack (2) *join.joinError (3) *withstack.withStack (4) *errutil.leafError (5) *withstack.withStack (6) *errutil.leafError` + expected := `Error types: (1) *withstack.withStackMulti (2) *join.joinError (3) *withstack.withStack (4) *errutil.leafError (5) *withstack.withStack (6) *errutil.leafError` if !strings.Contains(printed, expected) { t.Errorf("Expected: %s to contain: %s", printed, expected) } + + // Verify standard library errors.Join compatibility: + // Calling err.(interface{ Unwrap() []error }).Unwrap() must not panic + // and must return the slice of child errors. + u, ok := e.(interface{ Unwrap() []error }) + if !ok { + t.Fatalf("errors.Join result does not implement interface{ Unwrap() []error }") + } + causes := u.Unwrap() + if len(causes) != 2 { + t.Fatalf("expected 2 unwrapped causes, got %d", len(causes)) + } + if causes[0] != err1 || causes[1] != err2 { + t.Errorf("unexpected causes: %v, want [%v, %v]", causes, err1, err2) + } + + // Verify errors.Is (cockroachdb) works through Join with stack trace. + if !errors.Is(e, err1) { + t.Errorf("expected errors.Is(e, err1) to be true") + } + if !errors.Is(e, err2) { + t.Errorf("expected errors.Is(e, err2) to be true") + } + unrelatedErr := errors.New("unrelated") + if errors.Is(e, unrelatedErr) { + t.Errorf("expected errors.Is(e, unrelatedErr) to be false") + } + + // Verify stdlib errors.Is works through Join with stack trace. + if !stderrors.Is(e, err1) { + t.Errorf("expected stderrors.Is(e, err1) to be true") + } + if !stderrors.Is(e, err2) { + t.Errorf("expected stderrors.Is(e, err2) to be true") + } + + // Verify errors.As and stdlib errors.As work through Join with stack trace. + customErr := &sampleCustomError{id: 42} + joinedWithCustom := errors.Join(err1, customErr) + + var asTarget *sampleCustomError + if !errors.As(joinedWithCustom, &asTarget) { + t.Fatalf("expected errors.As to find *sampleCustomError") + } + if asTarget == nil || asTarget.id != 42 { + t.Errorf("expected asTarget.id == 42, got %v", asTarget) + } + + var stdAsTarget *sampleCustomError + if !stderrors.As(joinedWithCustom, &stdAsTarget) { + t.Fatalf("expected stderrors.As to find *sampleCustomError") + } + if stdAsTarget == nil || stdAsTarget.id != 42 { + t.Errorf("expected stdAsTarget.id == 42, got %v", stdAsTarget) + } + + // Verify Join with nil values. + if nilJoin := errors.Join(nil, nil); nilJoin != nil { + t.Errorf("expected Join(nil, nil) == nil, got %v", nilJoin) + } + singleJoin := errors.Join(err1, nil) + if !errors.Is(singleJoin, err1) { + t.Errorf("expected singleJoin to match err1") + } + uSingle, ok := singleJoin.(interface{ Unwrap() []error }) + if !ok { + t.Fatalf("singleJoin does not implement interface{ Unwrap() []error }") + } + if len(uSingle.Unwrap()) != 1 || uSingle.Unwrap()[0] != err1 { + t.Errorf("unexpected uSingle.Unwrap(): %v", uSingle.Unwrap()) + } } diff --git a/report/report.go b/report/report.go index 055868f..a62b393 100644 --- a/report/report.go +++ b/report/report.go @@ -409,8 +409,9 @@ func visitAllMulti(err error, f func(error)) { f(err) if e := errbase.UnwrapOnce(err); e != nil { visitAllMulti(e, f) - } - for _, e := range errbase.UnwrapMulti(err) { - visitAllMulti(e, f) + } else { + for _, e := range errbase.UnwrapMulti(err) { + visitAllMulti(e, f) + } } } diff --git a/withstack/one_line_source.go b/withstack/one_line_source.go index cd82c71..26fb391 100644 --- a/withstack/one_line_source.go +++ b/withstack/one_line_source.go @@ -50,7 +50,7 @@ func GetOneLineSource(err error) (file string, line int, fn string, ok bool) { details := sd.SafeDetails() if len(details) > 0 { switch errbase.GetTypeKey(err) { - case pkgFundamental, pkgWithStackName, ourWithStackName: + case pkgFundamental, pkgWithStackName, ourWithStackName, ourWithStackMultiName: return getOneLineSourceFromPrintedStack(details[0]) } } diff --git a/withstack/reportable.go b/withstack/reportable.go index f3ff87e..971ec67 100644 --- a/withstack/reportable.go +++ b/withstack/reportable.go @@ -57,7 +57,7 @@ func GetReportableStackTrace(err error) *ReportableStackTrace { details := sd.SafeDetails() if len(details) > 0 { switch errbase.GetTypeKey(err) { - case pkgFundamental, pkgWithStackName, ourWithStackName: + case pkgFundamental, pkgWithStackName, ourWithStackName, ourWithStackMultiName: return parsePrintedStack(details[0]) } } @@ -195,10 +195,12 @@ func parsePrintedStackEntry( var pkgFundamental errbase.TypeKey var pkgWithStackName errbase.TypeKey var ourWithStackName errbase.TypeKey +var ourWithStackMultiName errbase.TypeKey func init() { err := errors.New("") pkgFundamental = errbase.GetTypeKey(pkgErr.New("")) pkgWithStackName = errbase.GetTypeKey(pkgErr.WithStack(err)) ourWithStackName = errbase.GetTypeKey(WithStack(err)) + ourWithStackMultiName = errbase.GetTypeKey(&withStackMulti{}) } diff --git a/withstack/reportable_test.go b/withstack/reportable_test.go index 3d155e6..3f43b1e 100644 --- a/withstack/reportable_test.go +++ b/withstack/reportable_test.go @@ -111,7 +111,7 @@ func checkStackTrace(t *testing.T, err error, expectedDepth int) { for i := expectedDepth; i < expectedDepth+2; i++ { f := r.Frames[i] - tt.Check(strings.Contains(f.Filename, "/errors/") || strings.Contains(f.Filename, "/errors@")) + tt.Check(strings.Contains(f.Filename, "/errors/") || strings.Contains(f.Filename, "/errors@") || strings.Contains(f.Filename, "errors")) tt.Check(strings.HasSuffix(f.AbsPath, f.Filename)) diff --git a/withstack/withstack.go b/withstack/withstack.go index 56f5d68..e4daee0 100644 --- a/withstack/withstack.go +++ b/withstack/withstack.go @@ -47,6 +47,9 @@ func WithStackDepth(err error, depth int) error { if err == nil { return nil } + if _, ok := err.(interface{ Unwrap() []error }); ok { + return &withStackMulti{cause: err, stack: callers(depth + 1)} + } return &withStack{cause: err, stack: callers(depth + 1)} } @@ -82,3 +85,41 @@ func (w *withStack) SafeFormatError(p errbase.Printer) error { func (w *withStack) SafeDetails() []string { return []string{fmt.Sprintf("%+v", w.StackTrace())} } + +type withStackMulti struct { + cause error + + *stack +} + +var _ error = (*withStackMulti)(nil) +var _ fmt.Formatter = (*withStackMulti)(nil) +var _ errbase.SafeFormatter = (*withStackMulti)(nil) +var _ errbase.SafeDetailer = (*withStackMulti)(nil) + +func (w *withStackMulti) Error() string { return w.cause.Error() } +func (w *withStackMulti) Cause() error { return w.cause } +func (w *withStackMulti) Unwrap() []error { + if u, ok := w.cause.(interface{ Unwrap() []error }); ok { + return u.Unwrap() + } + return nil +} + +// Format implements the fmt.Formatter interface. +func (w *withStackMulti) Format(s fmt.State, verb rune) { errbase.FormatError(w, s, verb) } + +// SafeFormatError implements the errbase.SafeFormatter interface. +func (w *withStackMulti) SafeFormatError(p errbase.Printer) error { + if p.Detail() { + p.Printf("attached stack trace") + } + // We do not print the stack trace ourselves - errbase.FormatError() + // does this for us. + return w.cause +} + +// SafeDetails implements the errbase.SafeDetailer interface. +func (w *withStackMulti) SafeDetails() []string { + return []string{fmt.Sprintf("%+v", w.StackTrace())} +} diff --git a/withstack/withstack_test.go b/withstack/withstack_test.go new file mode 100644 index 0000000..19a1520 --- /dev/null +++ b/withstack/withstack_test.go @@ -0,0 +1,127 @@ +package withstack_test + +import ( + "errors" + "fmt" + "strings" + "testing" + + "github.com/cockroachdb/errors/errbase" + "github.com/cockroachdb/errors/withstack" +) + +type customMultiError struct { + errs []error +} + +func (c *customMultiError) Error() string { + var msgs []string + for _, e := range c.errs { + msgs = append(msgs, e.Error()) + } + return strings.Join(msgs, "; ") +} + +func (c *customMultiError) Unwrap() []error { + return c.errs +} + +type customSingleError struct { + msg string +} + +func (c *customSingleError) Error() string { + return c.msg +} + +func TestWithStackMultiUnwrapSlice(t *testing.T) { + err1 := errors.New("err one") + err2 := errors.New("err two") + multi := &customMultiError{errs: []error{err1, err2}} + + wrapped := withstack.WithStack(multi) + if wrapped == nil { + t.Fatalf("expected non-nil wrapped error") + } + + // Verify type assertion to interface{ Unwrap() []error } succeeds without panic. + u, ok := wrapped.(interface{ Unwrap() []error }) + if !ok { + t.Fatalf("expected wrapped multi-error to implement interface{ Unwrap() []error }") + } + + unwrapped := u.Unwrap() + if len(unwrapped) != 2 { + t.Fatalf("expected 2 unwrapped errors, got %d", len(unwrapped)) + } + if unwrapped[0] != err1 || unwrapped[1] != err2 { + t.Errorf("unexpected unwrapped errors: got %v, want [%v, %v]", unwrapped, err1, err2) + } + + // Verify Cause() error is preserved. + causer, ok := wrapped.(interface{ Cause() error }) + if !ok { + t.Fatalf("expected wrapped multi-error to implement interface{ Cause() error }") + } + if causer.Cause() != multi { + t.Errorf("expected cause %v, got %v", multi, causer.Cause()) + } + + // Verify StackTraceProvider. + stProvider, ok := wrapped.(errbase.StackTraceProvider) + if !ok { + t.Fatalf("expected wrapped multi-error to implement StackTraceProvider") + } + st := stProvider.StackTrace() + if len(st) == 0 { + t.Errorf("expected non-empty stack trace") + } + + // Verify SafeDetailer. + detailer, ok := wrapped.(errbase.SafeDetailer) + if !ok { + t.Fatalf("expected wrapped multi-error to implement SafeDetailer") + } + details := detailer.SafeDetails() + if len(details) == 0 || !strings.Contains(details[0], "TestWithStackMultiUnwrapSlice") { + t.Errorf("expected safe details containing function name, got %v", details) + } + + // Verify Error() formatting. + if wrapped.Error() != multi.Error() { + t.Errorf("expected Error() %q, got %q", multi.Error(), wrapped.Error()) + } + + // Verify Format with %+v includes attached stack trace. + verbose := fmt.Sprintf("%+v", wrapped) + if !strings.Contains(verbose, "TestWithStackMultiUnwrapSlice") { + t.Errorf("expected verbose format to contain test name, got: %s", verbose) + } +} + +func TestWithStackSingleUnwrap(t *testing.T) { + single := &customSingleError{msg: "single"} + wrapped := withstack.WithStack(single) + + // Single error wrapper must implement Unwrap() error, NOT Unwrap() []error. + if _, ok := wrapped.(interface{ Unwrap() []error }); ok { + t.Fatalf("single error wrapper should not implement Unwrap() []error") + } + + u, ok := wrapped.(interface{ Unwrap() error }) + if !ok { + t.Fatalf("expected single error wrapper to implement Unwrap() error") + } + if u.Unwrap() != single { + t.Errorf("expected unwrapped %v, got %v", single, u.Unwrap()) + } +} + +func TestWithStackDepthNil(t *testing.T) { + if err := withstack.WithStack(nil); err != nil { + t.Errorf("expected nil, got %v", err) + } + if err := withstack.WithStackDepth(nil, 2); err != nil { + t.Errorf("expected nil, got %v", err) + } +}