fix: fall back to the status when an HTTP error body cannot be read - #15
Merged
Merged
Conversation
Both tests fail against the current handler: a socket.timeout from e.read() escapes unconverted, and non-UTF-8 bytes raise a UnicodeDecodeError that the JSONDecodeError clause does not catch. Pushed ahead of the fix so CI records the failure. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
e.read() ran inside the HTTPError handler, where the sibling OSError clause cannot catch what it raises, so a timeout or reset while reading the error body escaped save_file unconverted. The same handler caught only JSONDecodeError, so a non-UTF-8 body escaped as UnicodeDecodeError. Both now fall back to the existing status-only message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Self-review rubric (anchored on CI run 36686820165 at head
Observations outside the diff:
This review comment was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener). drafted by Claude on behalf of Daniel Stephenson |
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.
Summary
GitHubDocsClient._requestread the HTTP error body withe.read()inside theexcept urllib.error.HTTPErrorhandler. An exception raised inside anexceptblock is not routed to that block's sibling clauses, so asocket.timeout/OSErrorduring that read escapedsave_fileunconverted, contradicting the README's "Everything that stopped an edit from landing raisesGitHubDocsError". The read is now wrapped so that such a failure falls back to an empty body, which yields the existing status-only messageGitHub API returned HTTP <code>with.statusset.json.JSONDecodeError, but bytes that are not UTF-8 fail earlier as aUnicodeDecodeError(aValueError, not aJSONDecodeError), so a gateway answering an error with a non-UTF-8 body escaped the same way. The clause is widened toValueError, matching the success-path handling added in fix: raise GitHubDocsError for a read timeout or a non-JSON 2xx body #13. This second escape has no tracking issue; it was found while implementing A timeout while reading an HTTP error body escapes save_file as something other than GitHubDocsError #14 and sits in the same four lines, under the same README promise.TestErrorSurfacecases were added, one per escape.Half touched:
python/only.Docs:
python/README.md's Errors section already states the behavior this fix restores; no README change was needed.Regression evidence
Local test execution was unavailable in the dispatch sandbox (only Python 3.8 is present, below the 3.9 floor, and running the suite was not permitted), so the local anchor is UNVERIFIED and CI served as the anchor. The tests were pushed as a separate commit ahead of the fix:
88956c9(tests only): CI red on everypythonleg, with exactlytest_a_timeout_while_reading_an_error_body_falls_back_to_the_statusandtest_an_error_body_that_is_not_utf8_falls_back_to_the_statuserroring (Ran 51 tests … FAILED (errors=2)on 3.9).158b961(fix): see the CI result on the PR head.Test plan
_redact; no non-stdlib import;TOKENsentinel unchangedDeferred issues
Closes #14
This PR description was drafted during a Gardener session (https://github.com/Stephenson-Software/gardener).
🤖 Generated with Claude Code
drafted by Claude on behalf of Daniel Stephenson