fix(cmd): exit non-zero when a command handler returns an error (#4255) - #4258
Open
NitinKumar004 wants to merge 2 commits into
Open
NitinKumar004 wants to merge 2 commits into
NitinKumar004 wants to merge 2 commits into
Conversation
…-dev#4255) CMD apps exited 0 even when the sub-command handler (or command resolution) returned an error, so shells and CI could not detect the failure. Responder now records whether it responded with an error; cmd.Run propagates that, and runCMD exits with a non-zero status after telemetry is flushed and the logger is closed, so a failed command still reports its final metrics/traces. os.Exit is wrapped in a testable seam.
The non-zero exit was terminating in-process tests that invoke Run (including apps' own main() tests, e.g. examples/sample-cmd). Guard the exit with testing.Testing() so real CLI binaries still exit non-zero while go test runs are not killed. Extract the telemetry flush so its deferred cancel runs before the exit, and cover cmd.Run's error/success/unknown-command return value.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description:
Fixes #4255.
CMD apps built with GoFr exited
0even when the sub-command handler (or commandresolution) returned an error, so shells, pipelines, and CI could not detect the
failure.
Changes:
Respondernow records whether it responded with a non-nil error (Errored()).cmd.Runpropagates that up to the caller.runCMDexits with a non-zero status after telemetry is flushed and the loggeris closed, so a failed command still reports its final metrics/traces before the
process ends.
os.Exitis wrapped in a small testable seam (osExit) so the exitpath can be unit-tested without terminating the test binary — this also addresses
the existing TODO in
responder.go.Behavior (verified end-to-end with a real CMD app):
011Breaking Changes (if applicable):
None to the public API. The only behavioral change is the intended one: a CMD app now
exits non-zero when a command fails (previously it always exited
0).Additional Information:
No new dependencies.
cmd.Run's unexported signature changed to return abool(internal only).
Checklist:
goimportandgolangci-lint.