feat(downloads): platform cooldown honors server Retry-After — B1 (plan #708)
Owning the native client means we see the 429 Retry-After header — previously discarded. PatreonAPIError now carries `retry_after`; on a PERSISTENT page-fetch 429 the client attaches the server's raw Retry-After seconds. New DownloadResult.retry_after_seconds; patreon_ingester._failure_result sets it on RATE_LIMITED. download_service._update_source_health passes it to set_platform_cooldown as `seconds=`, clamped to [60, 3600] (a tiny hint can't leave the platform effectively un-cooled; a huge one can't strand it for hours); no hint → the flat 900s default. So a rate-limited platform cools for as long as the server actually asks, not a fixed guess. Tests: terminal 429 surfaces retry_after (test_patreon_client); cooldown honors + clamps the hint, falls back to default when absent (test_download_service). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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()
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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(
|
||||
|
||||
Reference in New Issue
Block a user