Repository navigation
fix: report non-JSON error responses by their HTTP status - #1350
Merged
Merged
Conversation
An error response whose body is not JSON, such as a proxy's HTML page or an empty 401 or 404, was reported by the JSON decoder's complaint alone, e.g. "invalid character '<' looking for beginning of value". The status was carried on the APIError but never reached the message, so the user could not tell a firewall block from a missing resource. The message is now the status, such as "HTTP 403 Forbidden". When the body is not empty it adds that the response is not JSON and, unless --debug is already on, that rerunning with --debug shows it, since the body is logged at debug level only.
Contributor
|
Claude finished @mbevc1's task in 32s —— View job Follow-up review complete
Both previous findings are resolved in commit bd6e126:
No new bugs/regressions found in the changed lines. Summary posted: #1350 (comment) |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Some proxies answer a 401 or 404 with a lone newline. That body counted as non-empty, so the user was told to rerun with --debug to see it, and --debug then showed nothing. The body is now trimmed before the check. Also renames the test case for an unrouted GET, which gets an empty 404 rather than the HTML body its old name described.
Contributor
Follow-up review of bd6e126 (whitespace-only body fix)Both previous findings are resolved, confirmed on the new commit:
No new bugs/regressions found in the changed lines. 0 new inline comments beyond closing the two resolved threads above. |
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
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.
An error response whose body is not JSON, such as a proxy's or WAF's HTML page or an empty 401 or 404, is reported only by the JSON decoder's complaint:
The status is carried on the
APIErrorbut never reaches the message, so a user cannot tell a firewall block from a missing resource. The message is now the status, with a pointer to--debugwhen there is a body to see:HTTP 404 Not Found.--debugalready on, the hint is dropped, since the body is logged just above.APIError.HasServerMessagestays false, sokosli evaluate, which reports only the status for such responses, is unchanged.Stacked on #1349, which adds the
httpStatushelper this reuses. The base will move tomainonce #1349 merges.Checklist
charts/k8s-reporter/) updated, if needed. Note: these changes live in a separate PR (not needed)