errors icon indicating copy to clipboard operation
errors copied to clipboard

fix bug: Unwrap for withStack should first try to unwrap withMessage

Open colin404 opened this issue 4 years ago • 1 comments

colin404 avatar Jun 19 '20 09:06 colin404

-1 this bug fix introduces more bugs than it actually resolves, and does not actually perform the action that the title of this PR even declares it to do.

Given:

  err := errors.WithStack(&net.OpError{
    Err: io.EOF,
  })

  err.Unwrap() == io.EOF // this is true

To obtain what the PR title says it should, the interface type assertion should be a type assertion to to the concrete *withMessage type. But this still still introduce more bugs.

Given:

func f() error {
 return WithMessage(io.EOF, "extra context")
}
func g() error {
 return WithStack(f())
}

I would expect the err.Unwrap() from the error returned by g() to return the error returned by f(), but it would not, it would return io.EOF. While this trivial example seems silly, the wrapping with the message and the stack could potentially happen significantly far apart from each other in code, and one would be left confused about why the WithMessage was also unwrapped.

The expected behavior that this PR is trying to solve is errors.Wrap(err) == err should be true. But errors.WithStack(err) == err and errors.WithMessage(err) == err should also universally be true.

In order to get the critical desired behavior errors.Wrap(err) == err, we would want to create a new withStackAndMessage error that does both behaviors, and properly unwraps as expected.

puellanivis avatar Jun 23 '20 11:06 puellanivis