feat(patreon): drift detection + error categorization — build step 4 (plan #697)
Typed, loud failure mapping for the native Patreon ingester so a changed
API shape or expired auth never silently zero-downloads as "success".
- New ErrorType.API_DRIFT (free varchar error_type col → no migration):
distinct from auth so the operator knows the fix is updating the
ingester, not rotating cookies.
- patreon_client: PatreonAPIError carries status_code; new PatreonAuthError
for 401/403 + HTML-login/non-JSON bodies (reclassified from drift —
expired-session is auth, actionable as "rotate cookies").
- patreon_ingester._failure_result maps: PatreonAuthError→AUTH_ERROR,
PatreonDriftError→API_DRIFT ("Patreon API changed — ingester needs
update"), HTTP 429→RATE_LIMITED, 404→NOT_FOUND, other HTTP→HTTP_ERROR,
transport→NETWORK_ERROR. (429 thus drives the platform cooldown.)
- FailingSourcesCard: api_drift chip (red) + hint.
Contract test (new test_patreon_contract.py): the recorded /api/posts
fixture must parse end-to-end (no drift, 5 media across 4 posts) AND the
request params must still carry every field the parser depends on
(file_name, image_urls/download_url, images/attachments_media/media
includes, content/post_file/image post fields) — a trim of either trips a
red build. Plus client HTTP-status classification tests and ingester
error-type mapping tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -40,6 +40,12 @@ class ErrorType(StrEnum):
|
||||
HTTP_ERROR = "http_error"
|
||||
UNSUPPORTED_URL = "unsupported_url"
|
||||
VALIDATION_FAILED = "validation_failed"
|
||||
# Native Patreon ingester only (plan #697): a response parsed as JSON but
|
||||
# didn't match the JSON:API shape the ingester depends on (missing `data`,
|
||||
# media lacking `file_name`/`url`, etc.). Distinct from AUTH_ERROR — the fix
|
||||
# is updating the ingester's field-set/parser, not rotating credentials. The
|
||||
# contract test guards against silently shipping this.
|
||||
API_DRIFT = "api_drift"
|
||||
# Run made real progress (downloaded ≥1 file) but did not finish in the
|
||||
# subprocess budget. Distinct from UNKNOWN_ERROR — the downstream status
|
||||
# mapping classifies this as "ok" because the next tick continues.
|
||||
|
||||
@@ -77,16 +77,34 @@ _MAX_EXT_LEN = 16
|
||||
|
||||
|
||||
class PatreonAPIError(Exception):
|
||||
"""Base for native Patreon client failures."""
|
||||
"""Base for native Patreon client failures.
|
||||
|
||||
`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, …).
|
||||
"""
|
||||
|
||||
def __init__(self, message: str, *, status_code: int | None = None):
|
||||
super().__init__(message)
|
||||
self.status_code = status_code
|
||||
|
||||
|
||||
class PatreonAuthError(PatreonAPIError):
|
||||
"""Authentication / authorization failure — missing or expired session
|
||||
cookies, an insufficient pledge tier, or an HTML login/challenge page served
|
||||
where JSON was expected. DISTINCT from drift: the fix is rotating the
|
||||
credential, not updating the ingester. Maps to error_type 'auth_error'.
|
||||
"""
|
||||
|
||||
|
||||
class PatreonDriftError(PatreonAPIError):
|
||||
"""A response did not match the JSON:API shape we depend on.
|
||||
"""A JSON response did not match the JSON:API shape we depend on.
|
||||
|
||||
Raised for: a non-JSON / HTML-login response where JSON was expected, a
|
||||
missing top-level `data` list, or a media resource lacking the
|
||||
`file_name`/`url` fields we resolve against. Fail loud so the later import
|
||||
step flags API drift instead of silently importing nothing.
|
||||
Raised for: a missing top-level `data` list, `data` not a list, or a media
|
||||
resource lacking the `file_name`/`url` fields we resolve against. Fail loud
|
||||
so the import step flags API drift (ingester needs update) instead of
|
||||
silently importing nothing. An HTML-login / non-JSON body is auth, not
|
||||
drift — that raises PatreonAuthError.
|
||||
"""
|
||||
|
||||
|
||||
@@ -221,20 +239,32 @@ class PatreonClient:
|
||||
f"Patreon posts request failed (campaign_id={campaign_id}): {exc}"
|
||||
) from exc
|
||||
|
||||
if resp.status_code in (401, 403):
|
||||
# Auth rejected — expired/missing cookies or an insufficient tier.
|
||||
# Actionable as "rotate credentials", so it's auth, not drift/http.
|
||||
raise PatreonAuthError(
|
||||
f"Patreon posts API returned HTTP {resp.status_code} — auth "
|
||||
f"rejected (cookies expired or tier insufficient; "
|
||||
f"campaign_id={campaign_id})",
|
||||
status_code=resp.status_code,
|
||||
)
|
||||
if resp.status_code != 200:
|
||||
raise PatreonAPIError(
|
||||
f"Patreon posts API returned HTTP {resp.status_code} "
|
||||
f"(campaign_id={campaign_id})"
|
||||
f"(campaign_id={campaign_id})",
|
||||
status_code=resp.status_code,
|
||||
)
|
||||
try:
|
||||
payload = resp.json()
|
||||
except ValueError as exc:
|
||||
# A non-JSON body here is almost always the HTML login/challenge
|
||||
# page served when cookies are missing/expired — that is drift
|
||||
# from the JSON:API contract, not a transient network error.
|
||||
raise PatreonDriftError(
|
||||
# page served when cookies are missing/expired — that is an AUTH
|
||||
# failure (rotate cookies), not API drift (update the ingester) and
|
||||
# not a transient network error.
|
||||
raise PatreonAuthError(
|
||||
"Patreon posts API returned a non-JSON response (likely an "
|
||||
f"HTML login/challenge page; campaign_id={campaign_id}): {exc}"
|
||||
f"HTML login/challenge page — session expired; "
|
||||
f"campaign_id={campaign_id}): {exc}"
|
||||
) from exc
|
||||
return payload
|
||||
|
||||
|
||||
@@ -51,6 +51,7 @@ from .gallery_dl import DownloadResult, ErrorType
|
||||
from .patreon_client import (
|
||||
MediaItem,
|
||||
PatreonAPIError,
|
||||
PatreonAuthError,
|
||||
PatreonClient,
|
||||
PatreonDriftError,
|
||||
)
|
||||
@@ -242,7 +243,9 @@ class PatreonIngester:
|
||||
break
|
||||
else:
|
||||
reached_bottom = True
|
||||
except (PatreonDriftError, PatreonAPIError) as exc:
|
||||
except PatreonAPIError as exc:
|
||||
# Base of PatreonAuthError + PatreonDriftError — catches every
|
||||
# client-level failure; _failure_result maps it to a typed error.
|
||||
return self._failure_result(exc, _result)
|
||||
|
||||
if errors:
|
||||
@@ -294,24 +297,35 @@ class PatreonIngester:
|
||||
def _failure_result(self, exc: Exception, _result) -> DownloadResult:
|
||||
"""Map a client-level exception to a loud, typed failed DownloadResult.
|
||||
|
||||
Step 3 keeps this deliberately coarse — auth-ish drift vs. everything
|
||||
else — so the integration is correct end-to-end. Step 4 (drift detection
|
||||
+ error categorization) refines the typed-error mapping and adds the
|
||||
contract test. Either way we NEVER return a silent zero-download
|
||||
"success": the whole point of the native ingester is to fail red when
|
||||
Patreon's API shape or our auth changes.
|
||||
We NEVER return a silent zero-download "success" — the whole point of the
|
||||
native ingester is to fail RED when Patreon's API shape or our auth
|
||||
changes. The typed mapping lets FailingSourcesCard render the right chip
|
||||
and tells the operator what to do:
|
||||
- PatreonAuthError → AUTH_ERROR (rotate cookies)
|
||||
- PatreonDriftError → API_DRIFT (ingester field-set/parser needs update)
|
||||
- HTTP 429 / 404 → RATE_LIMITED / NOT_FOUND
|
||||
- other HTTP status → HTTP_ERROR; transport failure → NETWORK_ERROR
|
||||
|
||||
PatreonAuthError and PatreonDriftError both subclass PatreonAPIError, so
|
||||
they must be matched before the generic HTTP/transport fallthrough.
|
||||
"""
|
||||
message = str(exc)
|
||||
lowered = message.lower()
|
||||
if isinstance(exc, PatreonDriftError):
|
||||
if "login" in lowered or "challenge" in lowered or "auth" in lowered:
|
||||
error_type = ErrorType.AUTH_ERROR
|
||||
else:
|
||||
error_type = ErrorType.UNKNOWN_ERROR
|
||||
if isinstance(exc, PatreonAuthError):
|
||||
error_type = ErrorType.AUTH_ERROR
|
||||
elif isinstance(exc, PatreonDriftError):
|
||||
error_type = ErrorType.API_DRIFT
|
||||
message = f"Patreon API changed — ingester needs update: {message}"
|
||||
else:
|
||||
error_type = ErrorType.NETWORK_ERROR
|
||||
log.warning("Patreon ingest failed: %s", message)
|
||||
else: # generic PatreonAPIError: HTTP non-2xx (status_code set) or transport
|
||||
status = getattr(exc, "status_code", None)
|
||||
if status == 429:
|
||||
error_type = ErrorType.RATE_LIMITED
|
||||
elif status == 404:
|
||||
error_type = ErrorType.NOT_FOUND
|
||||
elif status is not None:
|
||||
error_type = ErrorType.HTTP_ERROR
|
||||
else:
|
||||
error_type = ErrorType.NETWORK_ERROR
|
||||
log.warning("Patreon ingest failed (%s): %s", error_type.value, message)
|
||||
return _result(
|
||||
success=False, return_code=1,
|
||||
error_type=error_type, error_message=message,
|
||||
|
||||
@@ -93,6 +93,7 @@ const ERROR_TYPE_COLOR = {
|
||||
unsupported_url: 'error',
|
||||
http_error: 'error',
|
||||
unknown_error: 'error',
|
||||
api_drift: 'error',
|
||||
partial: 'info',
|
||||
tier_limited: 'info',
|
||||
no_new_content: 'info',
|
||||
@@ -108,6 +109,7 @@ const ERROR_TYPE_HINT = {
|
||||
http_error: 'Generic HTTP error — see Logs.',
|
||||
unsupported_url: 'gallery-dl does not support this URL pattern.',
|
||||
unknown_error: 'Could not classify — see Logs.',
|
||||
api_drift: 'Patreon changed its API shape — the native ingester needs a code update.',
|
||||
}
|
||||
function errorTypeColor(t) { return ERROR_TYPE_COLOR[t] || 'error' }
|
||||
function errorTypeHint(t) { return ERROR_TYPE_HINT[t] || '' }
|
||||
|
||||
@@ -12,6 +12,8 @@ import pytest
|
||||
|
||||
from backend.app.services.patreon_client import (
|
||||
MediaItem,
|
||||
PatreonAPIError,
|
||||
PatreonAuthError,
|
||||
PatreonClient,
|
||||
PatreonDriftError,
|
||||
parse_cursor_from_url,
|
||||
@@ -184,3 +186,55 @@ def test_media_referenced_but_absent_raises_drift(client):
|
||||
}
|
||||
with pytest.raises(PatreonDriftError):
|
||||
client.extract_media(post, {})
|
||||
|
||||
|
||||
# -- HTTP status classification (plan #697 step 4) -------------------------
|
||||
|
||||
|
||||
class _FakeResp:
|
||||
def __init__(self, status_code, *, json_data=None, raise_json=False):
|
||||
self.status_code = status_code
|
||||
self._json_data = json_data
|
||||
self._raise_json = raise_json
|
||||
|
||||
def json(self):
|
||||
if self._raise_json:
|
||||
raise ValueError("Expecting value: line 1 column 1")
|
||||
return self._json_data
|
||||
|
||||
|
||||
def _client_returning(monkeypatch, resp):
|
||||
client = PatreonClient(cookies_path=None)
|
||||
monkeypatch.setattr(client._session, "get", lambda *a, **k: resp)
|
||||
return client
|
||||
|
||||
|
||||
@pytest.mark.parametrize("status", [401, 403])
|
||||
def test_fetch_auth_status_raises_auth_error(monkeypatch, status):
|
||||
client = _client_returning(monkeypatch, _FakeResp(status))
|
||||
with pytest.raises(PatreonAuthError) as ei:
|
||||
client._fetch("5555", None)
|
||||
assert ei.value.status_code == status
|
||||
|
||||
|
||||
def test_fetch_non_json_body_is_auth_not_drift(monkeypatch):
|
||||
# An HTML login/challenge page (non-JSON) means the session expired — auth,
|
||||
# actionable as "rotate cookies", NOT API drift.
|
||||
client = _client_returning(monkeypatch, _FakeResp(200, raise_json=True))
|
||||
with pytest.raises(PatreonAuthError):
|
||||
client._fetch("5555", None)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("status", [404, 429, 500])
|
||||
def test_fetch_other_status_raises_api_error_with_code(monkeypatch, status):
|
||||
client = _client_returning(monkeypatch, _FakeResp(status))
|
||||
with pytest.raises(PatreonAPIError) as ei:
|
||||
client._fetch("5555", None)
|
||||
assert ei.value.status_code == status
|
||||
# Not auth — the ingester maps these by status, not to auth_error.
|
||||
assert not isinstance(ei.value, PatreonAuthError)
|
||||
|
||||
|
||||
def test_fetch_ok_returns_payload(monkeypatch):
|
||||
client = _client_returning(monkeypatch, _FakeResp(200, json_data={"data": []}))
|
||||
assert client._fetch("5555", None) == {"data": []}
|
||||
|
||||
@@ -0,0 +1,72 @@
|
||||
"""Patreon API contract test (native ingester build step 4).
|
||||
|
||||
The drift tripwire the plan calls for: if the field-set we SEND or the parser we
|
||||
resolve responses against drifts from what real Patreon returns, this build goes
|
||||
RED — instead of the ingester silently importing nothing in production (the exact
|
||||
failure mode the native ingester exists to escape).
|
||||
|
||||
Two halves:
|
||||
- the recorded `/api/posts` fixture must still parse end-to-end (no drift), and
|
||||
- the request params must still carry every field the parser depends on (the
|
||||
fixture already has the data, so trimming the request would NOT trip the
|
||||
parse half — this half guards the request shape directly).
|
||||
|
||||
Pure: no network, no DB.
|
||||
"""
|
||||
|
||||
import json
|
||||
from pathlib import Path
|
||||
|
||||
from backend.app.services.patreon_client import MediaItem, PatreonClient
|
||||
|
||||
_FIXTURE = Path(__file__).parent / "fixtures" / "patreon_posts_page1.json"
|
||||
|
||||
# Pinned media total across the 4 fixture posts: 1001→2 (gallery, image_large
|
||||
# cover deduped), 1002→1 (attachment+postfile collapse), 1003→1 (inline content
|
||||
# img), 1004→1 (video postfile). A parser change that drops or doubles media
|
||||
# trips this.
|
||||
_EXPECTED_MEDIA_TOTAL = 5
|
||||
_KNOWN_KINDS = {"images", "image_large", "attachments", "postfile", "content"}
|
||||
|
||||
|
||||
def test_recorded_fixture_parses_end_to_end():
|
||||
response = json.loads(_FIXTURE.read_text())
|
||||
client = PatreonClient(cookies_path=None)
|
||||
|
||||
client._validate_response(response) # top-level shape must hold
|
||||
index = client._transform(response)
|
||||
|
||||
total = 0
|
||||
for post in response["data"]:
|
||||
items = client.extract_media(post, index) # must not raise drift
|
||||
for m in items:
|
||||
assert isinstance(m, MediaItem)
|
||||
assert m.url, f"empty url in post {post['id']}"
|
||||
assert m.filename, f"empty filename in post {post['id']}"
|
||||
assert m.kind in _KNOWN_KINDS, f"unknown kind {m.kind!r}"
|
||||
assert m.post_id == post["id"]
|
||||
total += len(items)
|
||||
|
||||
assert total == _EXPECTED_MEDIA_TOTAL
|
||||
|
||||
|
||||
def test_request_field_set_carries_parser_dependencies():
|
||||
"""The params we SEND must keep requesting the fields the parser resolves
|
||||
against. Trimming `fields[media]=file_name`, an `include` relationship, or a
|
||||
`fields[post]` key would make real responses parse to nothing — caught here,
|
||||
not by the fixture parse above."""
|
||||
params = PatreonClient(cookies_path=None)._params("5555", None)
|
||||
|
||||
media_fields = params["fields[media]"]
|
||||
assert "file_name" in media_fields
|
||||
assert "image_urls" in media_fields or "download_url" in media_fields
|
||||
|
||||
includes = params["include"]
|
||||
for rel in ("images", "attachments_media", "media"):
|
||||
assert rel in includes, f"missing include relationship {rel}"
|
||||
|
||||
post_fields = params["fields[post]"]
|
||||
for field in ("content", "post_file", "image"):
|
||||
assert field in post_fields, f"missing post field {field}"
|
||||
|
||||
assert params["filter[campaign_id]"] == "5555"
|
||||
@@ -13,7 +13,12 @@ from sqlalchemy.orm import sessionmaker
|
||||
|
||||
from backend.app.models import Artist, PatreonSeenMedia, Source
|
||||
from backend.app.services.gallery_dl import ErrorType
|
||||
from backend.app.services.patreon_client import MediaItem, PatreonDriftError
|
||||
from backend.app.services.patreon_client import (
|
||||
MediaItem,
|
||||
PatreonAPIError,
|
||||
PatreonAuthError,
|
||||
PatreonDriftError,
|
||||
)
|
||||
from backend.app.services.patreon_downloader import MediaOutcome
|
||||
from backend.app.services.patreon_ingester import PatreonIngester, _ledger_key
|
||||
|
||||
@@ -302,20 +307,63 @@ async def test_mark_seen_is_idempotent(source_id, sync_engine, tmp_path):
|
||||
assert _count_ledger(sync_engine, source_id) == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_drift_fails_loud(source_id, sync_engine, tmp_path):
|
||||
client = _FakeClient([], raise_exc=PatreonDriftError("missing top-level 'data' key"))
|
||||
downloader = _FakeDownloader(tmp_path)
|
||||
ing = _ingester(sync_engine, tmp_path, client, downloader)
|
||||
|
||||
result = ing.run(
|
||||
def _run_with_client_error(source_id, sync_engine, tmp_path, exc):
|
||||
client = _FakeClient([], raise_exc=exc)
|
||||
ing = _ingester(sync_engine, tmp_path, client, _FakeDownloader(tmp_path))
|
||||
return ing.run(
|
||||
source_id=source_id, campaign_id="c1", artist_slug="ingest",
|
||||
url="https://patreon.com/ingest", mode="tick",
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_drift_maps_to_api_drift_and_fails_loud(source_id, sync_engine, tmp_path):
|
||||
result = _run_with_client_error(
|
||||
source_id, sync_engine, tmp_path,
|
||||
PatreonDriftError("missing top-level 'data' key"),
|
||||
)
|
||||
assert result.success is False
|
||||
assert result.error_type == ErrorType.API_DRIFT
|
||||
assert "Patreon API changed" in (result.error_message or "")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auth_error_maps_to_auth_error(source_id, sync_engine, tmp_path):
|
||||
result = _run_with_client_error(
|
||||
source_id, sync_engine, tmp_path,
|
||||
PatreonAuthError("HTTP 403 — auth rejected", status_code=403),
|
||||
)
|
||||
assert result.success is False
|
||||
assert result.error_type == ErrorType.AUTH_ERROR
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_rate_limit_status_maps_to_rate_limited(source_id, sync_engine, tmp_path):
|
||||
result = _run_with_client_error(
|
||||
source_id, sync_engine, tmp_path,
|
||||
PatreonAPIError("HTTP 429", status_code=429),
|
||||
)
|
||||
assert result.error_type == ErrorType.RATE_LIMITED
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_not_found_status_maps_to_not_found(source_id, sync_engine, tmp_path):
|
||||
result = _run_with_client_error(
|
||||
source_id, sync_engine, tmp_path,
|
||||
PatreonAPIError("HTTP 404", status_code=404),
|
||||
)
|
||||
assert result.error_type == ErrorType.NOT_FOUND
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_transport_error_maps_to_network_error(source_id, sync_engine, tmp_path):
|
||||
result = _run_with_client_error(
|
||||
source_id, sync_engine, tmp_path,
|
||||
PatreonAPIError("connection reset"), # no status_code → transport
|
||||
)
|
||||
assert result.error_type == ErrorType.NETWORK_ERROR
|
||||
|
||||
|
||||
def test_ledger_key_video_has_no_filehash():
|
||||
video = MediaItem(
|
||||
url="https://stream.mux.com/abc.m3u8", filename="clip.mp4",
|
||||
|
||||
Reference in New Issue
Block a user