I'm splitting this off from #29934 to allow for more focused discussion.
There are three common patterns used in Go code to make control flow decisions based on an error's value.
err == X._, ok := err.(T); okf(err).The error values proposal provides Is and As functions to test an error's chain against a sentinel value or underlying type, respectively. It does, however, not currently provide direct support for the third pattern of predicate functions.
Proposal: Add an IsFunc function to test an error's chain against a predicate function to the golang.org/x/xerrors and errors packages.
// IsFunc calls f for each error in err's chain. IsFunc immediately returns true if f
// returns true. Otherwise, it returns false.
func IsFunc(err error, f func(error) bool) bool { ... }
IsFunc can be used directly with existing predicate functions like os.IsNotExist:
if errors.IsFunc(err, os.IsNotExist) { ... }
Why not update os.IsNotExist et al. to unwrap errors?
No good way to write code that works with current and future versions of Go: You can't rely on os.IsNotExist unwrapping so you need to do it yourself to support pre-1.13 Go.
This puts the onus on the user to determine if any given predicate function supports unwrapping, and as of what version.
Passing a wrapped error to a predicate which doesn't unwrap is a silent runtime bug. The error can't be detected at compile time, and it isn't even a runtime panic; you just silently take the wrong branch.
If we were starting entirely anew, then it might make sense to put the unwrapping in the predicate functions. Doing so at this time seems difficult to accomplish without encouraging the creation of many subtle bugs.
I also played with a version that returns the error that matches the predicate or nil, but
!= nil on the endThe example predicate I used in the previous thread:
func temporary(err error) bool {
t, ok := err.(interface{ Temporary() bool })
return ok && t.Temporary()
}
There's talk in the original thread of updating As to let you assign to interfaces. With that you could do
type temp interface { Temporary() bool }
if xerrors.As(err, &temp) && temp.Temporary() { ... }
but note a subtle difference: IsFunc(err, temporary) returns true if any error in the chain both matches that interface and returns true but the As example only takes the error path if the first error in the chain that matches the interface returns true. Which is correct depends on the application and the potential composition of the error chain, so it's hard to say whether that subtlety is a feature or a bug. This would apply to xerrors.IsFunc(err, os.IsTimeout).
For os.IsExist et al., many of those predicates has a corresponding sentinel error like os.ErrExist that is among the errors that cause the predicate to return true. With appropriate Is methods on *os.PathError et al., those methods would allow you to write xerrors.Is(err, os.ErrExist). That would work nicely and consistently for code that does not need compatibility but otherwise has the same drawbacks as updating the predicates themselves.
With appropriate Is methods on *os.PathError et al., those methods would allow you to write xerrors.Is(err, os.ErrExist).
I think this is absolutely the cleanest API to expose to a user, and we should consider doing it. It does, as you say, have the same drawback of potentially becoming a silent bug in older Go versions. On the other hand, if you always write errors.Is(err, os.ErrExist) (not xerrors.Is), then the failure is at compilation time.
If a predicate function eventually adds support for unwrapping for the user, wouldn't this result in a O(n^2) runtime?
Suppose you had errors E3 (outer-most), E2, and E1 (inner-most). None of which match the predicate function. Then calling:
xerrors.IsFunc(E3, MyFunc)
would result in a call tree like:
call xerrors.IsFunc(E3, MyFunc)
โโโ handle E3
โ โโโ call MyFunc(E3)
โ โโโ handle E3
โ โโโ handle E2
โ โโโ handle E1
โโโ handle E2
โ โโโ call MyFunc(E2)
โ โโโ handle E2
โ โโโ handle E1
โโโ handle E1
โโโ call MyFunc(E1)
โโโ handle E1
The problem is that both IsFunc and the predicate function walks the entire chain. IsFunc doesn't know if the provided predicate function will already walk the chain, and the predicate function doesn't know if it is being called by code that is already walking the chain. As such, both functions duplicate the work and walk the chain.
For sufficiently long error-chains, this can be kinda costly.
If a predicate function eventually adds support for unwrapping for the user, wouldn't this result in a
O(n^2)runtime?
O(n log n), I think?
But yes, adding errors.IsFunc implies that predicate functions should never add support for implicit unwrapping. Fortunately this doesn't require any work, since none of them unwrap today.
I also played with a version that returns the error that matches the predicate or nil, but
I use a version that returns an error
- it's almost never needed. In situations where this comes up you mostly just care whether there is an error that matches the predicate.
- when it's not needed, you have to tack a
!= nilon the end
same as the universal if err := ...; err != nil {...}, it's the go way.
- when it is needed, you still need to do whatever assertion/testing again to recover the desired error so at that point it makes more sense to write your own helper that does the tests and returns the correct kind of error
Yes, but the helper might as well use the error-returning version. Also Code generation makes helper writing trivial (if it wasn't already).
After discussing this internally, we've decided not to add errors.IsFunc or the equivalent at this time.
We believe that the cleanest and best API to expose to users is one based on sentinel errors. You should write errors.Is(err, os.ErrIsNotExist), not errors.IsFunc(err, os.IsNotExist). Adding IsFunc would encourage packages to use the latter. We will update the os package to permit the former.
We will not update os.IsNotExist to unwrap errors, for the reasons detailed in the original message above: Having this function behave differently in Go 1.12 and Go 1.13 is too hazardous.
Nothing, of course, prevents third-party packages from adding IsFunc. We can revisit the decision to add it in the future if it turns out to be broadly useful.
Most helpful comment
If a predicate function eventually adds support for unwrapping for the user, wouldn't this result in a
O(n^2)runtime?Suppose you had errors E3 (outer-most), E2, and E1 (inner-most). None of which match the predicate function. Then calling:
would result in a call tree like:
The problem is that both
IsFuncand the predicate function walks the entire chain.IsFuncdoesn't know if the provided predicate function will already walk the chain, and the predicate function doesn't know if it is being called by code that is already walking the chain. As such, both functions duplicate the work and walk the chain.For sufficiently long error-chains, this can be kinda costly.