-
Notifications
You must be signed in to change notification settings - Fork 100
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider wrapping returned errors with stack #94
Comments
Thanks for opening this Daniel, my thoughts on this:
Would implenting a Cause() function on our internal Error type yield benefits? // Cause returns the underlying cause of the error, if possible.
// An error value has a cause if it implements the following
// interface:
//
// type causer interface {
// Cause() error
// } |
That's wise. Package
I haven't used it, either. It also only imports from stdlib. I do see that it is (publicly) used far less than
Yes, I'm looking forward to that. Russ Cox has said that they want to include the error values proposal in go1.13, but that's still some 8+ months away.
I was specifically referring to Related to that, I think your suggestion of implementing
Needs more investigation, but I think impact would be very minimal. Any string comparison users may be doing on returned errors would not change (unless they are using the As there are no public Since no custom types implementing Edit: All that said, the last couple of bugs I've addressed have involved me wrapping these returned errors with errors.WithStack (locally) just to help me debug the problem, and understand the code path that resulted in the error. It has been quite helpful. It would be nice to have to do that each time I find it might be useful. |
Hah, I didn't realize that no public repos were using errorx. It's relatively new, but that's still not a great sign. Then again, after several years enmime only has a handful... 🤷♂️ Given that adding/formatting stack traces would be quite a bit of copying, I'm OK with adopting pkg/errors. With the understanding that it will be removed >=6 months after Go 2 error inspection make it into a release build. At that point I'll probably look at refactoring our Error type as well. |
I was pretty surprised to find that, too.
Well, I'd bet receiving email with golang is a bit more of a niche thing. At least more so than sending.
Sounds good!
That sounds good, too. There might be a point when I can take a look at that. Probably not soon, though. |
When bugs are encountered, it's easier to track them down if there's a stack trace that goes with the error.
To that end, I know I would find it useful to wrap returned errors with errors.WithStack.
As you can see from the two examples there, normal usage of the wrapped error will change nothing. But using the
%+v
format specifier will produce a useful stack trace.Already when I'm debugging this library I sprinkle
errors.WithStack
in place to help me see what's going on, but I haven't (yet) included those changes in a PR. I think it would be a useful change for contributors to be able to use these stack traces to help track down bugs.The text was updated successfully, but these errors were encountered: