Safely implementing Is(target error) bool

[Diego Augusto Molina]

Hello everyone. In relation to standard errors package, I wanted to ask about safe ways of implementing Is(target error) bool. The current documentation (https://cs.opensource.google/go/go/+/refs/tags/go1.20.6:src/errors/wrap.go;l=35) says the following:

// An error type might provide an Is method so it can be treated as equivalent

// to an existing error. For example, if MyError defines

//

// func (m MyError) Is(target error) bool { return target == fs.ErrExist }

This seems wrong and actually leads users in the wrong direction, as if someone implements an error type like "type MyErrs []error" then it's sufficient to make that implementation panic, so the documentation is leading users to create panic-prone code. While it's true that interfaces are comparable, concrete types implementing them may not, and the error will happen at runtime.

Note that the code for the Is function in errors package actually uses reflectlite.TypeOf(target).Comparable() to avoid these panics, but that as users do not have access to reflectlite and, even if they do, the documentation is not warning against these possible panics, then it's very easy to make mistakes. Of course, nil comparisons are ok, because the comparison is of the value of the interface, not the value of the concrete type.

I propose: consider changing the documentation to warn users against this possible problem when implementing the Is method of their own error types.

My question to the community: how do you currently go about this issue.

Thank you.

[Diego Augusto Molina]

I apologize, I think the actual issue is with the implementation I found. Let me explain what happened  to clarify:

<code>

var sentinelErr = someInternalLib.NewError("something happened. The implementation of this contains a slice")

type SomeInnocentErr struct {}

func (e SomeInnocentErr) Error() string { return "wrapped: " + e.Err.Error() }
func (e SomeInnocentErr) Is(target error) bool { return sentinelErr == target }
</code>

If the above sentinelErr had been defined with the standard errors.New then there had been no problem, or at least instead of doing "return sentinelErr == target" it had done something like "errors.Is(target, sentinelErr)".

This is evidently an internal issue, not with the standard library, sorry about the fuzz.