diff --git a/http_retry.py b/http_retry.py new file mode 100644 index 0000000..d43b31a --- /dev/null +++ b/http_retry.py @@ -0,0 +1,44 @@ +"""Bounded retries for HTTP operations that are safe to repeat.""" + +import logging +import ssl +import time +from http.client import IncompleteRead, RemoteDisconnected +from urllib.error import HTTPError, URLError + + +def retry_http(operation): + """Try a read-only operation up to three times, retaining its own timeout. + + The operation must consume and close its response before returning. Do not + use this for requests that mutate server state, even if a timeout is raised. + """ + for attempt in range(3): + try: + return operation() + except HTTPError as error: + error.close() + if error.code not in {408, 429, 500, 502, 503, 504} or attempt == 2: + raise + reason = str(error) + except URLError as error: + if isinstance(error.reason, ssl.SSLCertVerificationError) or attempt == 2: + raise + reason = str(error) + except ( + TimeoutError, + ConnectionError, + IncompleteRead, + RemoteDisconnected, + ) as error: + if attempt == 2: + raise + reason = str(error) + delay = 2**attempt + logging.warning( + "HTTP attempt %d/3 failed: %s; retrying in %d seconds", + attempt + 1, + reason, + delay, + ) + time.sleep(delay) diff --git a/install_html_meta_data.py b/install_html_meta_data.py index 586be04..c491056 100644 --- a/install_html_meta_data.py +++ b/install_html_meta_data.py @@ -8,6 +8,11 @@ from lxml.html import document_fromstring, parse +if __package__: + from .http_retry import retry_http +else: + from http_retry import retry_http + change_detected = False READ_ONLY = False @@ -34,9 +39,14 @@ def fetch_bibliography(url, tags): headers={"Content-Type": "application/json", "Accept": "application/json"}, method="POST", ) - try: + + def fetch(): with urlopen(request, timeout=30) as response: - payload = json.load(response) + return json.load(response) + + try: + # This POST only looks up bibliography records, so retrying is safe. + payload = retry_http(fetch) except (HTTPError, URLError, TimeoutError, ValueError) as error: raise RuntimeError( f"ELIB: Failed to fetch bibliography from {url}: {error}" diff --git a/test_check_website.py b/test_check_website.py index 048c46a..4e9f569 100644 --- a/test_check_website.py +++ b/test_check_website.py @@ -1,10 +1,16 @@ -import requests +from urllib.request import urlopen + from htmlbook.book_name import get_project_name +from htmlbook.http_retry import retry_http def test_website_up() -> None: - url = f"http://{get_project_name()}.mit.edu/python/index.html" - response = requests.get(url, timeout=30) - assert ( - response.status_code == 200 - ), f"Website {url} returned status code {response.status_code}" + url = f"https://{get_project_name()}.mit.edu/python/index.html" + + def check(): + with urlopen(url, timeout=30) as response: + assert ( + response.status == 200 + ), f"Website {url} returned status code {response.status}" + + retry_http(check) diff --git a/test_http_retry.py b/test_http_retry.py new file mode 100644 index 0000000..5b9b557 --- /dev/null +++ b/test_http_retry.py @@ -0,0 +1,61 @@ +import io +import ssl +from http.client import IncompleteRead +from unittest.mock import Mock, call +from urllib.error import HTTPError, URLError + +import pytest +from htmlbook import http_retry + + +@pytest.mark.parametrize( + "error", + [ + TimeoutError("timed out"), + URLError(TimeoutError("connect timed out")), + ConnectionResetError("connection reset"), + IncompleteRead(b"partial"), + HTTPError("https://example.org", 503, "Unavailable", {}, io.BytesIO()), + ], +) +def test_transient_failure_recovers(monkeypatch, error): + sleep = Mock() + monkeypatch.setattr(http_retry.time, "sleep", sleep) + operation = Mock(side_effect=[error, "response"]) + assert http_retry.retry_http(operation) == "response" + assert operation.call_count == 2 + sleep.assert_called_once_with(1) + if isinstance(error, HTTPError): + assert error.closed + + +def test_retry_budget_is_bounded(monkeypatch): + sleep = Mock() + monkeypatch.setattr(http_retry.time, "sleep", sleep) + error = TimeoutError("still unavailable") + operation = Mock(side_effect=error) + with pytest.raises(TimeoutError) as caught: + http_retry.retry_http(operation) + assert caught.value is error + assert operation.call_count == 3 + assert sleep.call_args_list == [call(1), call(2)] + + +@pytest.mark.parametrize( + "error", + [ + HTTPError("https://example.org", 400, "Bad Request", {}, io.BytesIO()), + HTTPError("https://example.org", 404, "Not Found", {}, io.BytesIO()), + URLError(ssl.SSLCertVerificationError("invalid certificate")), + ValueError("invalid JSON"), + ], +) +def test_permanent_failure_is_not_retried(monkeypatch, error): + sleep = Mock() + monkeypatch.setattr(http_retry.time, "sleep", sleep) + operation = Mock(side_effect=error) + with pytest.raises(type(error)) as caught: + http_retry.retry_http(operation) + assert caught.value is error + operation.assert_called_once_with() + sleep.assert_not_called() diff --git a/test_install_html_meta_data.py b/test_install_html_meta_data.py index 53606b9..1585a7c 100644 --- a/test_install_html_meta_data.py +++ b/test_install_html_meta_data.py @@ -4,6 +4,7 @@ from urllib.error import URLError import pytest +from htmlbook import http_retry from htmlbook import install_html_meta_data as metadata from htmlbook.install_html_meta_data import install_html_meta_data @@ -54,6 +55,8 @@ def test_invalid_or_missing_bibliography(monkeypatch, payload): def test_network_failure_precedes_any_writes(monkeypatch): + monkeypatch.setattr(http_retry.time, "sleep", Mock()) + def fail(*args, **kwargs): raise URLError("unavailable") @@ -63,3 +66,20 @@ def fail(*args, **kwargs): with pytest.raises(RuntimeError, match="Failed to fetch"): install_html_meta_data() write.assert_not_called() + + +def test_bibliography_recovers_from_connection_timeout(monkeypatch): + monkeypatch.setattr(http_retry.time, "sleep", Mock()) + entry = {"bibtag": "A", "bibtype": "article", "title": "Title", "year": "2025"} + response = io.BytesIO(json.dumps({"entries": {"A": entry}, "missing": []}).encode()) + fetch = Mock(side_effect=[URLError(TimeoutError("timed out")), response]) + monkeypatch.setattr(metadata, "urlopen", fetch) + assert metadata.fetch_bibliography("https://example.org/elib.cgi", ["A"]) == { + "A": entry + } + assert fetch.call_count == 2 + for invocation in fetch.call_args_list: + assert invocation.kwargs == {"timeout": 30} + assert invocation.args[0].get_method() == "POST" + assert json.loads(invocation.args[0].data) == ["A"] + assert response.closed