wrong MatchError behavior when the error has an Is function

[marten-seemann]

I have an error that (in its minimal form) looks like this:

type MyError struct {
	ErrorCode  int
}

func (e *MyError) Error() string {
	return fmt.Sprintf("Error %d", e.ErrorCode)
}

func (e *MyError) Is(target error) bool {
	_, ok := target.(*MyError)
	if ok {
		return true
	}
	return target == net.ErrClosed
}

Now I have the following test case:

It("recognizes different errors", func() {
	err1 := &qerr.MyError{
		ErrorCode:    1,
		ErrorMessage: "foo",
	}
	err2 := &qerr.MyError{
		ErrorCode:    2,
		ErrorMessage: "bar",
	}
	Expect(err1).ToNot(MatchError(err2))
})

This test fails because the second part of this if statement:

gomega/matchers/match_error_matcher.go

Lines 27 to 29 in 2f04e6e

	if isError(expected) {
	return reflect.DeepEqual(actualErr, expected) || errors.Is(actualErr, expected.(error)), nil
	}

In my understanding, Go's errors.Is is supposed to be used to one error has the same type as another error, i.e. perform

myErr, ok := err.(*MyError)

recursively, unwrapping err using errros.Unwrap until a match is found.

[marten-seemann]

Related to my earlier issue #439.

Maybe the solution is to remove the problematic second part of the if statement I describe above, and actually implement the BeAssignableToError?

[onsi]

hey @marten-seemann

you mentioned:

In my understanding, Go's errors.Is is supposed to be used to one error has the same type as another error, i.e. perform

but I don't think I'm drawing the same conclusion after reading the go docs.

They seem to be saying that if an error type defines Is then that function is called and honored when computing equivalence regardless of type. (Their example with the MyError type being equivalent to fs.ErrExist seems to make this explicit) - am I missing something?