go: proposal: errors: add Like for testing that error values appear as expected
Problem
Currently, there is no general way to test whether a returned error value is what the developer expected:
- The obvious == operator doesn’t work, since *errors.stringError is designed to be unique: https://github.com/golang/go/issues/17226#issuecomment-309125918.
- Using errors.Is is also unsatisfactory because the developer must go through extra effort to test both the wrapped error and the Error string, as demonstrated by fmt_test.TestErrorf.
- reflect.DeepEqual is sometimes correct for testing errors, but makes these tests particularly brittle when it comes to wrapped errors.
- Finally, cmp.Equal is a popular third-party package for comparisons, but even its cmpopts.EquateErrors option doesn’t do the right thing here.
Ideally, a developer could write the following test:
func TestFunc(t *testing.T) {
wantErr := fmt.Errorf("my: %w", fs.ErrNotExist)
err := my.Func()
if !errors.Like(err, wantErr) {
t.Fatalf("err %v, want %v", err, wantErr)
}
}
Workarounds
Often, we see a helper function inside the test suite that helps compare Error text by dealing with nil errors:
func Error(err error) string {
if err == nil {
return ""
}
return err.Error()
}
func TestFunc(t *testing.T) {
wantErr := fmt.Errorf("my: %w", fs.ErrNotExist)
err := my.Func()
if Error(err) != wantErr {
t.Fatalf("err %v, want %v", err, wantErr)
}
}
This is unsatisfying now that we have wrapped errors, because even if the Error strings are equal, that doesn’t mean that any wrapped errors match.
Concerns
- The name of errors.Like may not be obvious. I considered errors.Match, but the errors documentation already uses the word “match” with a slightly different meaning.
- Since the primary use-case for this function is to test errors, and not to handle them, it may not belong in the errors package. We don’t want developers to choose Like when they mean Is. Perhaps it should go in a new errors/errorstest package?
Proposed implementation
The following is a proposed implementation of errors.Like:
// Like reports whether err is equivalent to target.
//
// An error is considered to be equivalent if it is equal to the target.
// It is also equivalent if its Error string is equal to the target’s Error
// and its wrapped error is equivalent to the target’s wrapped error.
func Like(err, target error) bool {
if err == target {
return true
}
if err == nil || target == nil {
return false
}
if utarget := Unwrap(target); utarget != nil {
if err.Error() != target.Error() {
return false
}
return Like(errors.Unwrap(err), utarget)
}
return false
}
You can also find an implementation with test cases in the Go Playground: https://play.golang.org/p/qnBbkSbMlLO
See also
About this issue
- Original URL
- State: closed
- Created 3 years ago
- Comments: 20 (19 by maintainers)
See https://pkg.go.dev/upspin.io/errors and https://commandcenter.blogspot.com/2017/12/error-handling-in-upspin.html.
As explained in the article, to work well this functionality tends to be application-specific. Your proposal puts too much weight on equality to be widely useful. Things like user names and file names must be factored out.
This seems to me to be an excellent argument against adding a function like this. We should not encourage treating error strings as part of a package’s API.
strings.Contains(err, "permision denied")contains a typo, but it can forerrors.Is(err, ErrPermissionDenied).ErrPermissionDeniedsentinel) makes the API surface clear.Ad hoc error text matching is simply not a good API. There are much better alternatives.
This is not the position taken for the Go standard library. We do not consider changes to error text to be backwards-incompatible changes, and therefore can improve errors without violating the Go compatibility promise. (We have on occasion reverted error text changes when the practical impact was large, although arguably doing so sets a bad precedent.)
Comparing error strings should be a last resort, when other methods of classification fail. And even then, it would be better to look for stable substrings in the message instead of comparing it whole, for the reason Rob mentioned above.