decide on the future use of errors, pkg/errors and other errors

[thaJeztah]

relates to:

• daemon/containerd: Replace pkg/errors with std errors #46353
• api: Endpoints can now return a tree of errors #46099
• Add a temporary drop-in replacement for errors.Join #46188,
• multierror: make it consistent when there's a single error #46275

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")

• 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

• the package has been archived by the author (but has been stable, so that alone should not necessarily have to be a concern)
• it allows for traces to be obtained from errors (although I'm not sure if that's always provided); this allows us to do things like api: add DOCKER_DEBUG_TRACE to return stack-traces in API error responses #43252, which COULD be quite useful. Is something similar possible with stdlib errors? (to be looked into)
• similar / related, I know @tonistiigi was able to get a full trace of errors as part of OTEL traces. This was possible even up to containerd internals. The same was not possible with stdlib errors, and containerd removed uses of 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:

• 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)
• if that decision leads us to using 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)

[corhere]

stdlib errors multi-errors

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 only errors.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.

[thaJeztah]

stdlib vs. pkg/errors is a false dichotomy. They're all just different implementations of the error interface.

Yeah, that's a "for better words"; I guess the overall meaning is clear though! 🤗

Several drop-in replacements for / forks of github.com/pkg/errors already exist,

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.