errors
errors copied to clipboard
Fixed #102, a message formatting bug.
Fixes https://github.com/pkg/errors/issues/102
With these changes, when using a plain withMessage(), we'll print the additional message in front of the original error message, above the stack trace. This maintains the correct ordering when an error is being Wrap()'d, printing the message + second stack trace after the first message + stack trace.
Not LGTM sorry. Do we need a field on withMessage? Can't you do type assertion instead?
Essentially, there are two use cases we want to meet here.
- In the case where a
withMessageis added to the chain independently of awithStack, we want to print them separately. In this case, we print the message prepended to the entire error format, and the additional stack appended to the error format. - In the case where a
Wrap()call adds awithMessagefirst, and then immediately adds awithStack, we want to keep them together because they are related. In this case, we would want to print the entire error format, then thewithMessageand then thewithStack. This will keep the message associated with its related stack.
If you use type assertion to accomplish this, you might do the type assertion in the withStack Format() function.
func (w *withStack) Format(s fmt.State, verb rune) {
switch verb {
case 'v':
if s.Flag('+') {
withMessageWrapper, ok := w.Cause().(*withMessage) // Type Assertion
if ok { // Run this whenever we have a withMessage next in the chain
fmt.Fprintf(s, "%+v\n", withMessageWrapper.Cause())
io.WriteString(s, withMessageWrapper.msg)
w.stack.Format(s, verb)
} else { // Run this for anything else next in the chain
fmt.Fprintf(s, "%+v", w.Cause())
w.stack.Format(s, verb)
}
return
}
fallthrough
case 's':
io.WriteString(s, w.Error())
case 'q':
fmt.Fprintf(s, "%q", w.Error())
}
}
This introduces an issue that from that scope there is no way to determine whether the withStack wraps a withMessage as a part of a Wrap() call. It could be a withMessage that should be kept separate, or it could be a withMessage that should be kept with its related withStack.
The scenario can easily occur where a pre-existing withMessage is placed in the chain before being passed to a WithStack() call. In these cases, the preexisting withMessage is indistinguishable from a withMessage added by calling Wrap(), and will be assumed to be related to the earlier withStack in the chain.
Maybe that is a viable solution? Is there a scenario where a WithMessage() followed next by WithStack() wouldn't be assumed to be related to one another?
Does it help to use causer instead of Cause for the type assertion to only go one level deep?
What is the status of this PR? Does the comment above helped the discussion?