Hey friends,
We own a library, multierr that supports writing combining zero or more
errors into a single error. It supports an API like,
err := multierr.Combine(err1, err2, ...)
With the release of Go 1.13, we’re trying to evaluate what the expected
behavior is for an error like this with regards to errors.Is
anderrors.As
.
We understand that the discussion around error trees was deferred as part of
the original Error Values discussion, but we’d like to get feedback on what
the community expects from error trees with errors.Is
and errors.As
.
The PR we have open implements the following behavior:
errors.As
, the first error in the list where errors.As
succeeds iserrors.Is
, all errors are checked until one matches, in which case weThis means that if you have multiple os.PathError
s, you can use errors.As
to extract the first of those, and you can use errors.Is
to match for
equality against any of those.
What we’d like to ask the community and the Go Developers is whether this
behavior is what you would expect from a combined error tree like this.
Thanks.
Abhinav
The PR we have open implements the following behavior:
- For
errors.As
, the first error in the list whereerrors.As
succeeds is
returned.- For
errors.Is
, all errors are checked until one matches, in which case we
succeed. If none of the errors matched, we fail.This means that if you have multiple
os.PathError
s, you can useerrors.As
to extract the first of those, and you can useerrors.Is
to match for
equality against any of those.What we’d like to ask the community and the Go Developers is whether this
behavior is what you would expect from a combined error tree like this.
Thanks.
Abhinav
Hey,
Thanks for your response. Although we were unsure about this at first (which
is why we started this thread), after further discussion we agree that this
API makes sense. Part of the discussion is summarized below for others trying
to make a similar decision, and to help plan future APIs around multi-error
situations.
We had a couple requirements in mind during this discussion:
errors.Is
errors.As
. That is, if errors.Is(err, target)
succeeds,errors.As(err, &targetType)
should also succeed.error
unless explicitly documented. So introducing newmultierr
package for errors.Is/As
was to be avoidedThe first issue we discussed was what users expect when they callerrors.Is(err, cause)
where err
is a multierr error (but the user does not
know that). Does the user expect a match if all errors inside err
match,
or if any error inside err
matches? Valid scenarios are conceivable in
both directions. We agreed that there were more cases for the “any error may
match” route than the “all errors must match.”
The second issue was around loss of information with errors.As
when the
error is a multierr error. For non-multierr errors, when extracting an error
with errors.As
, you will usually have one instance of each error type in the
chain.
fmt.Errorf("something went wrong: %w", myError{Cause: net.OpError{Err: ..}})
It will be uncommon to have, myError{...{Err: myError{...}}}
where myError
is meaningful. So the “first match succeeds” behavior implemented byerrors.As
suffices for most cases.
With multierr errors, it will be more common to have multiple error chains
with the same wrapper type as these will be commonly produced from the same
context.
err = multierr.Combine(
fooFailed{Cause: net.OpError{..}},
fmt.Errorf("...: %w", fooFailed{Cause: context.DeadlineExceeded}),
net.OpError{Err: fooFailed{cause: os.ErrNotExist}},
)
Returning just the first match with errors.As
seems like a significant loss
of information, and non-deterministic if the error list is constructed
non-deterministically (from multiple concurrent operations, for example).
We think that the ideal behavior here would be for errors.As
to produce a
view of the original multierr error, filtered down to the requested type. That
is, something roughly equivalent to the following:
// var target fooFailed
// errors.As(err, &target)
*target = multierr.Combine(
fooFailed{Cause: net.OpError{..}},
fooFailed{Cause: context.DeadlineExceeded},
fooFailed{cause: os.ErrNotExist},
)
// or,
// var target []fooFailed
But that’s impossible to do cleanly with the current design of errors.As
:target
is expected to be of type fooFailed
, not multierr
‘s error type.
Eventually we decided that the loss of information with errors.As
, although
regrettable, is acceptable in the short term while we experiment with the new
functionality internally.
We may also end up bending requirement (2) above and implement amultierr.Find
API that extracts a slice of matching error objects by type,
but we’d like to experiment with just the basic functionality first.
In conclusion, we decided that the PR on multierr
is fine as-is. We’ll merge
and release it in the near future.
We hope this discussion was helpful to others, and perhaps provided
inspiration for future APIs around multi-error use cases.
Thanks.
Abhinav
--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/4af1fe63-bdc6-484b-a05f-49667f89f86d%40googlegroups.com.