Is() is not safe as replacement to errors.Is

[titouanfreville]

Hello,
While working around go errors, I had an issue with the Is function.
It does not use the default errors.Is and only compare for strict equality between base errors witch is the default comportment of original Is but not its full implementation.
Basically, we are using a custom error type witch reimplement Is and I expected it to be called on goerrors.Is but it is not the cased.
It also mean that current goerrors.Is will not have expected behaviour on wrapped errors.

//
// Is unwraps its first argument sequentially looking for an error that matches the
// second. It reports whether it finds a match. It should be used in preference to
// simple equality checks:
//
// if errors.Is(err, os.ErrExist)
//
// is preferable to
//
// if err == os.ErrExist
//
// because the former will succeed if err wraps os.ErrExist.
(errors/errors.go)

[ConradIrwin]

Thanks for the report!

This module was written before the new go error changes, but it seems like a good idea to integrate with the decisions they chose.

It's not technically backward compatible to start using .Is() instead of ==, but I think it's what most users of this library would expect, so it's a change I'm in favor of making.

Would you like to send a pull request to fix this?

P.S. Out of curiosity, what leads to you still using this library in go > 1.14? I haven't thought hard about it, and at @superhuman we diverged from this library a while ago, so I'm curious if this is still useful now that the standard library has better wrapping/unwrapping and stack-trace support.

[titouanfreville]

Hello, I just opened a PR to fix it. :)

We did not found the support for stack-trace under standard library so we switched to this lib. I did not found any indication on standard library being able to log the caller trace without a panic and we use those stacks in all errors wright now.
Though it does have an impact on perf, it still helps a lot :)

Though if standard library know allow stack-trace, i'll take a new look on it.

[ConradIrwin]

Ah, it looks like they didn't ship that part of the proposal, my mistake! golang/go#29934 (comment) (it would be great if they did :D — https://go.googlesource.com/proposal/+/master/design/29934-error-values.md#formatting)

[titouanfreville]

Interesting. I still found this in official documentation today:
https://godoc.org/github.com/pkg/errors#hdr-Retrieving_the_stack_trace_of_an_error_or_wrapper
Will have a look

[ConradIrwin]

FWIW That's the sneakily named github.com/pkg/errors, which is not the same as the builtin errors module.

[titouanfreville]

👍 I'll stay with go-errors for now then :)