diff --git a/backend/app/services/download_service.py b/backend/app/services/download_service.py index 5295dcd..66e960b 100644 --- a/backend/app/services/download_service.py +++ b/backend/app/services/download_service.py @@ -407,6 +407,7 @@ class DownloadService: await self._update_source_health( source_id=ctx["source_id"], status=status, error_message=ev.error, error_type=dl_result.error_type.value if dl_result.error_type else None, + retry_after_seconds=getattr(dl_result, "retry_after_seconds", None), ) await self._apply_backfill_lifecycle(ctx, dl_result) await self.async_session.commit() @@ -501,7 +502,7 @@ class DownloadService: async def _update_source_health( self, *, source_id: int, status: str, error_message: str | None, - error_type: str | None = None, + error_type: str | None = None, retry_after_seconds: float | None = None, ) -> None: """FC-3d: update Source.{consecutive_failures, last_error, last_checked_at}. @@ -533,7 +534,17 @@ class DownloadService: # by error class without opening Logs per row. source.error_type = error_type if error_type == "rate_limited": - await set_platform_cooldown(self.async_session, source.platform) + # plan #708 B1: honor the server's Retry-After when the native + # client surfaced one (clamped to a sane [60, 3600] window so a + # tiny hint can't leave the platform effectively un-cooled and a + # huge one can't strand it for hours); else the flat default. + if retry_after_seconds is not None: + seconds = int(min(max(retry_after_seconds, 60), 3600)) + await set_platform_cooldown( + self.async_session, source.platform, seconds=seconds, + ) + else: + await set_platform_cooldown(self.async_session, source.platform) elif status == "skipped": source.last_error = None source.last_checked_at = now diff --git a/backend/app/services/gallery_dl.py b/backend/app/services/gallery_dl.py index d9e9541..3add0e1 100644 --- a/backend/app/services/gallery_dl.py +++ b/backend/app/services/gallery_dl.py @@ -163,6 +163,10 @@ class DownloadResult: run_stats: dict | None = None cursor: str | None = None posts_processed: int = 0 + # Plan #708 B1 — the server's Retry-After seconds on a RATE_LIMITED result, so + # the platform cooldown matches the hint instead of a flat default. None when + # unknown (no header, or not a rate-limit failure). + retry_after_seconds: float | None = None def extract_errors_warnings(stderr: str) -> str: diff --git a/backend/app/services/patreon_client.py b/backend/app/services/patreon_client.py index f2af42f..69846db 100644 --- a/backend/app/services/patreon_client.py +++ b/backend/app/services/patreon_client.py @@ -108,11 +108,21 @@ class PatreonAPIError(Exception): `status_code` carries the HTTP status when the failure was an HTTP response (None for transport-level / parse failures), so the ingester can map it to a DownloadResult.error_type (429 → rate_limited, 404 → not_found, …). + `retry_after` carries the server's `Retry-After` seconds on a terminal 429, so + the platform cooldown can match the server's hint instead of a flat default + (plan #708 B1). """ - def __init__(self, message: str, *, status_code: int | None = None): + def __init__( + self, + message: str, + *, + status_code: int | None = None, + retry_after: float | None = None, + ): super().__init__(message) self.status_code = status_code + self.retry_after = retry_after class PatreonAuthError(PatreonAPIError): @@ -294,10 +304,22 @@ class PatreonClient: status_code=resp.status_code, ) if resp.status_code != 200: + # A persistent 429 (retries exhausted) is terminal RATE_LIMITED — carry + # the server's raw Retry-After seconds so the cooldown matches its hint + # (plan #708 B1). Header is uncapped here; the cooldown clamps it. + retry_after = None + if resp.status_code == 429: + hdr = resp.headers.get("Retry-After") + if hdr: + try: + retry_after = float(hdr) + except (TypeError, ValueError): + retry_after = None raise PatreonAPIError( f"Patreon posts API returned HTTP {resp.status_code} " f"(campaign_id={campaign_id})", status_code=resp.status_code, + retry_after=retry_after, ) try: payload = resp.json() diff --git a/backend/app/services/patreon_ingester.py b/backend/app/services/patreon_ingester.py index 3862088..5915da7 100644 --- a/backend/app/services/patreon_ingester.py +++ b/backend/app/services/patreon_ingester.py @@ -162,10 +162,14 @@ class PatreonIngester(Ingester): else: error_type = ErrorType.NETWORK_ERROR log.warning("Patreon ingest failed (%s): %s", error_type.value, message) - return _result( + result = _result( success=False, return_code=1, error_type=error_type, error_message=message, ) + # plan #708 B1: carry the server's Retry-After up to the cooldown. + if error_type == ErrorType.RATE_LIMITED: + result.retry_after_seconds = getattr(exc, "retry_after", None) + return result async def verify_patreon_credential( diff --git a/tests/test_download_service.py b/tests/test_download_service.py index 354e04e..a4b4779 100644 --- a/tests/test_download_service.py +++ b/tests/test_download_service.py @@ -437,6 +437,40 @@ async def test_finalize_error_increments_failures_and_sets_error(db): assert row.last_checked_at is not None +@pytest.mark.asyncio +async def test_rate_limited_cooldown_honors_retry_after(db, monkeypatch): + """plan #708 B1: a RATE_LIMITED result with a Retry-After stamps the platform + cooldown at that duration, clamped to [60, 3600]; no hint → the flat default.""" + from unittest.mock import AsyncMock, MagicMock + + from backend.app.services import download_service as dl_mod + from backend.app.services.download_service import DownloadService + + source_id, _event_id = await _seed_source_with_health(db, suffix="rl") + cooldown = AsyncMock() + monkeypatch.setattr(dl_mod, "set_platform_cooldown", cooldown) + svc = DownloadService( + async_session=db, sync_session=None, + gdl=MagicMock(), importer=MagicMock(), cred_service=MagicMock(), + ) + + async def _health(retry): + cooldown.reset_mock() + await svc._update_source_health( + source_id=source_id, status="error", error_message="rate limited", + error_type="rate_limited", retry_after_seconds=retry, + ) + + await _health(120.0) + assert cooldown.await_args.kwargs["seconds"] == 120 # honored as-is + await _health(5.0) + assert cooldown.await_args.kwargs["seconds"] == 60 # floored + await _health(99999.0) + assert cooldown.await_args.kwargs["seconds"] == 3600 # capped + await _health(None) + assert "seconds" not in cooldown.await_args.kwargs # default cooldown + + @pytest.mark.asyncio async def test_finalize_skipped_preserves_failures_clears_error(db): from backend.app.services.download_service import DownloadService diff --git a/tests/test_patreon_client.py b/tests/test_patreon_client.py index f2939e2..0426147 100644 --- a/tests/test_patreon_client.py +++ b/tests/test_patreon_client.py @@ -327,6 +327,18 @@ def test_fetch_persistent_429_raises_after_retries(monkeypatch): assert not isinstance(ei.value, PatreonAuthError) +def test_fetch_persistent_429_surfaces_retry_after(monkeypatch): + """plan #708 B1: a terminal 429 carries the server's raw Retry-After seconds + (uncapped here — the cooldown clamps) so the platform cooldown matches it.""" + client, _slept = _client_with_sequence( + monkeypatch, [_FakeResp(429, headers={"Retry-After": "120"})] * 10, + ) + with pytest.raises(PatreonAPIError) as ei: + client._fetch("5555", None) + assert ei.value.status_code == 429 + assert ei.value.retry_after == 120.0 + + def test_request_sleep_paces_before_fetch(monkeypatch): client = PatreonClient(cookies_path=None, request_sleep=1.5) monkeypatch.setattr(