fix(cli): prevent handled errors from producing duplicate output [CLI-1765] - #7130
Draft
bdemeo12 wants to merge 2 commits into
Draft
fix(cli): prevent handled errors from producing duplicate output [CLI-1765]#7130bdemeo12 wants to merge 2 commits into
bdemeo12 wants to merge 2 commits into
Conversation
…-1765] `processError` combines the command exit error with collected network errors via `errors.Join`, both in `FindMostRelevantError` and `createErrorWithExitCode`. A joined error does not satisfy the direct type assertions in `displayError`, so handled auxiliary errors were printed after valid command output - emitting a second JSON object and breaking `JSON.parse(stdout)`. Decide displayability at the top of `processError`, while the concrete error type is still intact, and pass that decision through to teardown. The combined error is still used for exit-code derivation and analytics, so both are unchanged. Also pins snyk-docker-plugin to 9.20.0 so CI exercises the failure this fixes: 9.20.0 performs a best-effort provenance fetch whose handled failure is what triggered the duplicate output (see CLIA-1576). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Contributor
|
FindMostRelevantError returns promoted errors (e.g. maintenance windows) uncombined, so judge those on their own merits rather than on the original error. Without this, a maintenance notice was suppressed whenever the command also exited non-zero. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
Author
|
This PR originates from: #7047 |
This was referenced Aug 13, 2026
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.
Pull Request Submission Checklist
are release-note ready, emphasizing
what was changed, not how.
What does this PR do?
Fixes:
json output being unparseable because an extra error was printed after the result. This happens when the CLI receives an error (aside from exit status 1), it prints it — so once teardown joins the exit error together with the handled network errors, the result is no longer exit status 1 and gets printed after the real output.
Also pins
snyk-docker-pluginto9.20.0so CI exercises the failure being fixed. (Original PR: #7047 + discussion: https://snyksec.atlassian.net/servicedesk/customer/portal/64/CLIA-1576)Where should the reviewer start?
cliv2/pkg/core/main.go—shouldSuppressDisplayand the third return value fromprocessError, then the guardeddisplayErrorcall intearDown.Then
cliv2/pkg/core/main_test.gofor the two new cases inTest_processErrorand the newTest_shouldSuppressDisplay.How should this be manually tested?
Against an image whose provenance fetch fails (SDP version 9.20.0 +):
The
container.spec.tsacceptance job is the automated equivalent — it fails on 9.20.0 without this fix, which is why the pin is included here.What's the product update that needs to be communicated to CLI users?
n/a