From 54d5f61896260c4ce04a4a0c9b3b406a39eaf42b Mon Sep 17 00:00:00 2001 From: Aaron Zinger Date: Mon, 25 Apr 2022 13:55:58 -0400 Subject: [PATCH] secondary: add SummarizeErrors variadic method I was recently debugging a workflow with lots of async workers able to cancel each other, so that by the time an error bubbled up it was ambiguous where the longest stack would be and whether there were errors in two places because one wrapped the other or because of two separate root errors. I felt like what would be most convenient would be to smoosh everything together rather than try to be too smart about it. This PR implements SummarizeErrors, which is mainly the generalization of CombineErrors over n errors. If one argument is detectably already wrapping another, the result will ignore the latter. If two errors are distinct but both wrapping the same error, the result will preserve that information. Two errors are distinct if err1 != err2; there didn't look to be much benefit in bringing in things like .Is as we're really interested in identity, not equivalence. --- secondary/secondary.go | 60 +++++++++++++++++++++++++++++++++++++ secondary/secondary_test.go | 27 +++++++++++++++++ secondary_api.go | 8 +++++ 3 files changed, 95 insertions(+) diff --git a/secondary/secondary.go b/secondary/secondary.go index a680202..fd8f494 100644 --- a/secondary/secondary.go +++ b/secondary/secondary.go @@ -14,6 +14,8 @@ package secondary +import "github.com/cockroachdb/errors/errbase" + // WithSecondaryError enhances the error given as first argument with // an annotation that carries the error given as second argument. The // second error does not participate in cause analysis (Is, etc) and @@ -44,3 +46,61 @@ func CombineErrors(err error, otherErr error) error { } return WithSecondaryError(err, otherErr) } + +// SummarizeErrors reduces a collection of errors to a single +// error with the rest as secondary errors, making an effort +// at deduplication. Use when it's not clear, or not deterministic, +// which of many errors will be the root cause. +func SummarizeErrors(errs ...error) error { + if len(errs) == 0 { + return nil + } + uniqArgsInOrder := make([]error, 0, len(errs)) + uniqArgsMap := make(map[error]struct{}, len(errs)) + refCount := make(map[error]int) + for _, e := range errs { + if _, dup := uniqArgsMap[e]; !dup { + uniqArgsMap[e] = struct{}{} + uniqArgsInOrder = append(uniqArgsInOrder, e) + walk(e, func(w error) { refCount[w] = refCount[w] + 1 }) + } + } + var retVal error + for _, e := range uniqArgsInOrder { + if refCount[e] == 1 { + retVal = CombineErrors(retVal, e) + } + } + return retVal +} + +func walk(err error, fn func(error)) { + if err != nil { + fn(err) + walk(errbase.UnwrapOnce(err), fn) + if se, ok := err.(*withSecondaryError); ok { + walk(se.secondaryError, fn) + } + } +} + +func combineIfDistinct(err error, otherErr error) error { + // if err is one of the causes of otherErr, return otherErr. + e := otherErr + for e != nil { + if err == e { + return otherErr + } + e = errbase.UnwrapOnce(e) + } + // if otherErr is one of the causes of err, return err. + e = err + for e != nil { + if otherErr == e { + return err + } + e = errbase.UnwrapOnce(e) + } + // otherwise combine them. + return CombineErrors(err, otherErr) +} diff --git a/secondary/secondary_test.go b/secondary/secondary_test.go index e65b8f0..988060e 100644 --- a/secondary/secondary_test.go +++ b/secondary/secondary_test.go @@ -102,6 +102,33 @@ func TestCombineErrors(t *testing.T) { } } +// This test asserts SummarizeErrors output in terms of CombineErrors output. +func TestSummarizeErrors(t *testing.T) { + tt := testutils.T{T: t} + err1 := errors.New("err1") + err2 := errors.New("err2") + chainWith2 := &werrFmt{err2, "chainWith2"} + chainWith1And2 := secondary.CombineErrors(chainWith2, err1) + + testData := []struct { + args []error + summary error + }{ + {[]error{err1, err2}, secondary.CombineErrors(err1, err2)}, + {[]error{err2, err1}, secondary.CombineErrors(err2, err1)}, + {[]error{err1, err2, err1}, secondary.CombineErrors(err1, err2)}, + {[]error{chainWith2, err2}, chainWith2}, + {[]error{err2, chainWith2}, chainWith2}, + {[]error{chainWith2, err1, err2}, secondary.CombineErrors(chainWith2, err1)}, + {[]error{chainWith2, chainWith1And2, err1, err2}, chainWith1And2}, + } + + for _, test := range testData { + err := secondary.SummarizeErrors(test.args...) + tt.CheckDeepEqual(err, test.summary) + } +} + func TestFormat(t *testing.T) { tt := testutils.T{t} diff --git a/secondary_api.go b/secondary_api.go index 2f5e34f..03546cd 100644 --- a/secondary_api.go +++ b/secondary_api.go @@ -40,3 +40,11 @@ func WithSecondaryError(err error, additionalErr error) error { func CombineErrors(err, otherErr error) error { return secondary.CombineErrors(err, otherErr) } + +// SummarizeErrors reduces a collection of errors to a single +// error with the rest as secondary errors, making an effort +// at deduplication. Use when it's not clear, or not deterministic, +// which of many errors will be the root cause. +func SummarizeErrors(errs ...error) error { + return secondary.SummarizeErrors(errs...) +}