fix: raise GitHubDocsError for a read timeout or a non-JSON 2xx body

python/src/github_docs/client.py

@@ -236,7 +236,19 @@ 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 ValueError as e:
+                    # Something in front of GitHub answered 2xx with a page
+                    # 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,
+                    ) from e
                 return resp.status, parsed
         except urllib.error.HTTPError as e:
             raw = e.read()
@@ -252,6 +264,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 --------------------------------------------------------------
 

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,35 @@ 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"<html><body>captive portal</body></html>")
+        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))
+
+    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()