-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
Transition away from archived package github.com/pkg/errors
#4468
Comments
github.com/pkg/errors
to errors
github.com/pkg/errors
Dunno how authoritative this answer is but it seemed to make sense - basically use I'd lean towards approach 1, but at the same time I remember how happy I've been to have stack trace data... |
I prefer the first option. Would expect that the official lib will keep making good progress. |
Shameless plug: you can also switch it to drop-in replacement |
The drop-in replacement gitlab.com/tozd/go/errors does sound enticing. That would be my preference unless somebody volunteers to lead the overall error re-design (Approach 1 above). What do you think @yeasy @ale-linux ? |
It's surely tempting, why not. We just have to be aware that we're just kicking the can down the road, nothing else. gitlab.com/tozd/go/errors is supported until it isn't, at which point we'll meet back here 😄 |
Hopefully then stacktrace support will be in the Go standard library and we will all have easier time. :-) |
fair enough, tozd it is. anyone objects? |
IMO, need to make some MVP to ensure it's indeed drop-in replacement and we won't encounter compatibility issues. |
The gitlab.com/tozd/go/errors looks not widely adopted (only 4 stars) and quite young (about 2 years). I would prefer the standard errors. Otherwise, need to do some careful security/bug scan first. |
Any conclusion yet? |
@yeasy I think the reality is that we need to go with the simpler drop-in replacement gitlab.com/tozd/go/errors unless somebody volunteers to lead the overall error re-design (Approach 1 above). |
Let me do further investigation, and see if we can take approach 1 then. We do not need to finish the change before v3.0, but we can target 3.x. For Go v2, here's a new errors lib: https://pkg.go.dev/github.com/go-errors/errors |
This one is promising: https://github.com/go-errors/errors. |
Current Status
The Fabric project intentionally adopted use of
github.com/pkg/errors
package for error handling and publicized the guidance at:https://hyperledger-fabric.readthedocs.io/en/latest/error-handling.html
The rationale at the time was that
github.com/pkg/errors
injected stack trace context at the point of error in low level code, which could be easily logged at higher levels of the control flow with the%+v
formatter verb, and layers of code inbetween could add message context using WithMessage(), or simply pass the existing stack trace context up with the familiar and simple error handling like this:Since that time, Go added better error wrapping support in
errors
standard library as of Go 1.13 as described in the blog and wiki, andgithub.com/pkg/errors
got archived as not needed anymore.Goal
The project should transition away from the archived package
github.com/pkg/errors
.I see two basic approaches:
Approach 1 - Switch to
errors
package in standard libraryIt makes logical sense to use the standard
errors
package now, however without the stack trace context available anymore, each layer that passes an error up should add its own context, either by extending the error message or using the new wrappering approach available in standard libraryerrors
package.The downside is that transitioning to the standard library
errors
package will require a large work effort to update all the code, as we really don't want a mismatch of error handling assumptions and techniques across a single code flow. The best time to make an exhaustive change like this would be at a major release boundary, for example between v2.x and v3.x.Approach 2 - Keep overall approach by copying
github.com/pkg/errors
into a fabricerrors
packageIf we want to preserve the stack trace and limit the code changes, there isn't much risk of keeping the overall
github.com/pkg/errors
approach. Although the project is archived, the project was simple and didn't pull in other dependencies. The utility code could easily be copied into a fabric specificerrors
package to eliminate the external dependency ongithub.com/pkg/errors
, while keeping the existing overall approach and the associated benefits of having stack trace available.This approach would be simpler in the short term, but would come at the cost of maintaining the (small amount) of utility code, and educating developers to use it.
The purpose of this issue is to finalize an approach and then update the developer guidance and code accordingly.
The text was updated successfully, but these errors were encountered: