Skip to content

fix: restore Unwrap on the error returned by Exit - #2444

Open
fadiroot wants to merge 1 commit into
urfave:mainfrom
fadiroot:fix/exit-error-unwrap
Open

fadiroot wants to merge 1 commit into
urfave:mainfrom
fadiroot:fix/exit-error-unwrap

Conversation

@fadiroot

Copy link
Copy Markdown

What type of PR is this?

  • bug

What this PR does / why we need it:

Exit() wraps the caller's error in an unexported exitError. Since #1545 (issue #1090) that wrapper had an Unwrap() method so errors.Is / errors.As could reach the original error through the ExitCoder. The method was dropped in 05071ea ("Add error tests") without being mentioned in the commit, so on v3 today:

var ErrNotFound = errors.New("not found")
err := cli.Exit(ErrNotFound, 2)
errors.Is(err, ErrNotFound) // false on main; true on v2 and on v3 before Oct 2024

origin/v2-maint still has the method.

  • errors.go: add func (ee *exitError) Unwrap() error back.
  • errors_test.go: TestExit_Unwrap checks errors.Is, errors.Unwrap, errors.As to ExitCoder, and the non-error message form. It fails on main and passes with this change.

No exported API changes (exitError is unexported), so godoc-current.txt is unaffected.

Release Notes

Restore `Unwrap()` on the error returned by `Exit()`, so `errors.Is` and `errors.As` see the wrapped error again.

Exit() wraps the caller's error in an exitError, and since urfave#1545 that
wrapper had an Unwrap method so errors.Is and errors.As could see the
original error through the ExitCoder. The method was dropped in 05071ea
("Add error tests") with no mention in the commit, so on v3
errors.Is(cli.Exit(ErrNotFound, 2), ErrNotFound) is false where it was
true on v2 and on earlier v3.

Add the method back and cover it with a test.
@fadiroot
fadiroot requested a review from a team as a code owner September 24, 2026 12:52

This branch has not been deployed

No deployments
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.

2 participants