Skip to content

feat: support %w verb in Errorf for error wrapping - #254

Open
AdamMagued wants to merge 1 commit into
pkg:masterfrom
AdamMagued:fix/issue-244
Open

AdamMagued wants to merge 1 commit into
pkg:masterfrom
AdamMagued:fix/issue-244

Conversation

@AdamMagued

Copy link
Copy Markdown

Fixes #244

Solution

  • Add %w verb support to Errorf allowing callers to wrap errors compatible with standard errors.Unwrap and errors.Is/As.
  • Add comprehensive unit test coverage for single and multi-error wrapping, unwrapping, causes, stack traces, and formatting verbs.

Verified with container sandbox test suite.

Signed-off-by: AdamMagued <adamismailmageud@gmail.com>
@davecheney

Copy link
Copy Markdown
Member

Why not just make fundamental implement Unwrap?

@AdamMagued

Copy link
Copy Markdown
Author

There are a couple of architectural considerations around fundamental and Unwrap:

  1. Leaf errors vs. wrapped errors:
    fundamental represents a leaf error created by New(msg) containing only msg string and *stack. It holds no underlying error. If fundamental were given an Unwrap() error { return nil } method, it would be a no-op for chain traversal, as standard library errors.Is and errors.As already treat types without Unwrap (or returning nil) as terminal leaves.

  2. Storing the wrapped error:
    To support Errorf("%w", err), the wrapped error operand must be retained in memory so Unwrap() can return it. If fundamental were expanded to include an err error field, every leaf error returned by New would incur an unused error field (16 bytes on 64-bit architectures).

  3. Go 1.20 multi-error wrapping:
    Standard library fmt.Errorf supports multiple %w verbs (e.g., fmt.Errorf("%w %w", err1, err2)), returning a type that implements Unwrap() []error. In Go, a single struct cannot implement both Unwrap() error and Unwrap() []error due to conflicting method return signatures.
    Following the standard library pattern (fmt.wrapError and fmt.wrapErrors), separate types allow Errorf to return:

    • wrapError (Unwrap() error) for single %w
    • wrapErrors (Unwrap() []error) for multiple %w
    • fundamental for plain format strings without %w

That said, if you prefer to keep pkg/errors focused strictly on single-error wrapping and would rather unify single-wrap Errorf into fundamental (adding err error and Unwrap() error directly to fundamental), I am glad to adjust the implementation to match your preferred design.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Errorf should be able to wrap errors

2 participants