gomega icon indicating copy to clipboard operation
gomega copied to clipboard

wrong MatchError behavior when the error has an Is function

Open marten-seemann opened this issue 4 years ago • 2 comments

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: https://github.com/onsi/gomega/blob/2f04e6e3467d2c0c695786c3e7880f1e940274cf/matchers/match_error_matcher.go#L27-L29

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 avatar Jun 23 '21 01:06 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?

marten-seemann avatar Jun 23 '21 01:06 marten-seemann

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?

onsi avatar Jun 23 '21 03:06 onsi