Stop showing JSONDecodeError in the PR comment - #540
Open
aycohen-ai wants to merge 3 commits into
Open
aycohen-ai wants to merge 3 commits into
aycohen-ai wants to merge 3 commits into
Conversation
chart-verifier reports a report/content digest mismatch as:
error executing command: digest in report did not match report content
but SHA_ERROR was spelled with a capital "Digest", so the substring check
never matched. A submission with a bad report sha fell through to
json.loads(), and the resulting JSONDecodeError repr was written to the
errors file, which build.yml renders verbatim into the PR comment.
Submitters saw this instead of a usable message:
[ERROR] loading report output: /nerror executing command: digest in
report did not match report content [ERROR] exception was:
err=JSONDecodeError('Expecting value: line 1 column 1 (char 0)'),
type(err)=<class 'json.decoder.JSONDecodeError'>
Match the phrase case-insensitively so a digest mismatch is reported on its
own terms. When the output genuinely is not JSON, keep the exception detail
on the console for maintainers and show the submitter chart-verifier's
output with a real newline rather than the literal "/n".
Also narrow the bare `except BaseException` to json.JSONDecodeError, and
decode the verifier output in one place so a non-UTF-8 response produces
the same clean message instead of an unhandled UnicodeDecodeError.
The new unit tests are the first under scripts/src/report, so the Python
Test workflow is widened to cover that directory.
Fixes openshift-helm-charts#539
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two of the comments explained things the code already says a few lines away: that SHA_ERROR is matched case-insensitively, and that the decode result is used as text below. Keep only what a reader cannot recover from the code -- that the phrase is a substring of chart-verifier's "error executing command: ..." line and is emitted lowercase, and why undecodable bytes are replaced rather than raising. The narrative belongs in the commit message, which already has it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
aycohen-ai
force-pushed
the
fix/539-suppress-exception-in-pr-comment
branch
from
September 15, 2026 10:11
2c31573 to
f03f05f
Compare
Decode chart-verifier output strictly so undecodable bytes cannot be silently repaired into a corrupted digest, and fix the missing-section branch, which crashed on dict.strip() before it could write an error. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
aycohen-ai
force-pushed
the
fix/539-suppress-exception-in-pr-comment
branch
from
September 15, 2026 10:15
f03f05f to
9f83065
Compare
Contributor
Author
|
Verified end-to-end on a sandbox running this PR's code — aycohen-ai/sandbox-2025-11#6, same bad-digest report as the openshift-helm-charts/sandbox-2025-11#19150 example: [ERROR] The submitted chart has failed certification. Reason(s): [ERROR] digest in report did not match report content |
mgoerens
added
the
ok-to-test
This was referenced Sep 22, 2026
This was referenced Sep 22, 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
2 participants
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.
chart-verifier reports a report/content digest mismatch as:
but SHA_ERROR was spelled with a capital "Digest", so the substring check never matched. A submission with a bad report sha fell through to json.loads(), and the resulting JSONDecodeError repr was written to the errors file, which build.yml renders verbatim into the PR comment.
Submitters saw this instead of a usable message:
Match the phrase case-insensitively so a digest mismatch is reported on its own terms. When the output genuinely is not JSON, keep the exception detail on the console for maintainers and show the submitter chart-verifier's output with a real newline rather than the literal "/n".
Also narrow the bare
except BaseExceptionto json.JSONDecodeError, and decode the verifier output in one place so a non-UTF-8 response produces the same clean message instead of an unhandled UnicodeDecodeError.The new unit tests are the first under scripts/src/report, so the Python Test workflow is widened to cover that directory.
Fixes #539