Skip to content
This repository has been archived by the owner on Dec 1, 2021. It is now read-only.

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

colin404
Copy link

No description provided.

@puellanivis
Copy link

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants