fabric
fabric copied to clipboard
hyperledger
Transition away from archived package `github.com/pkg/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:
if err != nil {
return err
}
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, and github.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 library
It 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 library errors 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 fabric errors package
If 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 specific errors package to eliminate the external dependency on github.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.
Dunno how authoritative this answer is but it seemed to make sense - basically use github.com/pkg/errors.Errorf if one wants stack trace data, otherwise use fmt.Errorf (don't use github.com/pkg/errors.Wrapf anymore).
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 gitlab.com/tozd/go/errors. It fixes many issues, is maintained, and supports modern Go's error patterns (sentinel errors, %w formatting, joined errors, etc.). It also provides some nice utility functions and structured details so that it is easy to extract dynamic data out of errors (instead of trying to get them out of formatted strings). Has improved error formatting and JSON marshaling of errors. It is interoperable with other errors packages and does not require a dependency on it to extract data (e.g., stack trace) from errors.
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 :smile:
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.
@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