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

[carnott-snap]

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.

[ianlancetaylor]

CC @neild @jba

[neild]

How is this different from #40442?

I don't think we should make any changes to the semantics of Is(err, nil). The property that Is(err, nil) == (err == nil) is an important one.

[carnott-snap]

#40442 was filed as a bug, and I went into it with the impression that something was wrong. This is a proposal to change the current behaviour.

This property is undocumented, and (to my knowledge) was not mentioned as (an essential) part of the Go2 error handling discussion. If this is important, we should document it. That being said, can you describe why you feel this is an important property? We will still have err != nil, so it seems worthwhile to use errors.Is to do something that err != nil cannot. Plus, it makes for a simpler mental model if when I implement func (MyError) Is(error) bool { return true } it always gets executed before equality checks happen.

[neild]

There is a great deal of existing Go code which uses if err != nil {} to test for error conditions. We do not want to invalidate that code. If we introduce errors which are non-nil under equality, but match errors.Is(err, nil), then we break the existing convention of a nil error indicating success.

[carnott-snap]

There is a great deal of existing Go code which uses if err != nil {} to test for error conditions. We do not want to invalidate that code.

This change would not invalidate that code, similar to how one is not required to move from err == nil to errors.Is, we would simply add a new mechanism that (may be preferred over err != nil, and) could be used by new apis.

If we introduce errors which are non-nil under equality, but match errors.Is(err, nil), then we break the existing convention of a nil error indicating success.

Sure, but I do not think this is something that most users care about. To me, the whole point of errors.Is we allow implementers to design custom error equality, as opposed to just relying upon the runtime's equality definition. Why is nil sacred? If it is, why are we allowed to pass nil to errors.Is at all?

Reiterating the costs section, if this opinion is canonical, we need to update the documentation to make clear what is going on. But speaking to @ianlancetaylor's comment on a different issue, #39799, it seems problematic that we have two ways to do one thing.

[ianlancetaylor]

It sounds like you are suggesting that there should be cases where errors.Is(err, nil) returns true even if err != nil. That sounds extremely confusing.

[carnott-snap]

That is what I am suggesting, but to me this is very intuitive, since errors.Is is not just an equality check. Stated differently, why is "errors.Is(err1, err2) returns true even if err1 != err2" intuitive, but only if err2 is not nil?

This is not the implementation, but I always thought about errors.Is as if x, ok := err.(interface{...}); ok { return x.Is(target) } return err == target.

[ianlancetaylor]

@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.

[carnott-snap]

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.

[ianlancetaylor]

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).

That isn't the case. There is tons of Go code that correctly says err != nil. The great majority of code doesn't care what specific kind of error it has; it only cares whether it has an error or not. Today, the way to test that is err != nil. Even if we were able to adopt your suggestion, people would not change to errors.Is(err, ErrXxx); they would change to !errors.Is(err, nil).

I agree that new APIs could document that callers must test errors using errors.Is rather than err != nil. But since most people don't read API documentation in detail, that would lead to the confusing situation that I described. And, of course, there are plenty of old APIs that people would want to change, over time, to call new APIs. They can't change their error semantics, so we would see lots of code like

    err := newapi.F()
    if errors.Is(err, nil) {
        err = nil
    }
    return err

I am honestly struggling to imagine what possible benefit we could see from errors.Is(err, nil) that would be worth this cost.

Again, this suggestion might be plausible if we didn't have a large amount of existing code. But we can't ignore that code, we can't ignore the switching costs, we can't ignore the interoperability costs. We need an extraordinary benefit to be worth paying the cost. I don't see that benefit.

Every change has a cost. We can't judge changes solely by the benefit they bring. We must compare the benefit to the cost.

[carnott-snap]

I agree that for extant interfaces, they would need to keep whatever compatibility promise they offer today, but this is like with the inclusion of errors.Is(err, ErrXxx), if err == ErrXxx used to work, you cannot just drop it. One must document how your api works, even today.

A new paradigm may introduce a desire for adaptor packages, but there is no requirement. If we considered this a large enough population, we (or a third party library) could write an adaptor function:

package errors

func Old(err error) error {
        if !errors.Is(err, nil) {
                return err
        }
        return nil
}

And your example becomes:

err := errors.Old(newapi.F())

And if you need to add functionality from the new style to an old api, the code reads identically to err != nil:

func old() error {
        v, err := new()
        if !errors.Is(err, nil) {
                return err // or wrapped fmt.Errorf("old to new: %w", err)
        }
        _ = v // consume v
        return nil
}

I will not suggest this has no costs, and respect that we are implicitly making most of the ecosystem's error handling code an anti-pattern. However there is no interoperability issue, because we do not break any existing code. When you write new code, it will look different, and if you want to migrate, that is each library owner's choice, much like errors.Is(err, ErrXxx). My goal is to have a single interface for error checking that is easy to reason about.

[neild]

and respect that we are implicitly making most of the ecosystem's error handling code an anti-pattern.

We aren't going to make almost all existing error handling code an anti-pattern. No benefit we can realize here will be worth that cost.

[carnott-snap]

But we already did that for err == ErrXxx, how is this any different?

[ianlancetaylor]

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.

[carnott-snap]

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.

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?

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.

Is 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.

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.

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.

[ianlancetaylor]

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.

[carnott-snap]

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.

Just for your visibility, there are three linters that I have seen/used that have this property:

• https://github.com/golangci/golangci-lint
• https://github.com/Djarvur/go-err113
• https://github.com/romanyx/erris

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.

This nuance is lost on new users. When I read this blog post, I got the strong impression that because errors.Is quacks like == that it kind of replaced it. I appreciate the nuance on this topic, but you may well have an optics issue, if this it not made clear to the community.

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.

To someone who has been using err == io.EOF for years, sure, but to a new user, who is unclear what the difference between == and errors.Is, this is pretty weird. If we are optimising for domain experts that is fine, but that is what I expect from Rust, not from Go.

I think it would be unnecessarily confusing if errors.Is(err, nil) were different from err == nil.

Is it really any more confusing than errors.Is(err1, err2) being different err1 == err2? I find it more confusing that there is a special case for err2 == nil.

[ianlancetaylor]

Is it really any more confusing than errors.Is(err1, err2) being different err1 == err2? I find it more confusing that there is a special case for err2 == nil.

The only possible meaning I can think of for errors.Is(err, nil) is "err is not an error." And that is exactly what err == nil already means. I think it would be very strange if two expressions that mean the same thing do not always have the same truth value.

Thanks for your comments about optics and about those linters. Still, I'm confident that those linters don't warn about err == nil, and I don't see anything in the blog post about err == nil.

[rsc]

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.

[carnott-snap]

Thanks for your comments about optics and about those linters. Still, I'm confident that those linters don't warn about err == nil, and I don't see anything in the blog post about err == nil.

Correct, they suggest err == ErrXxx -> errors.Is(err, ErrXxx).

The weight of history weighs far too much against breaking all existing Go code in the world for a cleanup.

This is not a breaking change.

[rsc]

Telling everyone that all their code using err == nil is now incorrect and must be updated to use errors.Is(err, nil) is a breaking change. We can't rewrite all the code in the world.

[carnott-snap]

As long as old code continues to compile and run, a change is not breaking. It is not a breaking change to deprecate a design pattern.

[rsc]

No change in consensus, so declined.