Repository navigation
feat : add consistent exit codes - #3130
vikashkumar2020 wants to merge 2 commits into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
@vikashkumar2020 thanks for the PR! |
added into description |
divyansh42
left a comment
There was a problem hiding this comment.
The PR wraps errors in pkg/actions (get, list, delete, patch) but many commands produce errors through other paths: direct client calls in trigger/bundle commands, flag validation errors in RunE, etc. The issue asks for consistent exit codes "across all commands."
This is acceptable as a first pass, but please file a follow-up issue tracking the remaining commands so this doesn't fall through the cracks.
There was a problem hiding this comment.
Pull request overview
Implements a shared exit-code scheme for tkn by introducing a typed error that carries an intended exit code, classifying certain Kubernetes API failures at the pkg/actions boundary, and wiring the top-level CLI entrypoint to exit using the resolved code (with documentation for consumers).
Changes:
- Added
pkg/exitcodewith standard codes (0–5), a typedexitcode.Error, and Kubernetes API error classification helpers. - Updated the top-level command execution path to silence Cobra’s default error printing and to drive
os.Exit(...)fromexitcode.CodeFrom(err), plus structured JSON error output support. - Updated selected actions helpers to wrap API errors via
exitcode.FromAPIError(...)and alignedpipelinerun logs --exit-with-errorbehavior with the new scheme.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/exitcode/exitcode.go | Defines exit codes, typed error, and K8s API error classification. |
| pkg/exitcode/exitcode_test.go | Unit tests for CodeFrom and FromAPIError. |
| pkg/cmd/root.go | Silences Cobra errors and adds PrintError to emit JSON error payloads for --output json. |
| cmd/tkn/main.go | Switches to centralized error printing + exit code resolution. |
| pkg/actions/get.go | Wraps dynamic client Get errors via exitcode.FromAPIError. |
| pkg/actions/list.go | Wraps dynamic client List errors via exitcode.FromAPIError. |
| pkg/actions/delete.go | Wraps dynamic client Delete errors via exitcode.FromAPIError. |
| pkg/actions/patch.go | Wraps dynamic client Patch errors via exitcode.FromAPIError. |
| pkg/cmd/pipelinerun/logs.go | Adjusts “no conditions” exit code to avoid conflicting with NotFound. |
| pkg/cmd/pipelinerun/logs_test.go | Updates the corresponding unit test expectation. |
| docs/exit-codes.md | Documents the exit code table, examples, and structured JSON stderr errors. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
One more minor comment: |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Suppressed comments (2)
cmd/tkn/main.go:53
PrintErroris given the root command, but--outputis a local flag on the executed leaf command (PrintFlags.AddFlagsusescmd.Flags()). The lookup therefore fails and is ignored, so eventkn pipelinerun describe missing --output jsonemits the plain-text error instead of JSON. UseExecuteCand pass its returned command; an integration test with a leaf--output jsonflag would catch this.
if err := tkn.Execute(); err != nil {
tknCmd.PrintError(tkn, err, os.Stderr)
pkg/actions/list.go:61
- The API-boundary audit is incomplete.
pkg/actions/create.go:34andpkg/actions/watch.go:35still return raw Kubernetes API errors, so commands usingactions.Createoractions.Watchreturn code 1 for forbidden or timeout responses instead of codes 5 or 4. Apply the same classification to every API operation in this package.
allRes, err := dynamic.Resource(*gvr).Namespace(ns).List(context.Background(), op)
if err != nil {
return nil, exitcode.FromAPIError(err)
|
@vikashkumar2020 is there any update on this? |
1 similar comment
|
@vikashkumar2020 is there any update on this? |
Hi, due to some complications around the contribution to open source in my org at IBM. I have to hold on making any further contribution |
Thanks, @vikashkumar2020 for informing us. I hope you come back to contribute to this project. Thank you for your contributions. 🙂 |
034007f to
b94d1b2
Compare
|
/retest |
1 similar comment
|
/retest |
b94d1b2 to
c095e71
Compare
|
/retest |
|
@copilot review |
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved error classification and propagation gaps produce incorrect exit codes, while direct stderr diagnostics and inconsistent output-mode handling break the JSON error contract.
14 open issues
Preserve typed errors through description, list, cancel, and delete paths · New Classify Cobra parsing and validation failures as InvalidInput · New Normalize output format before selecting structured error rendering · New Resolve output mode even when flag parsing fails · New Map client and server timeouts to the Timeout exit code · New Map Kubernetes Invalid and BadRequest errors to InvalidInput · New Classify untyped Kubernetes API errors before the GeneralError fallback · New Classifying at this boundary is not sufficient to make the code reachmain: several existing… Exit code 3 is currently unreachable in production: no command or validator creates an…k8serrors.IsTimeoutdoes not match KubernetesStatusReasonServerTimeout; that case has the… The PR description states that all Kubernetes API calls inpkg/actionsclassify errors via…FromAPIErrordoesn’t currently map Kubernetes validation/client errors (e.g. HTTP 400/422) to… This function prints a plain-text message directly to stderr before returning the error. With the… New behavior inPrintError(structured JSON to stderr when--output json) isn’t covered by unit…
1 resolved since last review
Review effort: Max
c095e71 to
5b41f4b
Compare
|
/retest |
There was a problem hiding this comment.
🟡 Changes recommended
Existing command paths lose classified error causes, validation failures return incorrect exit codes, and JSON error output depends on flag order.
11 open issues
Classify flag and argument validation errors as invalid input · New Map client and server timeouts to the Timeout exit code Resolve output mode even when flag parsing fails Classify Cobra parsing and validation failures as InvalidInput Preserve typed errors through description, list, cancel, and delete paths Classifying at this boundary is not sufficient to make the code reachmain: several existing… Exit code 3 is currently unreachable in production: no command or validator creates an… The PR description states that all Kubernetes API calls inpkg/actionsclassify errors via… Use the registered PipelineRun logs flag in the documentation · New Update help and generated docs for the new exit code · New New behavior inPrintError(structured JSON to stderr when--output json) isn’t covered by unit…
6 resolved since last review
Classify untyped Kubernetes API errors before the GeneralError fallback Map Kubernetes Invalid and BadRequest errors to InvalidInput Normalize output format before selecting structured error renderingk8serrors.IsTimeoutdoes not match KubernetesStatusReasonServerTimeout; that case has the…FromAPIErrordoesn’t currently map Kubernetes validation/client errors (e.g. HTTP 400/422) to… This function prints a plain-text message directly to stderr before returning the error. With the…
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Command handlers still discard classified API errors and leave input errors unclassified, producing incorrect exit codes, while JSON error output inconsistently depends on flag order.
12 open issues
Detect JSON error output when flag parsing fails · New Classify flag and argument validation errors as invalid input Map client and server timeouts to the Timeout exit code Resolve output mode even when flag parsing fails Classify Cobra parsing and validation failures as InvalidInput Preserve typed errors through description, list, cancel, and delete paths Classifying at this boundary is not sufficient to make the code reachmain: several existing… Exit code 3 is currently unreachable in production: no command or validator creates an… The PR description states that all Kubernetes API calls inpkg/actionsclassify errors via… Update help and generated docs for the new exit code Use the registered PipelineRun logs flag in the documentation New behavior inPrintError(structured JSON to stderr when--output json) isn’t covered by unit…
Review effort: Balanced
5b41f4b to
d4afff0
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved error-handling defects produce incorrect exit codes for invalid input and wrapped API failures, while malformed flags make JSON error output depend on argument order.
13 open issues
Classify flag and validation errors as invalid input · New Preserve classified errors through command wrappers · New Detect JSON error output when flag parsing fails Classify flag and argument validation errors as invalid input Map client and server timeouts to the Timeout exit code Resolve output mode even when flag parsing fails Classify Cobra parsing and validation failures as InvalidInput Preserve typed errors through description, list, cancel, and delete paths Classifying at this boundary is not sufficient to make the code reachmain: several existing… Exit code 3 is currently unreachable in production: no command or validator creates an… The PR description states that all Kubernetes API calls inpkg/actionsclassify errors via… Update help and generated docs for the new exit code New behavior inPrintError(structured JSON to stderr when--output json) isn’t covered by unit…
1 resolved since last review
Review effort: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Lost API error causes and unclassified client-side input errors produce incorrect exit codes, while JSON error output depends on flag order.
10 open issues
Preserve classified API errors in human-readable handlers · New Preserve classified errors through command wrappers Classify flag and validation errors as invalid input Detect JSON error output when flag parsing fails Classify flag and argument validation errors as invalid input Resolve output mode even when flag parsing fails Classify Cobra parsing and validation failures as InvalidInput Preserve typed errors through description, list, cancel, and delete paths Classifying at this boundary is not sufficient to make the code reachmain: several existing… Exit code 3 is currently unreachable in production: no command or validator creates an…
4 resolved since last review
Review effort: Balanced
d4afff0 to
cb85acc
Compare
Introduce a pkg/exitcode package that defines the standard tkn exit
codes (0 success, 1 general error, 2 not found, 3 invalid input,
4 timeout, 5 unauthorized) and a typed error so that errors can carry
their intended exit code through the call stack.
Classify Kubernetes API errors at the boundary in the shared actions
layer (get, list, delete, patch) via exitcode.FromAPIError, and emit
the resolved code from main using exitcode.CodeFrom. When --output
json is set, errors are written to stderr as a structured
{"error":"...","code":N} payload. Document the full scheme in
docs/exit-codes.md.
Closes tektoncd#2855
Signed-off-by: Vikash Kumar <kvvik2020@gmail.com>
Build on the exit-code support with correctness and coverage fixes:
- FromAPIError now keeps the original Kubernetes error as the wrapped
error and exposes it via Unwrap, so k8serrors helpers such as
IsNotFound/IsForbidden still match after classification. Replacing
the typed error broke callers like customrun delete that branch on
IsNotFound.
- main passes the command returned by ExecuteC (the one that actually
ran) to PrintError, and PrintError takes an io.Writer. --output is a
subcommand-local flag, so reading it from the root command meant the
structured {"error":...,"code":N} JSON output never triggered.
PrintError now normalizes the flag so "JSON"/"Json" also match.
- Classify invalid/bad-request errors as InvalidInput (3) and
timeout/server-timeout/context.DeadlineExceeded as Timeout (4) via a
shared classify helper, and let CodeFrom fall back to it for raw
Kubernetes/context errors preserved by intermediate callers.
- Wrap Create and Watch API errors through FromAPIError, and drop the
stray stderr message in Patch so failures surface only as the typed
error.
- Align the --exit-with-pipelinerun-error help text (pipelinerun logs
and pipeline start) and regenerated docs/man pages with the new
"no conditions" behavior, which now exits 1 instead of 2.
- Document that code 2 (not found) intentionally diverges from the Bash
"misuse of builtins" convention, and reference the real
--exit-with-pipelinerun-error flag name in docs/exit-codes.md.
Address review follow-ups so the documented codes actually reach the
caller and cover client-side input errors:
- Preserve the error cause with %w across the describe, list, cancel and
delete paths that previously formatted it with %v/%s or discarded it
(for example pkg/taskrun/description.go, pkg/cmd/pipeline/list.go and
pkg/deleter/deleter.go). NotFound and Forbidden now propagate through
CodeFrom regardless of output format. Where a caller must keep an exact
message without the cause, it returns exitcode.New(CodeFrom(err), ...)
so the code is retained.
- Classify client-side input errors as InvalidInput (3): a root
SetFlagErrorFunc tags flag-parsing failures, a visitCommands pass wraps
positional-argument validators, and option validators (negative
--limit and invalid --output/kind) are wrapped at their source.
- Resolve the requested --output mode independently of successful flag
parsing. Cobra stops at the first invalid flag, so PrintError now falls
back to scanning the raw arguments (honoring the -- terminator) and the
JSON error format no longer depends on flag order.
Signed-off-by: divyansh42 <diagrawa@redhat.com>
Assisted-by: Claude Opus 4.8 (via GitHub Copilot CLI)
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
cb85acc to
a2f3bfb
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Argument validation incorrectly reclassifies API errors, while output-format detection mishandles option values and defaults, causing incorrect exit codes and error formats.
3 open issues
10 resolved since last review
Preserve classified API errors in human-readable handlers Preserve classified errors through command wrappers Classify flag and validation errors as invalid input Detect JSON error output when flag parsing fails Classify flag and argument validation errors as invalid input Resolve output mode even when flag parsing fails Classify Cobra parsing and validation failures as InvalidInput Preserve typed errors through description, list, cancel, and delete paths Classifying at this boundary is not sufficient to make the code reachmain: several existing… Exit code 3 is currently unreachable in production: no command or validator creates an…
Review effort: Balanced
| if outputFlag == "" { | ||
| outputFlag = outputFromArgs(os.Args[1:]) | ||
| } |
| case strings.HasPrefix(a, "-o") && len(a) > 2: | ||
| return a[2:] |
| if err := orig(c, args); err != nil { | ||
| return exitcode.New(exitcode.InvalidInput, "%s", err) |


Changes
Fixes : #2855
Implemented consistent exit codes across tkn. Added a new pkg/exitcode package defining codes 0–5 (success, general error, not found, invalid input, timeout, unauthorized) with a typed error that propagates through the call stack. All Kubernetes API calls in pkg/actions now classify errors at the boundary via exitcode.FromAPIError(). The root command wires up exitcode.CodeFrom() to drive os.Exit, and adds structured {"error":"…","code":N} JSON output to stderr when --output json is set. A reference doc at docs/exit-codes.md covers the full scheme with examples.
Submitter Checklist
These are the criteria that every PR should meet, please check them off as you
review them:
make checkmake generatedSee the contribution guide
for more details.
Release Notes