Skip to content

errors: Error drops the cause chain (no Unwrap, GetErrCode uses a bare type assertion) #123

Description

@Prabhjot-Sethi

Problem

errors.Error discards the underlying cause. Two related gaps:

1. No Unwrap(). Wrap/Wrapf flatten the cause to a string:

func Wrap(code ErrCode, msg string) error {
	return &Error{code: code, msg: msg}
}

Call sites do errors.Wrap(errors.NotFound, err.Error()), so the original error is gone. errors.Is(err, context.DeadlineExceeded), errors.As(err, &netErr), and mongo.IsNetworkError(err) all stop working the moment an error crosses into this package.

2. GetErrCode uses a bare type assertion, not errors.As:

func GetErrCode(err error) ErrCode {
	val, ok := err.(*Error)
	if ok {
		return ErrCode(val.code)
	}
	return Unknown
}

So a *Error wrapped by anything else — fmt.Errorf("%w", ...) included — reports Unknown. Codes only survive while the *Error is the outermost error.

Why it matters more now

#121 adds errors.Unavailable and makes interpretMongoError classify transient MongoDB failures into it. That makes correct one-shot classification load-bearing: if isTransientMongoError misjudges a given driver error, no downstream caller can recover the original to re-check it, because the cause has already been flattened to a string. Before #121 the codes were coarse enough that this rarely bit; afterwards, callers are explicitly encouraged to branch on IsUnavailable vs IsNotFound.

Proposal

func (e *Error) Unwrap() error { return e.cause }

// WrapErr preserves the cause chain alongside the code.
func WrapErr(code ErrCode, err error, msg string) error {
	return &Error{code: code, msg: msg, cause: err}
}

and switch GetErrCode to errors.As:

func GetErrCode(err error) ErrCode {
	var e *Error
	if errors.As(err, &e) {
		return ErrCode(e.code)
	}
	return Unknown
}

Existing Wrap/Wrapf stay as-is for source compatibility (cause simply nil), so this is additive. Migrating call sites to WrapErr can happen incrementally.

Notes

Deliberately scoped out of #121 — this touches every Wrap call site in the repo and has its own blast radius. Filing separately so it can be evaluated on its own.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions