go: proposal: errors: allow implementing err.Is(nil) == true

background

Currently you can implement, func (MyError) Is(target) bool { return true }, but MyError.Is will only be called if target, in errors.Is, is non-nil. This can be confusing to implementers of interface{ Is(error) bool }, and disallows signalling that a given returned error is nil. (This is useful when you want to return error metadata, but nothing actually failed.)

description

I propose removing the target == nil check from errors.Is. This will hand full control of equality checking to interface { Is(error) bool }, and make the user experience more consistent.

--- a/src/errors/wrap.go
+++ b/src/errors/wrap.go
@@ -37,10 +37,6 @@ func Unwrap(err error) error {
 // then Is(MyError{}, os.ErrExist) returns true. See syscall.Errno.Is for
 // an example in the standard library.
 func Is(err, target error) bool {
-       if target == nil {
-               return err == target
-       }
-
        isComparable := reflectlite.TypeOf(target).Comparable()
        for {
                if isComparable && err == target {

costs

It is my understanding that we do not currently advocate for !errors.Is(err, nil) over err != nil, so the impact of this should be limited. Furthermore, if you have implemented func (MyError) Is(error) bool { return true }, you are either relying upon undefined behaviour or have a bug. That being said, this is a semantic change, so we should be careful to understand what the impact is.

alternatives

If the current implementation is preferred, it should be explicitly documented.Is reports whether any error in err's chain matches target, does not describe that for target == nil MyTarget.Isonly equality, and neverMyError.Is`, will checked.

About this issue

Original URL
State: closed
Created 4 years ago
Reactions: 2
Comments: 23 (13 by maintainers)

Most upvoted comments

Based on the conversation above, this seems like a likely decline. The weight of history weighs far too much against breaking all existing Go code in the world for a cleanup.

rsc on Aug 12, 2020

While this nuance may be accurate, it is not well captured in documentation or (community) linters. Many linters outright require use of errors.Is over ==. Plus, it does not seem valuable: why have two canonical ways to do roughly the same thing?

Hmmm. If linters are suggesting that code change err == io.EOF to errors.Is(err, io.EOF), then I think those linters are making a mistake, and I hope they stop doing that. Similarly, it is absolutely fine when using golang.org/x/sys/unix to write if fd, err := unix.Open(...); err == unix.ENOENT.

I don’t think of == and errors.Is as being roughly the same thing. They are related, of course, but not the same. Using == lets you test whether an error is exactly some value. This is clearly useful and meaningful when using syscall.Errno, for example. Using errno.Is lets you test whether an error contains some error, or more generally has some property. For example, errors.Is(syscall.EACCES, os.ErrPermission) returns true. This is so even though obviously syscall.EACCES != os.ErrPermission.

s this not worse than migrating wholesale to errors.Is. Currently I have to guess/decipher if a given function returns an errors.Isable. This speaks to the linters above, but many people just want one way to check errors.

For almost all code there is just one way to check errors: err != nil. What we are talking about here is when we want to pick out a particular error property. And there isn’t just one way to do that. Sometimes we use ==. Sometimes we use type assertions. Sometimes we use errors.Is. Sometimes we use errors.As. They all work in different cases. None of them are wrong.

It’s true that we could say "everybody stop writing err == io.EOF and start writing errors.Is(err, io.EOF). It’s fairly likely that all code would continue to work if we made that change. But the Go compatibility rules mean that to the greatest extent possible, err == io.EOF must continue to work in the future for cases where it works today. And err == io.EOF is perfectly clear. So there is no reason to for us to tell people to change it to errors.Is(err, io.EOF). So we don’t do it.

Can this change not be staged in the same way? I will admit I assumed that errors.Is(err, ErrXxx) was cannon, and that has flavoured both my proposal and this discussion. But similarly this need not be required, if people want to have callers use errors.Is(err, nil) they can, but it is not mandated and existing code does not break.

If people really want to write errors.Is(err, nil), I won’t stop them. But lots of code writes err != nil today and in the future. We are never going to change that. We don’t want to change that. Given that fact, I think it would be unnecessarily confusing if errors.Is(err, nil) were different from err == nil.

ianlancetaylor on Aug 12, 2020

That is not what we did. In the past, it only made sense to write err == ErrXxx if the API of the function promised that it would return ErrXxx in some scenarios. And in the present and the future, that remains true: if the API of the function promises that it returns ErrXxx in some scenarios, then it is entirely reasonable and appropriate to write err == ErrXxx. There is no need to change any of the existing code, and it is perfectly fine to continue to write code that way.

What we changed was to add a new capability: some functions may now explicitly define the API as returning an error that wraps some other error, typically be providing additional information. This isn’t going to be changed for code that already specifies that it returns ErrXxx, of course. This specification is only going to be defined for APIs that return wrapping errors. For those APIs, you can now use errors.Is to check for the error being wrapped. This intended to be convenient for cases where it is appropriate to wrap errors, such as uses of os.PathError.

The fact that we introduced error.Is as useful for working with wrapping errors does not mean that we expect people to start using errors.Is everywhere. We don’t expect that. Functions can continue to return specific errors, and those errors should normally be checked using err == ErrXxx, not errors.Is. Nobody is expected to write errors.Is(err, io.EOF). They are expected to write err == io.EOF. Similarly for other explicitly exported errors, such as flag.ErrHelp.

In other words, err == ErrXxx is not an anti-pattern now, any more than it ever was. Using error.Is provides a new facility for some functions. It doesn’t change how code is expected to work with errors that they return.

ianlancetaylor on Aug 11, 2020

@carnott-snap Today we have a world in which a function either returns an error (err != nil) or does not return an error (err == nil). You seem to be envisioning a world in which a function either returns an error (!errors.Is(err, nil)) or does not return an error (errors.Is(err, nil)). If we had started with the second world, that would be a reasonable approach to consider. But today we are in the first world. Trying to change that today will give us a situation where nobody knows how to check whether a function returns an error, because they won’t know whether to write err != nil or !errors.Is(err, nil). What you suggest may well be very intuitive if we had done things that way from the start. But we didn’t. If we tried to change it now, the overall effect on the Go ecosystem would be, in my opinion, extremely confusing.

ianlancetaylor on Aug 11, 2020

No change in consensus, so declined.

rsc on Aug 26, 2020

You seem to be suggesting that we cannot migrate to away from equality checks. This is inaccurate for new apis. If I document that my api requires go1.13+ and returned errors must be checked with errors.Is, we do not need to worry about what == or != do. This to me feels like what we have done with errors.Is(err, ErrBad), assuming one implementing interface { Is(error) bool }, err == ErrBad will not work. It confuses me that we are holding nil as a special case, where we need to retain compatibility for err == nil, but we are fine improving (and implicitly breaking) things for err == ErrBad.

Also, your world view seems to be missing a step:

Before go1.13, we had a world in which a function either returns an error (err != nil) or does not return an error (err == nil).
Today, we have a world in which a function either returns an error (errors.Is(err, ErrXxx)) or does not return an error (err == nil).
Tomorrow we could have a world in which a function either returns an error (errors.Is(err, ErrXxx)) or does not return an error (errors.Is(err, nil)).

Depending on how the compatibility guarantee is read, I would be understanding if we needed to deprecate errors.Is and use a new name, like errors.Is2, for the few people who are relying upon errors.Is(err, nil) today.

carnott-snap on Aug 11, 2020