diff --git a/shelfmark/metadata_providers/hardcover.py b/shelfmark/metadata_providers/hardcover.py index 28df49a28..acfa6b66d 100644 --- a/shelfmark/metadata_providers/hardcover.py +++ b/shelfmark/metadata_providers/hardcover.py @@ -1,12 +1,15 @@ """Hardcover.app metadata provider. Requires API key.""" +import math +import random import re +import threading import time from contextlib import suppress from dataclasses import dataclass from datetime import UTC, datetime from http import HTTPStatus -from typing import Any, ClassVar +from typing import TYPE_CHECKING, Any, ClassVar from urllib.parse import urlparse import requests @@ -43,6 +46,9 @@ logger = setup_logger(__name__) +if TYPE_CHECKING: + from collections.abc import Callable + HARDCOVER_API_URL = "https://api.hardcover.app/v1/graphql" HARDCOVER_PAGE_SIZE = 25 # Hardcover API returns max 25 results per page HARDCOVER_MIN_AUTHOR_PARTS = 2 @@ -507,6 +513,83 @@ class HardcoverGraphQLError(ValueError): """GraphQL request was rejected by Hardcover.""" +class HardcoverRateLimitError(RuntimeError): + """Hardcover kept answering 429 after the retries, or asked for a wait too long to sit out.""" + + +# Hardcover's documented limits (docs.hardcover.app, Getting Started > Rate Limits): a token +# bucket of 10 (Free) that refills at 60 per minute, 5,000 requests a day. The per-user limit +# is shared with every other app and session on the same account, so pace below it. +HARDCOVER_RATE_BURST = 5 +HARDCOVER_RATE_PER_SECOND = 0.8 +HARDCOVER_RATE_LIMIT_ATTEMPTS = 3 # the first try plus two retries +HARDCOVER_RETRY_AFTER_CAP_SECONDS = 30.0 # longer than this (spent daily quota): give up + + +class _TokenBucket: + """Thread-safe token bucket. ``acquire`` blocks until a request may go out. + + The clock and sleep are injectable so tests never wait for real. + """ + + def __init__( + self, + capacity: int, + per_second: float, + clock: Callable[[], float] = time.monotonic, + sleep: Callable[[float], None] = time.sleep, + ) -> None: + self._capacity = float(capacity) + self._per_second = per_second + self._clock = clock + self._sleep = sleep + self._lock = threading.Lock() + self._tokens = float(capacity) + self._updated = clock() + + def _take_or_wait(self) -> float: + """Take a token and return 0, or return how long to wait before trying again.""" + with self._lock: + now = self._clock() + if now > self._updated: + gained = (now - self._updated) * self._per_second + self._tokens = min(self._capacity, self._tokens + gained) + self._updated = now + if self._tokens >= 1.0: + self._tokens -= 1.0 + return 0.0 + # _updated can sit in the future after penalize(): nothing refills before then. + return (self._updated - now) + (1.0 - self._tokens) / self._per_second + + def acquire(self) -> None: + while (wait := self._take_or_wait()) > 0.0: + self._sleep(wait) + + def penalize(self, seconds: float) -> None: + """Empty the bucket and hold every caller back for ``seconds``.""" + with self._lock: + self._tokens = 0.0 + self._updated = max(self._updated, self._clock() + seconds) + + +_rate_limiter = _TokenBucket(HARDCOVER_RATE_BURST, HARDCOVER_RATE_PER_SECOND) + + +def _retry_jitter() -> float: + return random.uniform(0.0, 1.0) # noqa: S311 - spreading retries, not security + + +def _throttle_wait_seconds(response: requests.Response, attempt: int) -> float: + """Seconds to wait after a 429: Retry-After when sent, else exponential backoff.""" + raw = response.headers.get("Retry-After") + if raw is not None: + with suppress(ValueError): + seconds = float(raw) + if math.isfinite(seconds) and seconds >= 0: + return seconds + return float(2**attempt) + _retry_jitter() + + class HardcoverTargetPayloadError(RuntimeError): """Hardcover returned an invalid payload while loading book targets.""" @@ -2557,7 +2640,9 @@ def get_book(self, book_id: str) -> BookMetadata | None: try: book_id_int = int(book_id) - result = self._execute_query(graphql_query, {"id": book_id_int}) + # A failed lookup is not "no such book": let throttling and outages reach the + # caller instead of coming back as None. + result = self._execute_query(graphql_query, {"id": book_id_int}, raise_on_error=True) if not result: return None @@ -2567,6 +2652,9 @@ def get_book(self, book_id: str) -> BookMetadata | None: return self._parse_book(books[0]) + except HardcoverGraphQLError: + logger.exception("Hardcover rejected the book lookup for %s", book_id) + return None except ValueError: logger.exception("Invalid book ID: %s", book_id) return None @@ -2645,6 +2733,40 @@ def search_by_isbn(self, isbn: str) -> BookMetadata | None: logger.exception("Hardcover ISBN search error") return None + def _post_with_backoff(self, payload: dict[str, Any]) -> requests.Response: + """POST one GraphQL request, paced by the shared limiter, retrying a 429. + + Raises HardcoverRateLimitError when the retries run out or Hardcover asks for a + wait longer than HARDCOVER_RETRY_AFTER_CAP_SECONDS. + """ + for attempt in range(1, HARDCOVER_RATE_LIMIT_ATTEMPTS + 1): + _rate_limiter.acquire() + response = self.session.post( + HARDCOVER_API_URL, + json=payload, + timeout=15, + verify=get_ssl_verify(HARDCOVER_API_URL), + ) + if response.status_code != HTTPStatus.TOO_MANY_REQUESTS: + return response + + wait = _throttle_wait_seconds(response, attempt) + if wait > HARDCOVER_RETRY_AFTER_CAP_SECONDS: + msg = f"Hardcover rate limit reached; it asked for a {wait:.0f}s wait" + raise HardcoverRateLimitError(msg) + if attempt == HARDCOVER_RATE_LIMIT_ATTEMPTS: + msg = f"Hardcover rate limit still hit after {attempt} attempts" + raise HardcoverRateLimitError(msg) + logger.warning( + "Hardcover rate limited (429); retrying in %.0fs (attempt %d/%d)", + wait, + attempt, + HARDCOVER_RATE_LIMIT_ATTEMPTS, + ) + _rate_limiter.penalize(wait) + msg = "unreachable" # the loop always returns or raises + raise AssertionError(msg) + def _execute_query( self, query: str, @@ -2658,12 +2780,7 @@ def _raise_graphql_error(message: str) -> None: raise HardcoverGraphQLError(message) try: - response = self.session.post( - HARDCOVER_API_URL, - json={"query": query, "variables": variables}, - timeout=15, - verify=get_ssl_verify(HARDCOVER_API_URL), - ) + response = self._post_with_backoff({"query": query, "variables": variables}) response.raise_for_status() data = response.json() @@ -2699,6 +2816,11 @@ def _raise_graphql_error(message: str) -> None: return None except HardcoverGraphQLError: raise + except HardcoverRateLimitError as e: + logger.warning("Hardcover API: %s", e) + if raise_on_error: + raise + return None except ValueError as e: logger.exception("Hardcover API returned invalid JSON") if raise_on_error: diff --git a/tests/metadata/test_hardcover_rate_limit.py b/tests/metadata/test_hardcover_rate_limit.py new file mode 100644 index 000000000..91eed1f35 --- /dev/null +++ b/tests/metadata/test_hardcover_rate_limit.py @@ -0,0 +1,272 @@ +"""Hardcover allows a 10 request burst refilling at 60/min and answers over-limit +calls with HTTP 429 plus a Retry-After header. Every request goes through +``_execute_query``, so that is where the client paces itself and backs off.""" + +from typing import Any + +import pytest +import requests + +from shelfmark.core.cache import get_metadata_cache +from shelfmark.metadata_providers import hardcover +from shelfmark.metadata_providers.hardcover import ( + HardcoverProvider, + HardcoverRateLimitError, + _TokenBucket, +) + +PAT = "hc_pat_" + "a" * 32 +BOOK = { + "books": [ + { + "id": 7, + "title": "Dune", + "contributions": [{"author": {"name": "Frank Herbert"}}], + } + ] +} + + +class FakeClock: + """Monotonic clock whose sleep just moves time forward.""" + + def __init__(self) -> None: + self.now = 1000.0 + self.sleeps: list[float] = [] + + def __call__(self) -> float: + return self.now + + def sleep(self, seconds: float) -> None: + self.sleeps.append(seconds) + self.now += seconds + + +class FakeResponse: + def __init__(self, status: int, body: dict | None = None, headers: dict | None = None): + self.status_code = status + self._body = body if body is not None else {} + self.headers = headers or {} + + def raise_for_status(self) -> None: + if self.status_code >= 400: + raise requests.HTTPError(f"{self.status_code} error", response=self) + + def json(self) -> dict: + return self._body + + +class FakeSession: + def __init__(self, responses: list[FakeResponse]) -> None: + self.responses = list(responses) + self.posts = 0 + + def post(self, *_args: Any, **_kwargs: Any) -> FakeResponse: + self.posts += 1 + return self.responses.pop(0) if len(self.responses) > 1 else self.responses[0] + + +def ok(data: dict | None = None) -> FakeResponse: + return FakeResponse(200, {"data": data if data is not None else BOOK}) + + +def throttled(retry_after: str | None = None) -> FakeResponse: + headers = {"Retry-After": retry_after} if retry_after is not None else {} + return FakeResponse(429, {"error": "Too Many Requests"}, headers) + + +@pytest.fixture(autouse=True) +def _fresh_metadata_cache(): + """get_book is cached by id; keep one test's book from answering the next.""" + get_metadata_cache().clear() + yield + get_metadata_cache().clear() + + +@pytest.fixture +def clock(monkeypatch) -> FakeClock: + """Swap the shared limiter for a fake clock; nothing really sleeps.""" + fake = FakeClock() + monkeypatch.setattr(hardcover, "_rate_limiter", _TokenBucket(5, 1.0, fake, fake.sleep)) + monkeypatch.setattr(hardcover, "_retry_jitter", lambda: 0.0) + return fake + + +def provider_with(*responses: FakeResponse) -> tuple[HardcoverProvider, FakeSession]: + provider = HardcoverProvider(api_key=PAT) + session = FakeSession(list(responses)) + provider.session = session # type: ignore[assignment] + return provider, session + + +class TestTokenBucket: + def test_allows_a_burst_without_waiting(self): + fake = FakeClock() + bucket = _TokenBucket(5, 1.0, fake, fake.sleep) + + for _ in range(5): + bucket.acquire() + + assert fake.sleeps == [] + + def test_waits_for_a_token_once_the_burst_is_spent(self): + fake = FakeClock() + bucket = _TokenBucket(5, 0.5, fake, fake.sleep) + for _ in range(5): + bucket.acquire() + + bucket.acquire() + + assert sum(fake.sleeps) == pytest.approx(2.0) + + def test_refills_with_time(self): + fake = FakeClock() + bucket = _TokenBucket(5, 1.0, fake, fake.sleep) + for _ in range(5): + bucket.acquire() + fake.now += 3.0 + + for _ in range(3): + bucket.acquire() + + assert fake.sleeps == [] + + def test_idle_time_does_not_bank_more_than_the_burst(self): + fake = FakeClock() + bucket = _TokenBucket(5, 1.0, fake, fake.sleep) + fake.now += 10_000.0 + for _ in range(5): + bucket.acquire() + assert fake.sleeps == [] + + bucket.acquire() + + assert sum(fake.sleeps) == pytest.approx(1.0) + + def test_sustained_rate_never_exceeds_the_refill_rate(self): + fake = FakeClock() + bucket = _TokenBucket(5, 1.0, fake, fake.sleep) + start = fake.now + + for _ in range(65): + bucket.acquire() + + # 5 free, then one per second. + assert fake.now - start == pytest.approx(60.0) + + def test_penalize_blocks_the_next_acquire(self): + fake = FakeClock() + bucket = _TokenBucket(5, 1.0, fake, fake.sleep) + + bucket.penalize(12.0) + bucket.acquire() + + assert sum(fake.sleeps) >= 12.0 + + +class TestExecuteQueryThrottling: + def test_every_request_takes_a_token(self, clock): + provider, session = provider_with(ok()) + + for _ in range(8): + provider._execute_query("query { me { id } }", {}) + + assert session.posts == 8 + # 5 burst + 3 paced at one per second. + assert sum(clock.sleeps) == pytest.approx(3.0) + + def test_429_is_retried_after_the_retry_after_delay(self, clock): + provider, session = provider_with(throttled("7"), ok()) + + result = provider._execute_query("query { me { id } }", {}) + + assert result == BOOK + assert session.posts == 2 + assert sum(clock.sleeps) >= 7.0 + + def test_429_without_retry_after_backs_off_and_grows(self, clock): + provider, session = provider_with(throttled(), throttled(), ok()) + + result = provider._execute_query("query { me { id } }", {}) + + assert result == BOOK + assert session.posts == 3 + waits = [s for s in clock.sleeps if s > 0] + assert len(waits) == 2 + assert waits[1] > waits[0] > 0 + + def test_unparseable_retry_after_falls_back_to_backoff(self, clock): + provider, session = provider_with(throttled("Wed, 21 Oct 2026 07:28:00 GMT"), ok()) + + assert provider._execute_query("query { me { id } }", {}) == BOOK + assert session.posts == 2 + assert sum(clock.sleeps) > 0 + + def test_retries_are_bounded(self, clock): + provider, session = provider_with(throttled("1")) + + with pytest.raises(HardcoverRateLimitError): + provider._execute_query("query { me { id } }", {}, raise_on_error=True) + + assert session.posts == hardcover.HARDCOVER_RATE_LIMIT_ATTEMPTS + + def test_exhausted_429_returns_none_without_raise_on_error(self, clock): + provider, session = provider_with(throttled("1")) + + assert provider._execute_query("query { me { id } }", {}) is None + assert session.posts == hardcover.HARDCOVER_RATE_LIMIT_ATTEMPTS + + def test_long_retry_after_is_not_waited_out(self, clock): + """A spent daily quota says to wait until midnight UTC; don't hold a worker for that.""" + provider, session = provider_with(throttled("3600")) + + with pytest.raises(HardcoverRateLimitError): + provider._execute_query("query { me { id } }", {}, raise_on_error=True) + + assert session.posts == 1 + assert all(s < 60 for s in clock.sleeps) + + def test_a_429_holds_back_other_callers_too(self, clock): + provider, _session = provider_with(throttled("20"), ok()) + provider._execute_query("query { me { id } }", {}) + clock.sleeps.clear() + before = clock.now + + hardcover._rate_limiter.acquire() + + # The retry already waited 20s; the shared bucket starts empty afterwards. + assert clock.now - before <= 1.0 + 1e-9 + + @pytest.mark.parametrize("status", [401, 403, 500]) + def test_other_http_errors_are_not_retried(self, clock, status): + provider, session = provider_with(FakeResponse(status, {"error": "no"})) + + assert provider._execute_query("query { me { id } }", {}) is None + assert session.posts == 1 + + +class TestGetBookDuringThrottle: + def test_get_book_recovers_from_a_single_429(self, clock): + provider, session = provider_with(throttled("1"), ok()) + + book = provider.get_book("7") + + assert book is not None + assert book.title == "Dune" + assert session.posts == 2 + + def test_get_book_raises_instead_of_reporting_not_found(self, clock): + provider, _session = provider_with(throttled("1")) + + with pytest.raises(HardcoverRateLimitError): + provider.get_book("7") + + def test_get_book_still_returns_none_when_graphql_rejects_it(self, clock): + provider, _session = provider_with(FakeResponse(200, {"errors": [{"message": "nope"}]})) + + assert provider.get_book("7") is None + + def test_get_book_still_returns_none_for_a_missing_book(self, clock): + provider, _session = provider_with(ok({"books": []})) + + assert provider.get_book("7") is None