You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
We need to decide which direction we want to go with errors, as there were multiple PRs / discussions related to that. On one hand, it'd be great if we could just depend on stdlib; it's what the language provides, and what most packages we use will be using.
On the other hand, stdlib errors may be limiting in some areas, and we must look if we can live with that 😄
stdlib errors
stdlib now provides errors.Join, which allows for "multi-errors" (w00t!)
current versions of go also allow multiple errors to be included in an error-format string, so this is now allowed:
fmt.Errorf("foo: %w, and other errors: %w, %w", err, err1, err2")
(I must admit that I did not yet use that format, and don't know how those nested errors are unwrapped; possibly that change relates to the "multi-errors")
In short (I'll open a tracking ticket / discussion for this), we should decide:
decide / describe what we want to get out of errors? (also in light of work being done on OTEL / tracing)
do a more in-depth comparison between implementations to have better faces about what's possible (and what not) which each.
decide what implementation to use going forward; I'd be happy if that is "stdlib", but we should decide (and write down) that decision, and base it on actual needs / requirements (beyond use stdlib, not external dependencies)
A multi-error is any error which implements interface { Unwrap() []error }. errors.Join(...) and fmt.Errorf("%w %w", ...) both return errors which implement that interface. It is onlyerrors.Join which returns multi-errors that have a "not ideal" Error() string implementation. The formatted error from fmt.Errorf is, as always, the interpolated format string. If we don't like it, we have the option to write our own interoperable multi-error types which format however we want them to.
github.com/pkg/errors
No, none of the stdlib error types embed a stack trace. That is a unique feature of github.com/pkg/errors errors. The error value which carries a stack trace needs to be constructed at the innermost stack frame to be useful, so the code needs to be explicitly instrumented either way. Given that instrumenting the code with OTEL gives us a superset of the tracing powers afforded by github.com/pkg/errors, I think we should go all-in on OTEL and ditch github.com/pkg/errors.
Topics to discuss / look into
stdlib vs. pkg/errors is a false dichotomy. They're all just different implementations of the error interface.
Several drop-in replacements for / forks of github.com/pkg/errors already exist, which are interoperable with each other and pkg/errors itself. I'd leave github.com/pkg/errors alone. The only thing that needs a single canonical definition to be interoperable is the StackTrace type, which can simply be imported from pkg/errors.
Thanks for that link; I didn't go looking yet for replacements, so that's a good list to look at if we decide to "keep".
My main motivation for this discussion is to make sure we have all faces in the same direction before we either start blindly ripping out pkg/errors or the reverse. I think it's a good discussion to have so that everyone is clear what to do when working on code changes.
It may also not be an either/or, and it may depend on "context" (e.g., for packages that we may want to move "out", we may want to reduce external dependencies, and stick to stdlib, unless really compelling, whereas for other parts of the code, we may want to have more "in-depth" information).
In either case; there was a short internal thread where I brought this up (following @vvoland's PR), and at least @tonistiigi mentioned that pkg/errors was still very useful for BuildKit's case, so we can discuss this in one of our calls.
reacted with thumbs up emoji reacted with thumbs down emoji reacted with laugh emoji reacted with hooray emoji reacted with confused emoji reacted with heart emoji reacted with rocket emoji reacted with eyes emoji
-
relates to:
We need to decide which direction we want to go with errors, as there were multiple PRs / discussions related to that. On one hand, it'd be great if we could just depend on
stdlib
; it's what the language provides, and what most packages we use will be using.On the other hand,
stdlib
errors may be limiting in some areas, and we must look if we can live with that 😄stdlib errors
stdlib now provides
errors.Join
, which allows for "multi-errors" (w00t!)current versions of go also allow multiple errors to be included in an error-format string, so this is now allowed:
(I must admit that I did not yet use that format, and don't know how those nested errors are unwrapped; possibly that change relates to the "multi-errors")
however, stdlib error's formatting of multi-errors is not "ideal", which led us to PR's like api: Endpoints can now return a tree of errors #46099, Add a temporary drop-in replacement for errors.Join #46188, and multierror: make it consistent when there's a single error #46275
github.com/pkg/errors
stdlib
errors? (to be looked into)pkg/errors
, so perhaps that ship has sailed (see sandbox: replace github.com/pkg/errors with native errors containerd/containerd#6937 (comment))Topics to discuss / look into
In short (I'll open a tracking ticket / discussion for this), we should decide:
pkg/errors
, we should try to reach out to the original author to see if they're open to handing over maintenance, or "hard-fork" it, and maintain our own; see chore: remove fmt.Errorf across the codebase in preference of errors.Errorf/Wrap buildkit#3305 (comment)Beta Was this translation helpful? Give feedback.
All reactions