Harden fetch error handling per review (HTTPException, charset, backoff cap)

noderaven · noderaven · commit db78b805451e · 2026-06-24T17:09:37.000-07:00
diff --git a/breach_scraper/wa_atg_scraper.py b/breach_scraper/wa_atg_scraper.py
@@ -16,6 +16,7 @@
 from collections.abc import Iterable
 from dataclasses import dataclass, field
 from html.parser import HTMLParser
+from http.client import HTTPException
 from pathlib import Path
 from urllib.error import HTTPError, URLError
 from urllib.parse import urljoin
@@ -159,7 +160,11 @@ def fetch_html(
             with urlopen(request, timeout=timeout) as response:  # nosec B310
                 charset = response.headers.get_content_charset() or "utf-8"
                 body: bytes = response.read()
-                return body.decode(charset, errors="replace")
+                try:
+                    return body.decode(charset, errors="replace")
+                except LookupError:
+                    # Server advertised an unknown charset; fall back to UTF-8.
+                    return body.decode("utf-8", errors="replace")
         except HTTPError as exc:
             if exc.code == 403:
                 raise RuntimeError(
@@ -172,10 +177,10 @@ def fetch_html(
                 last_error = exc
             else:
                 raise RuntimeError(f"Failed to fetch source page: HTTP {exc.code}.") from exc
-        except (URLError, TimeoutError) as exc:
+        except (URLError, TimeoutError, HTTPException) as exc:
             last_error = exc
         if attempt < attempts - 1:
-            time.sleep(backoff * (2**attempt))
+            time.sleep(min(backoff * (2**attempt), 30.0))
     raise RuntimeError(
         f"Failed to fetch source page after {attempts} attempt(s): {last_error}"
     ) from last_error
@@ -285,7 +290,7 @@ def main(argv: list[str] | None = None) -> int:
             records = records[: args.limit]
 
         write_output(records, output_format=args.output, out_file=args.out_file)
-    except (RuntimeError, OSError) as exc:
+    except (RuntimeError, OSError, ValueError) as exc:
         print(f"error: {exc}", file=sys.stderr)
         return 1
     return 0
diff --git a/tests/test_wa_atg_scraper.py b/tests/test_wa_atg_scraper.py
@@ -4,6 +4,7 @@
 import json
 import unittest
 from contextlib import redirect_stderr, redirect_stdout
+from http.client import BadStatusLine
 from pathlib import Path
 from unittest import mock
 from urllib.error import HTTPError, URLError
@@ -81,6 +82,33 @@ def test_http_403_raises_actionable_error(self, mock_urlopen: mock.Mock) -> None
         self.assertIn("--input-html", message)
         self.assertEqual(mock_urlopen.call_count, 1)
 
+    @mock.patch("breach_scraper.wa_atg_scraper.time.sleep", return_value=None)
+    @mock.patch("breach_scraper.wa_atg_scraper.urlopen")
+    def test_retries_on_http_protocol_error(
+        self, mock_urlopen: mock.Mock, _sleep: mock.Mock
+    ) -> None:
+        mock_urlopen.side_effect = [BadStatusLine("oops"), _FakeResponse(b"<html>ok</html>")]
+        html = fetch_html("https://example.test", retries=3, backoff=0)
+        self.assertEqual(html, "<html>ok</html>")
+        self.assertEqual(mock_urlopen.call_count, 2)
+
+    @mock.patch("breach_scraper.wa_atg_scraper.urlopen")
+    def test_unknown_charset_falls_back_to_utf8(self, mock_urlopen: mock.Mock) -> None:
+        mock_urlopen.return_value = _FakeResponse(
+            b"<html>caf\xc3\xa9</html>", charset="bogus-charset"
+        )
+        html = fetch_html("https://example.test", retries=1, backoff=0)
+        self.assertIn("caf", html)
+
+    @mock.patch("breach_scraper.wa_atg_scraper.time.sleep")
+    @mock.patch("breach_scraper.wa_atg_scraper.urlopen")
+    def test_backoff_is_capped(self, mock_urlopen: mock.Mock, mock_sleep: mock.Mock) -> None:
+        mock_urlopen.side_effect = URLError("down")
+        with self.assertRaises(RuntimeError):
+            fetch_html("https://example.test", retries=20, backoff=1.0)
+        max_sleep = max(call.args[0] for call in mock_sleep.call_args_list)
+        self.assertLessEqual(max_sleep, 30.0)
+
 
 class TestMainCli(unittest.TestCase):
     def _run(self, argv: list[str]) -> tuple[int, str, str]:
@@ -119,6 +147,11 @@ def test_missing_input_file_returns_error_code(self) -> None:
         self.assertEqual(rc, 1)
         self.assertIn("error:", err)
 
+    def test_malformed_url_returns_error_code(self) -> None:
+        rc, _, err = self._run(["--url", "not a url"])
+        self.assertEqual(rc, 1)
+        self.assertIn("error:", err)
+
 
 if __name__ == "__main__":
     unittest.main()
