Skip to content

Commit

Permalink
Common: Fix fmt msg lost in AppendError().Error()
Browse files Browse the repository at this point in the history
Retain the .msg field of a go fmt.Errorf .msg field returned by .Error()
when wrapping multiple errors.
This fixes a situation where a nested stack of errors would lose
formatting information, which is often used to supply identifying
context.
e.g.
```
err = fmt.Errorf("%w `%s`: %w", errParsingField, fieldName,
parsingError)
errs = common.AppendError(errs, err)
```

This isn't really an issue with our implementation; Calling Unwrap() on a
fmt.Errorf() which returns a wrapErrors will lose that formatting.
Our issue was that we were using just Unwrap() to bind together our
chain-of-custody.
  • Loading branch information
gbjk committed Feb 19, 2024
1 parent 9f96815 commit ae74d1d
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 12 deletions.
55 changes: 43 additions & 12 deletions common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -463,11 +463,22 @@ func InArray(val, array interface{}) (exists bool, index int) {
return
}

// fmtError holds a formatted msg and the errors which formatted it
type fmtError struct {
errs []error
msg string
}

// multiError holds errors as a slice
type multiError struct {
errs []error
}

type unwrappable interface {
Unwrap() []error
Error() string
}

// AppendError appends an error to a list of exesting errors
// Either argument may be:
// * A vanilla error
Expand All @@ -481,20 +492,35 @@ func AppendError(original, incoming error) error {
if original == nil {
return incoming
}
newErrs := []error{incoming}
if u, ok := incoming.(interface{ Unwrap() []error }); ok {
newErrs = u.Unwrap()
if u, ok := incoming.(unwrappable); ok {
incoming = &fmtError{
errs: u.Unwrap(),
msg: u.Error(),
}
}
if u, ok := original.(interface{ Unwrap() []error }); ok {
return &multiError{
errs: append(u.Unwrap(), newErrs...),
switch v := original.(type) {
case *multiError:
v.errs = append(v.errs, incoming)
return v
case unwrappable:
original = &fmtError{
errs: v.Unwrap(),
msg: v.Error(),
}
}
return &multiError{
errs: append([]error{original}, newErrs...),
errs: append([]error{original}, incoming),
}
}

func (e *fmtError) Error() string {
return e.msg
}

func (e *fmtError) Unwrap() []error {
return e.errs
}

// Error displays all errors comma separated
func (e *multiError) Error() string {
allErrors := make([]string, len(e.errs))
Expand All @@ -506,11 +532,16 @@ func (e *multiError) Error() string {

// Unwrap returns all of the errors in the multiError
func (e *multiError) Unwrap() []error {
return e.errs
}

type unwrappable interface {
Unwrap() []error
errs := make([]error, 0, len(e.errs))
for _, e := range e.errs {
switch v := e.(type) {
case unwrappable:
errs = append(errs, unwrapDeep(v)...)
default:
errs = append(errs, v)
}
}
return errs
}

// unwrapDeep walks down a stack of nested fmt.Errorf("%w: %w") errors
Expand Down
7 changes: 7 additions & 0 deletions common/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -703,6 +703,13 @@ func TestErrors(t *testing.T) {
assert.ErrorIs(t, ExcludeError(err, e5), e3, "Excluding e5 should retain e3")
assert.ErrorIs(t, ExcludeError(err, e5), e4, "Excluding e5 should retain the vanilla co-wrapped e4")
assert.NotErrorIs(t, ExcludeError(err, e5), e5, "e4 should be excluded")

// Formatting retention
err = AppendError(e1, fmt.Errorf("%w: Run out of `%s`: %w", e3, "sausages", e5))
assert.ErrorIs(t, err, e1, "Should be an e1")
assert.ErrorIs(t, err, e3, "Should be an e3")
assert.ErrorIs(t, err, e5, "Should be an e5")
assert.ErrorContains(t, err, "sausages", "Should know about secret sausages")
}

func TestParseStartEndDate(t *testing.T) {
Expand Down

0 comments on commit ae74d1d

Please sign in to comment.