From 4e6c761abd096230ec6d63ecd856c25db3abe6d3 Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Sat, 26 Sep 2026 01:56:31 -0600 Subject: [PATCH 1/2] fix: raise GitHubDocsError for a read timeout or a non-JSON 2xx body _request caught HTTPError and URLError only, so a timeout raised by resp.read() and a JSONDecodeError on a 2xx body escaped as themselves, breaking the README's promise that everything which stops an edit landing is a GitHubDocsError. Both are now converted, through _redact, with the body never quoted back. Co-Authored-By: Claude Opus 5.5 --- python/src/github_docs/client.py | 18 +++++++++++++++++- python/tests/test_client.py | 22 ++++++++++++++++++++++ 2 files changed, 39 insertions(+), 1 deletion(-) diff --git a/python/src/github_docs/client.py b/python/src/github_docs/client.py index 1210218..2b1c203 100644 --- a/python/src/github_docs/client.py +++ b/python/src/github_docs/client.py @@ -236,7 +236,17 @@ def _request( try: with urllib.request.urlopen(req, timeout=self.config.timeout) as resp: raw = resp.read() - parsed = json.loads(raw) if raw else {} + try: + parsed = json.loads(raw) if raw else {} + except json.JSONDecodeError as e: + # Something in front of GitHub answered 2xx with a page + # rather than JSON. The body is not quoted back, for the + # same reason as on the error path: the status is the + # actionable part. + raise GitHubDocsError( + self._redact(f"GitHub API returned an undecodable response (HTTP {resp.status})"), + status=resp.status, + ) from e return resp.status, parsed except urllib.error.HTTPError as e: raw = e.read() @@ -252,6 +262,12 @@ def _request( ) from e except urllib.error.URLError as e: raise GitHubDocsError(self._redact(f"could not reach GitHub: {e.reason}")) from e + except OSError as e: + # urlopen wraps a failure to connect in URLError, but a timeout or a + # reset while the body is being read arrives bare. Both mean the + # edit did not land, and the README promises a caller that every + # such failure is a GitHubDocsError. + raise GitHubDocsError(self._redact(f"could not reach GitHub: {e}")) from e # -- Paths -------------------------------------------------------------- diff --git a/python/tests/test_client.py b/python/tests/test_client.py index 893ec23..171978c 100644 --- a/python/tests/test_client.py +++ b/python/tests/test_client.py @@ -9,6 +9,7 @@ import base64 import json +import socket import unittest import urllib.error import urllib.parse @@ -536,6 +537,27 @@ def test_a_network_failure_becomes_a_readable_error(self): self.assertIn("could not reach GitHub", str(ctx.exception)) self.assertIsNone(ctx.exception.status) + def test_a_timeout_while_reading_the_body_becomes_a_readable_error(self): + # urlopen only wraps a connect timeout in URLError; one raised by + # read() arrives as a bare socket.timeout. + resp = _response({}) + resp.read.side_effect = socket.timeout("timed out") + with mock.patch("urllib.request.urlopen", return_value=resp): + with self.assertRaises(GitHubDocsError) as ctx: + self.client.get_default_branch() + self.assertIn("could not reach GitHub", str(ctx.exception)) + self.assertIsNone(ctx.exception.status) + + def test_a_success_body_that_is_not_json_becomes_an_error_with_its_status(self): + # The error path already tolerates a non-JSON body; the success path + # must too, without quoting the body back. + resp = _response(None, raw=b"captive portal") + with mock.patch("urllib.request.urlopen", return_value=resp): + with self.assertRaises(GitHubDocsError) as ctx: + self.client.get_default_branch() + self.assertEqual(ctx.exception.status, 200) + self.assertNotIn("captive portal", str(ctx.exception)) + if __name__ == "__main__": unittest.main() From 31d40afbfdf87f739a249020828ebd9bd9128af1 Mon Sep 17 00:00:00 2001 From: Daniel McCoy Stephenson Date: Sat, 26 Sep 2026 01:57:47 -0600 Subject: [PATCH 2/2] fix: also convert a non-UTF-8 2xx body into GitHubDocsError json.loads on bytes that are not UTF-8 raises UnicodeDecodeError, which is not a JSONDecodeError, so it still escaped. Catch ValueError, which covers both. Co-Authored-By: Claude Opus 5.5 --- python/src/github_docs/client.py | 10 ++++++---- python/tests/test_client.py | 8 ++++++++ 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/python/src/github_docs/client.py b/python/src/github_docs/client.py index 2b1c203..0c54869 100644 --- a/python/src/github_docs/client.py +++ b/python/src/github_docs/client.py @@ -238,11 +238,13 @@ def _request( raw = resp.read() try: parsed = json.loads(raw) if raw else {} - except json.JSONDecodeError as e: + except ValueError as e: # Something in front of GitHub answered 2xx with a page - # rather than JSON. The body is not quoted back, for the - # same reason as on the error path: the status is the - # actionable part. + # rather than JSON. ValueError rather than JSONDecodeError, + # because bytes that are not UTF-8 fail as a + # UnicodeDecodeError before they ever reach the parser. The + # body is not quoted back, for the same reason as on the + # error path: the status is the actionable part. raise GitHubDocsError( self._redact(f"GitHub API returned an undecodable response (HTTP {resp.status})"), status=resp.status, diff --git a/python/tests/test_client.py b/python/tests/test_client.py index 171978c..3b71449 100644 --- a/python/tests/test_client.py +++ b/python/tests/test_client.py @@ -558,6 +558,14 @@ def test_a_success_body_that_is_not_json_becomes_an_error_with_its_status(self): self.assertEqual(ctx.exception.status, 200) self.assertNotIn("captive portal", str(ctx.exception)) + def test_a_success_body_that_is_not_utf8_becomes_an_error_with_its_status(self): + # Undecodable bytes fail before the JSON parser, as a UnicodeDecodeError. + resp = _response(None, raw=b"\x80\x81 not text") + with mock.patch("urllib.request.urlopen", return_value=resp): + with self.assertRaises(GitHubDocsError) as ctx: + self.client.get_default_branch() + self.assertEqual(ctx.exception.status, 200) + if __name__ == "__main__": unittest.main()