pkg / errors

Simple error handling primitives
https://godoc.org/github.com/pkg/errors
BSD 2-Clause "Simplified" License
8.18k stars 691 forks source link

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

Open colin404 opened 4 years ago

puellanivis commented 4 years ago

-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.