From 3b2f7a41c399100618ec57685ca20b519ca54cfd Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Fri, 5 Jun 2026 23:13:46 -0400 Subject: [PATCH] =?UTF-8?q?feat(patreon):=20ingester=20rate-limit=20resili?= =?UTF-8?q?ence=20=E2=80=94=20#703=20step=201?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A single 429 mid-walk used to fail the run → RATE_LIMITED → platform-wide cooldown → every Patreon source dark ("testing dead"). The native path also ignored the operator's existing politeness setting. Fixed both: - Pacing (avoid 429s): honor download_rate_limit_seconds (gallery-dl's `rate_limit`, read off self.gdl) as a pre-download sleep on real media downloads only (skips don't pace); pace /api/posts page fetches with the per-source sleep_request override, defaulting to max(0.5, rate_limit/4) — the same API-pacing default gallery-dl used for `sleep-request`. - 429 backoff (ride out transient limits): PatreonClient._fetch retries a 429 with backoff (honor Retry-After, else exponential 2·2^(n-1), capped 30s, ≤3 tries); only a PERSISTENT 429 propagates as terminal RATE_LIMITED. Light 2-retry on a media-GET 429 too. Threaded via PatreonIngester(rate_limit=, request_sleep=) → PatreonClient/PatreonDownloader; download_service sources them. Injected test client/downloader are unaffected (carry their own pacing). Tests mock time.sleep (no real sleeping): retry-then-success, persistent 429 raises after N, Retry-After honored, request_sleep paces, media pacing per real download, skips don't pace, media 429 retried. Co-Authored-By: Claude Opus 4.8 (1M context) --- backend/app/services/download_service.py | 15 ++++ backend/app/services/patreon_client.py | 79 +++++++++++++++++++--- backend/app/services/patreon_downloader.py | 37 +++++++++- backend/app/services/patreon_ingester.py | 14 +++- tests/test_patreon_client.py | 64 +++++++++++++++++- tests/test_patreon_downloader.py | 63 ++++++++++++++++- 6 files changed, 254 insertions(+), 18 deletions(-) diff --git a/backend/app/services/download_service.py b/backend/app/services/download_service.py index 118ccfb..b33e5b4 100644 --- a/backend/app/services/download_service.py +++ b/backend/app/services/download_service.py @@ -207,11 +207,26 @@ class DownloadService: None, ) + # Honor the operator's existing rate-limit knobs on the native path + # (plan #703): the global download_rate_limit_seconds (gallery-dl's + # `rate_limit`, here on self.gdl) paces media downloads. Page fetches use + # the per-source sleep_request override, else `max(0.5, rate_limit/4)` — + # the SAME API-pacing default gallery-dl applied as its `sleep-request` + # (gallery_dl._get_default_config), so the rate-limited /api/posts + # endpoint stays politely paced without over-throttling. + rate_limit = self.gdl._rate_limit + request_sleep = ( + source_config.sleep_request + if source_config.sleep_request is not None + else max(0.5, rate_limit / 4) + ) ingester = PatreonIngester( images_root=self.gdl.images_root, cookies_path=ctx["cookies_path"], session_factory=self.sync_session_factory, validate=self.gdl._validate_files, + rate_limit=rate_limit, + request_sleep=request_sleep, ) loop = asyncio.get_running_loop() dl_result = await loop.run_in_executor( diff --git a/backend/app/services/patreon_client.py b/backend/app/services/patreon_client.py index 87fb0b5..5d1ef2b 100644 --- a/backend/app/services/patreon_client.py +++ b/backend/app/services/patreon_client.py @@ -30,6 +30,7 @@ import http.cookiejar import logging import os import re +import time from collections.abc import Iterator from dataclasses import dataclass from html import unescape @@ -47,6 +48,34 @@ _USER_AGENT = ( ) _TIMEOUT_SECONDS = 30.0 +# 429 backoff (plan #703): ride out a transient API rate-limit instead of +# failing the whole walk (which would stamp RATE_LIMITED → platform-wide +# cooldown → every Patreon source dark). Honor the server's `Retry-After`; +# otherwise exponential base·2^(n-1), capped. Only after the retries are +# exhausted does the 429 propagate as terminal RATE_LIMITED. +_MAX_429_RETRIES = 3 +_BACKOFF_BASE_SECONDS = 2.0 +_BACKOFF_CAP_SECONDS = 30.0 + + +def _retry_after_seconds( + resp: requests.Response, + attempt: int, + *, + base: float = _BACKOFF_BASE_SECONDS, + cap: float = _BACKOFF_CAP_SECONDS, +) -> float: + """Backoff delay for a 429: the `Retry-After` seconds header if present and + numeric, else exponential `base·2^(attempt-1)`, both capped. (HTTP-date form + of Retry-After is rare here and falls through to exponential.)""" + header = resp.headers.get("Retry-After") + if header: + try: + return min(float(header), cap) + except (TypeError, ValueError): + pass + return min(base * (2 ** max(0, attempt - 1)), cap) + # JSON:API request contract (observed from real traffic — see module plan). _INCLUDE = ( "campaign,access_rules,attachments,attachments_media,audio,images,media," @@ -205,9 +234,19 @@ class PatreonClient: requests.Session; no secure-context APIs are used. """ - def __init__(self, cookies_path: str | Path | None): + def __init__( + self, + cookies_path: str | Path | None, + *, + request_sleep: float = 0.0, + max_retries: int = _MAX_429_RETRIES, + ): self.cookies_path = str(cookies_path) if cookies_path else None self._session = _load_session(cookies_path) + # Politeness: seconds to sleep before each /api/posts page fetch (paces + # the rate-limited API endpoint). 0 = no pacing. plan #703. + self._request_sleep = request_sleep or 0.0 + self._max_retries = max_retries # -- request ----------------------------------------------------------- @@ -228,16 +267,34 @@ class PatreonClient: return params def _fetch(self, campaign_id: str, cursor: str | None) -> dict: - try: - resp = self._session.get( - _POSTS_URL, - params=self._params(campaign_id, cursor), - timeout=_TIMEOUT_SECONDS, - ) - except requests.RequestException as exc: - raise PatreonAPIError( - f"Patreon posts request failed (campaign_id={campaign_id}): {exc}" - ) from exc + if self._request_sleep > 0: + time.sleep(self._request_sleep) # pace the API endpoint + attempt = 0 + while True: + try: + resp = self._session.get( + _POSTS_URL, + params=self._params(campaign_id, cursor), + timeout=_TIMEOUT_SECONDS, + ) + except requests.RequestException as exc: + raise PatreonAPIError( + f"Patreon posts request failed (campaign_id={campaign_id}): {exc}" + ) from exc + + # Transient rate-limit: back off and retry rather than failing the + # whole walk. Only a PERSISTENT 429 (retries exhausted) falls + # through to the terminal RATE_LIMITED raise below. + if resp.status_code == 429 and attempt < self._max_retries: + attempt += 1 + delay = _retry_after_seconds(resp, attempt) + log.warning( + "Patreon 429 (campaign_id=%s) — backing off %.1fs (retry %d/%d)", + campaign_id, delay, attempt, self._max_retries, + ) + time.sleep(delay) + continue + break if resp.status_code in (401, 403): # Auth rejected — expired/missing cookies or an insufficient tier. diff --git a/backend/app/services/patreon_downloader.py b/backend/app/services/patreon_downloader.py index 7103ec8..ed46d25 100644 --- a/backend/app/services/patreon_downloader.py +++ b/backend/app/services/patreon_downloader.py @@ -33,6 +33,7 @@ import logging import os import shutil import subprocess +import time from collections.abc import Callable from dataclasses import dataclass from datetime import datetime @@ -42,13 +43,16 @@ from urllib.parse import urlsplit import requests from .file_validator import is_validatable, validate_file -from .patreon_client import _load_session +from .patreon_client import _load_session, _retry_after_seconds log = logging.getLogger(__name__) _TITLE_MAX = 40 _TIMEOUT_SECONDS = 120.0 _CHUNK = 1 << 16 +# A CDN media GET rarely 429s, but if it does, a couple of backoff retries keep +# one throttled file from becoming a per-item error (plan #703). +_MAX_MEDIA_429_RETRIES = 2 # Referer/Origin yt-dlp must send for Mux-hosted Patreon video. Mux's JWT # playback policy checks Referer/Origin on every request, so yt-dlp must send @@ -148,11 +152,17 @@ class PatreonDownloader: cookies_path: str | None = None, *, validate: bool = True, + rate_limit: float = 0.0, session: requests.Session | None = None, ): self.images_root = Path(images_root) self.cookies_path = str(cookies_path) if cookies_path else None self._validate = validate + # Politeness: seconds to sleep before each actual media download (paces + # the CDN; honors ImportSettings.download_rate_limit_seconds, the same + # value gallery-dl used as its between-downloads `sleep`). 0 = no pacing. + # Applied only to real downloads, not to seen/disk skips. plan #703. + self._rate_limit = rate_limit or 0.0 # Build a cookie-loaded session the same way patreon_client does, so the # CDN GETs carry the creator's auth. self.session = session if session is not None else _load_session(cookies_path) @@ -227,6 +237,10 @@ class PatreonDownloader: post_dir.mkdir(parents=True, exist_ok=True) + # Pace real downloads only (the skips above already returned). plan #703. + if self._rate_limit > 0: + time.sleep(self._rate_limit) + if _is_video_url(media.url): out_path = self._run_ytdlp(media.url, media_path, _VIDEO_HEADERS) if out_path is None or not Path(out_path).exists(): @@ -267,8 +281,25 @@ class PatreonDownloader: return dest def _fetch_to_file(self, url: str, dest: Path) -> None: - """Stream a non-video URL to `dest` via the (stubbable) session.""" - resp = self.session.get(url, stream=True, timeout=_TIMEOUT_SECONDS) + """Stream a non-video URL to `dest` via the (stubbable) session. + + Retries a transient 429 with backoff (plan #703); any other non-2xx + falls through to raise_for_status → HTTPError, which download_post turns + into a resilient per-item error outcome. + """ + attempt = 0 + while True: + resp = self.session.get(url, stream=True, timeout=_TIMEOUT_SECONDS) + if resp.status_code == 429 and attempt < _MAX_MEDIA_429_RETRIES: + attempt += 1 + delay = _retry_after_seconds(resp, attempt) + log.warning( + "Patreon media 429 (%s) — backing off %.1fs (retry %d/%d)", + url, delay, attempt, _MAX_MEDIA_429_RETRIES, + ) + time.sleep(delay) + continue + break resp.raise_for_status() with open(dest, "wb") as fh: for chunk in resp.iter_content(chunk_size=_CHUNK): diff --git a/backend/app/services/patreon_ingester.py b/backend/app/services/patreon_ingester.py index 69a0b17..0a40fb6 100644 --- a/backend/app/services/patreon_ingester.py +++ b/backend/app/services/patreon_ingester.py @@ -104,18 +104,28 @@ class PatreonIngester: session_factory: Callable[[], object], *, validate: bool = True, + rate_limit: float = 0.0, + request_sleep: float = 0.0, client: PatreonClient | None = None, downloader: PatreonDownloader | None = None, ): self.images_root = Path(images_root) self.cookies_path = str(cookies_path) if cookies_path else None self.session_factory = session_factory - self.client = client if client is not None else PatreonClient(cookies_path) + # Pacing (plan #703): request_sleep paces the API page fetches, + # rate_limit paces the media downloads. Injected client/downloader (in + # tests) already carry their own pacing, so these only apply to the + # default-constructed ones. + self.client = ( + client + if client is not None + else PatreonClient(cookies_path, request_sleep=request_sleep) + ) self.downloader = ( downloader if downloader is not None else PatreonDownloader( - self.images_root, cookies_path, validate=validate + self.images_root, cookies_path, validate=validate, rate_limit=rate_limit ) ) diff --git a/tests/test_patreon_client.py b/tests/test_patreon_client.py index f1574ad..f2939e2 100644 --- a/tests/test_patreon_client.py +++ b/tests/test_patreon_client.py @@ -10,12 +10,14 @@ from pathlib import Path import pytest +from backend.app.services import patreon_client as pc_mod from backend.app.services.patreon_client import ( MediaItem, PatreonAPIError, PatreonAuthError, PatreonClient, PatreonDriftError, + _retry_after_seconds, parse_cursor_from_url, ) @@ -192,10 +194,11 @@ def test_media_referenced_but_absent_raises_drift(client): class _FakeResp: - def __init__(self, status_code, *, json_data=None, raise_json=False): + def __init__(self, status_code, *, json_data=None, raise_json=False, headers=None): self.status_code = status_code self._json_data = json_data self._raise_json = raise_json + self.headers = headers or {} def json(self): if self._raise_json: @@ -275,3 +278,62 @@ def test_verify_auth_inconclusive_on_network(monkeypatch): monkeypatch.setattr(client._session, "get", _raise) ok, _msg = client.verify_auth("5555") assert ok is None + + +# -- 429 backoff + pacing (plan #703) -------------------------------------- + + +def test_retry_after_seconds_honors_numeric_header(): + assert _retry_after_seconds(_FakeResp(429, headers={"Retry-After": "7"}), 1) == 7.0 + + +def test_retry_after_seconds_exponential_without_header(): + resp = _FakeResp(429) + assert _retry_after_seconds(resp, 1) == 2.0 + assert _retry_after_seconds(resp, 2) == 4.0 + assert _retry_after_seconds(resp, 3) == 8.0 + + +def test_retry_after_seconds_caps(): + assert _retry_after_seconds(_FakeResp(429), 10) == 30.0 + assert _retry_after_seconds(_FakeResp(429, headers={"Retry-After": "999"}), 1) == 30.0 + + +def _client_with_sequence(monkeypatch, responses): + """A client whose session yields `responses` in order; time.sleep stubbed so + backoff never really sleeps. Returns (client, slept_list).""" + client = PatreonClient(cookies_path=None) + it = iter(responses) + monkeypatch.setattr(client._session, "get", lambda *a, **k: next(it)) + slept: list[float] = [] + monkeypatch.setattr(pc_mod.time, "sleep", lambda s: slept.append(s)) + return client, slept + + +def test_fetch_retries_then_succeeds_on_429(monkeypatch): + client, slept = _client_with_sequence(monkeypatch, [ + _FakeResp(429, headers={"Retry-After": "5"}), + _FakeResp(200, json_data={"data": []}), + ]) + assert client._fetch("5555", None) == {"data": []} + assert 5.0 in slept # backed off once before the retry + + +def test_fetch_persistent_429_raises_after_retries(monkeypatch): + client, _slept = _client_with_sequence(monkeypatch, [_FakeResp(429)] * 10) + with pytest.raises(PatreonAPIError) as ei: + client._fetch("5555", None) + assert ei.value.status_code == 429 + assert not isinstance(ei.value, PatreonAuthError) + + +def test_request_sleep_paces_before_fetch(monkeypatch): + client = PatreonClient(cookies_path=None, request_sleep=1.5) + monkeypatch.setattr( + client._session, "get", + lambda *a, **k: _FakeResp(200, json_data={"data": []}), + ) + slept: list[float] = [] + monkeypatch.setattr(pc_mod.time, "sleep", lambda s: slept.append(s)) + client._fetch("5555", None) + assert slept == [1.5] diff --git a/tests/test_patreon_downloader.py b/tests/test_patreon_downloader.py index 6b79975..f96a6fb 100644 --- a/tests/test_patreon_downloader.py +++ b/tests/test_patreon_downloader.py @@ -10,7 +10,9 @@ from __future__ import annotations from pathlib import Path import pytest +import requests +from backend.app.services import patreon_downloader as pd_mod from backend.app.services.patreon_client import MediaItem from backend.app.services.patreon_downloader import ( MediaOutcome, @@ -26,10 +28,14 @@ _PNG_BYTES = _PNG_HEAD + (b"\x00" * 16) + _PNG_TAIL class _FakeResponse: - def __init__(self, payload: bytes): + def __init__(self, payload: bytes, status_code: int = 200, headers=None): self._payload = payload + self.status_code = status_code + self.headers = headers or {} def raise_for_status(self): + if self.status_code >= 400: + raise requests.HTTPError(f"HTTP {self.status_code}") return None def iter_content(self, chunk_size=65536): @@ -49,6 +55,18 @@ class _FakeSession: return _FakeResponse(self.payloads.get(url, _PNG_BYTES)) +class _SeqSession: + """Returns the queued responses in order (for retry tests).""" + + def __init__(self, responses: list[_FakeResponse]): + self._responses = list(responses) + self.calls: list[str] = [] + + def get(self, url, stream=False, timeout=None): + self.calls.append(url) + return self._responses.pop(0) + + def _post(post_id="1001", title="My Post Title", published="2026-05-01T12:00:00Z"): return { "id": post_id, @@ -250,3 +268,46 @@ def test_media_outcome_shape(): o = MediaOutcome(media=None, status="downloaded", path=Path("/tmp/x"), error=None) assert o.status == "downloaded" assert o.path == Path("/tmp/x") + + +# -- pacing + 429 backoff (plan #703) -------------------------------------- + + +def test_rate_limit_paces_each_real_download(tmp_path, monkeypatch): + slept: list[float] = [] + monkeypatch.setattr(pd_mod.time, "sleep", lambda s: slept.append(s)) + dl = PatreonDownloader( + images_root=tmp_path, cookies_path=None, validate=False, + rate_limit=2.0, session=_FakeSession(), + ) + dl.download_post(_post(), [_img("a.png"), _img("b.png")], "artist-x") + # One pacing sleep per actual download (two media downloaded). + assert slept == [2.0, 2.0] + + +def test_skips_do_not_pace(tmp_path, monkeypatch): + slept: list[float] = [] + monkeypatch.setattr(pd_mod.time, "sleep", lambda s: slept.append(s)) + dl = PatreonDownloader( + images_root=tmp_path, cookies_path=None, validate=False, + rate_limit=2.0, session=_FakeSession(), + ) + # is_seen → skipped_seen, never reaches the download/pacing path. + dl.download_post(_post(), [_img("a.png")], "artist-x", is_seen=lambda m: True) + assert slept == [] + + +def test_media_429_retried_then_succeeds(tmp_path, monkeypatch): + slept: list[float] = [] + monkeypatch.setattr(pd_mod.time, "sleep", lambda s: slept.append(s)) + session = _SeqSession([ + _FakeResponse(b"", status_code=429, headers={"Retry-After": "4"}), + _FakeResponse(_PNG_BYTES, status_code=200), + ]) + dl = PatreonDownloader( + images_root=tmp_path, cookies_path=None, validate=False, session=session, + ) + outcomes = dl.download_post(_post(), [_img("a.png")], "artist-x") + assert outcomes[0].status == "downloaded" + assert 4.0 in slept # backed off before the retry + assert len(session.calls) == 2