refactor(native-ingest): shared exception trio + base _failure_result (#899 DRY 2/3)
DRY pass commit 2. The two adapters re-implemented the same auth→drift→429→404
→http→network mapping in _failure_result; only the exception classes + drift
phrasing differed (divergence-bug risk: a new error_type handled in one and not
the other).
- native_ingest_common gains NativeIngestError / NativeAuthError / NativeDriftError
(status_code + retry_after on the base). Patreon{API,Auth,Drift}Error and
SubscribeStar{API,Auth,Drift}Error now subclass them via multiple inheritance,
keeping their isinstance-distinct platform names.
- Ingester._failure_result (base) does the whole mapping via the shared
NativeAuthError/NativeDriftError taxonomy + status_code; a new platform gets it
free. New drift_label kwarg supplies the per-platform API_DRIFT phrasing
("Patreon API" / "SubscribeStar markup"), preserving the existing message
(test asserts "Patreon API changed").
- Both adapters drop their near-identical _failure_result overrides and their now
-unused DownloadResult/ErrorType/*Auth/*Drift imports.
Verified at every consumer (rule 93/§8b): test_patreon_ingester (auth/drift/429/
404/network) and test_subscribestar_native (_failure_result mapping) both exercise
the base method now. Remaining: ingest_core L1/L3 logging (3/3).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
@@ -36,6 +36,7 @@ from sqlalchemy import delete, func, select, text
|
||||
from sqlalchemy.dialects.postgresql import insert as pg_insert
|
||||
|
||||
from .gallery_dl import DownloadResult, ErrorType, make_run_stats
|
||||
from .native_ingest_common import NativeAuthError, NativeDriftError
|
||||
|
||||
log = logging.getLogger(__name__)
|
||||
|
||||
@@ -88,6 +89,7 @@ class Ingester:
|
||||
ledger_key: Callable[[object], str],
|
||||
platform: str,
|
||||
error_base: type[Exception],
|
||||
drift_label: str | None = None,
|
||||
):
|
||||
self.client = client
|
||||
self.downloader = downloader
|
||||
@@ -99,6 +101,10 @@ class Ingester:
|
||||
self._ledger_key = ledger_key
|
||||
self._platform = platform
|
||||
self._error_base = error_base
|
||||
# Human label for the API_DRIFT message ("<label> changed — ingester needs
|
||||
# update"). Defaults to the platform name; adapters pass a richer phrase
|
||||
# (e.g. "Patreon API", "SubscribeStar markup").
|
||||
self._drift_label = drift_label or platform
|
||||
|
||||
# -- public ------------------------------------------------------------
|
||||
|
||||
@@ -601,13 +607,41 @@ class Ingester:
|
||||
# -- failure mapping (adapter overrides) -------------------------------
|
||||
|
||||
def _failure_result(self, exc: Exception, _result) -> DownloadResult:
|
||||
"""Map a platform client-error to a typed failed DownloadResult. The base
|
||||
gives a safe default; adapters override with their exception taxonomy."""
|
||||
log.warning("%s ingest failed: %s", self._platform, exc)
|
||||
return _result(
|
||||
"""Map a platform client-error to a loud, typed failed DownloadResult —
|
||||
NEVER a silent zero-download "success". The mapping is shared across
|
||||
platforms via the NativeAuthError/NativeDriftError taxonomy (the platform
|
||||
client raises subclasses), so a new platform gets it for free:
|
||||
- NativeAuthError → AUTH_ERROR (rotate the credential)
|
||||
- NativeDriftError → API_DRIFT (the ingester/scraper needs updating)
|
||||
- HTTP 429 / 404 → RATE_LIMITED / NOT_FOUND
|
||||
- other HTTP status→ HTTP_ERROR; transport failure → NETWORK_ERROR
|
||||
Auth/Drift are matched first (they also carry a status_code in some paths).
|
||||
"""
|
||||
message = str(exc)
|
||||
if isinstance(exc, NativeAuthError):
|
||||
error_type = ErrorType.AUTH_ERROR
|
||||
elif isinstance(exc, NativeDriftError):
|
||||
error_type = ErrorType.API_DRIFT
|
||||
message = f"{self._drift_label} changed — ingester needs update: {message}"
|
||||
else:
|
||||
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("%s ingest failed (%s): %s", self._platform, error_type.value, message)
|
||||
result = _result(
|
||||
success=False, return_code=1,
|
||||
error_type=ErrorType.UNKNOWN_ERROR, error_message=str(exc),
|
||||
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
|
||||
|
||||
# -- seen-ledger (short-lived sessions) --------------------------------
|
||||
|
||||
|
||||
@@ -57,6 +57,41 @@ _TITLE_MAX = 40
|
||||
_FORBIDDEN = set('<>:"/\\|?*')
|
||||
|
||||
|
||||
# -- shared exception taxonomy --------------------------------------------
|
||||
# Every native client raises one of these (platform subclasses keep an
|
||||
# isinstance-distinct platform name AND the semantic Auth/Drift class), so the
|
||||
# base Ingester._failure_result can map them platform-agnostically.
|
||||
|
||||
class NativeIngestError(Exception):
|
||||
"""Base for a native-ingest client failure. `status_code` carries the HTTP
|
||||
status when the failure was an HTTP response (None for transport/parse);
|
||||
`retry_after` carries the server's 429 Retry-After hint so the cooldown can
|
||||
match it (plan #708 B1)."""
|
||||
|
||||
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 NativeAuthError(NativeIngestError):
|
||||
"""Authentication/authorization failure — expired/missing credential or an
|
||||
insufficient tier. The fix is rotating the credential, NOT updating the
|
||||
ingester. Maps to error_type 'auth_error'."""
|
||||
|
||||
|
||||
class NativeDriftError(NativeIngestError):
|
||||
"""A response did not match the shape the ingester depends on (JSON:API field
|
||||
set, or scraped HTML structure). Fail loud so the import step flags 'the
|
||||
platform changed' instead of silently importing nothing. Maps to API_DRIFT."""
|
||||
|
||||
|
||||
# -- HTTP session ----------------------------------------------------------
|
||||
|
||||
def make_session(
|
||||
|
||||
@@ -41,6 +41,9 @@ from ..utils.paths import filehash_from_url
|
||||
from ..utils.prosemirror import post_body_html
|
||||
from .native_ingest_common import (
|
||||
_MAX_429_RETRIES,
|
||||
NativeAuthError,
|
||||
NativeDriftError,
|
||||
NativeIngestError,
|
||||
basename_from_url,
|
||||
make_session,
|
||||
retry_after_seconds,
|
||||
@@ -73,30 +76,12 @@ _FIELDS_CAMPAIGN = "name,url"
|
||||
_CONTENT_IMG_RE = re.compile(r"<img\b[^>]*?\bsrc=[\"']([^\"']+)[\"']", re.IGNORECASE)
|
||||
|
||||
|
||||
class PatreonAPIError(Exception):
|
||||
"""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, …).
|
||||
`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,
|
||||
retry_after: float | None = None,
|
||||
):
|
||||
super().__init__(message)
|
||||
self.status_code = status_code
|
||||
self.retry_after = retry_after
|
||||
class PatreonAPIError(NativeIngestError):
|
||||
"""Base for native Patreon client failures. status_code / retry_after are
|
||||
inherited from NativeIngestError (HTTP status; 429 Retry-After hint)."""
|
||||
|
||||
|
||||
class PatreonAuthError(PatreonAPIError):
|
||||
class PatreonAuthError(PatreonAPIError, NativeAuthError):
|
||||
"""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
|
||||
@@ -104,7 +89,7 @@ class PatreonAuthError(PatreonAPIError):
|
||||
"""
|
||||
|
||||
|
||||
class PatreonDriftError(PatreonAPIError):
|
||||
class PatreonDriftError(PatreonAPIError, NativeDriftError):
|
||||
"""A JSON response did not match the JSON:API shape we depend on.
|
||||
|
||||
Raised for: a missing top-level `data` list, `data` not a list, or a media
|
||||
|
||||
@@ -35,15 +35,8 @@ from collections.abc import Callable
|
||||
from pathlib import Path
|
||||
|
||||
from ..models import PatreonFailedMedia, PatreonSeenMedia
|
||||
from .gallery_dl import DownloadResult, ErrorType
|
||||
from .ingest_core import DEAD_LETTER_THRESHOLD, Ingester
|
||||
from .patreon_client import (
|
||||
MediaItem,
|
||||
PatreonAPIError,
|
||||
PatreonAuthError,
|
||||
PatreonClient,
|
||||
PatreonDriftError,
|
||||
)
|
||||
from .patreon_client import MediaItem, PatreonAPIError, PatreonClient
|
||||
from .patreon_downloader import PatreonDownloader
|
||||
from .patreon_resolver import extract_vanity, resolve_campaign_id_for_source
|
||||
|
||||
@@ -129,51 +122,11 @@ class PatreonIngester(Ingester):
|
||||
ledger_key=_ledger_key,
|
||||
platform="patreon",
|
||||
error_base=PatreonAPIError,
|
||||
# API_DRIFT message phrasing; the base Ingester._failure_result owns
|
||||
# the auth/drift/HTTP→error_type mapping now (shared across platforms).
|
||||
drift_label="Patreon API",
|
||||
)
|
||||
|
||||
# -- failure mapping (Patreon exception taxonomy) ----------------------
|
||||
|
||||
def _failure_result(self, exc: Exception, _result) -> DownloadResult:
|
||||
"""Map a client-level exception to a loud, typed failed DownloadResult.
|
||||
|
||||
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)
|
||||
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: # 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)
|
||||
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(
|
||||
url: str,
|
||||
|
||||
@@ -43,6 +43,9 @@ import requests
|
||||
from ..utils.paths import filehash_from_url
|
||||
from .native_ingest_common import (
|
||||
_MAX_429_RETRIES,
|
||||
NativeAuthError,
|
||||
NativeDriftError,
|
||||
NativeIngestError,
|
||||
basename_from_url,
|
||||
make_session,
|
||||
retry_after_seconds,
|
||||
@@ -76,30 +79,18 @@ _NEXT_PAGE_RE = re.compile(
|
||||
_LOGIN_MARKERS = ("/session/new", 'data-role="sign_in"', "age_confirmation_warning")
|
||||
|
||||
|
||||
class SubscribeStarAPIError(Exception):
|
||||
"""Base for native SubscribeStar client failures. `status_code` carries the
|
||||
HTTP status when the failure was an HTTP response (None for transport/parse),
|
||||
so the ingester can map it to a DownloadResult.error_type."""
|
||||
|
||||
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 SubscribeStarAPIError(NativeIngestError):
|
||||
"""Base for native SubscribeStar client failures. status_code / retry_after
|
||||
are inherited from NativeIngestError."""
|
||||
|
||||
|
||||
class SubscribeStarAuthError(SubscribeStarAPIError):
|
||||
class SubscribeStarAuthError(SubscribeStarAPIError, NativeAuthError):
|
||||
"""Auth/authorization failure — expired cookies, missing age cookie, or an
|
||||
HTML login/age wall served where the feed was expected. Fix = rotate the
|
||||
credential, not update the scraper. Maps to error_type 'auth_error'."""
|
||||
|
||||
|
||||
class SubscribeStarDriftError(SubscribeStarAPIError):
|
||||
class SubscribeStarDriftError(SubscribeStarAPIError, NativeDriftError):
|
||||
"""The feed HTML did not match the structure we scrape (no recognizable post
|
||||
blocks AND no known empty-feed state). The scrape analog of API drift — fail
|
||||
loud so the import step flags 'SubscribeStar changed its markup' instead of
|
||||
|
||||
@@ -21,15 +21,8 @@ from collections.abc import Callable
|
||||
from pathlib import Path
|
||||
|
||||
from ..models import SubscribeStarFailedMedia, SubscribeStarSeenMedia
|
||||
from .gallery_dl import DownloadResult, ErrorType
|
||||
from .ingest_core import DEAD_LETTER_THRESHOLD, Ingester
|
||||
from .subscribestar_client import (
|
||||
MediaItem,
|
||||
SubscribeStarAPIError,
|
||||
SubscribeStarAuthError,
|
||||
SubscribeStarClient,
|
||||
SubscribeStarDriftError,
|
||||
)
|
||||
from .subscribestar_client import MediaItem, SubscribeStarAPIError, SubscribeStarClient
|
||||
from .subscribestar_downloader import SubscribeStarDownloader
|
||||
|
||||
__all__ = [
|
||||
@@ -96,40 +89,11 @@ class SubscribeStarIngester(Ingester):
|
||||
ledger_key=_ledger_key,
|
||||
platform="subscribestar",
|
||||
error_base=SubscribeStarAPIError,
|
||||
# API_DRIFT message phrasing; the base Ingester._failure_result owns
|
||||
# the auth/drift/HTTP→error_type mapping (shared across platforms).
|
||||
drift_label="SubscribeStar markup",
|
||||
)
|
||||
|
||||
# -- failure mapping (SubscribeStar exception taxonomy) ----------------
|
||||
|
||||
def _failure_result(self, exc: Exception, _result) -> DownloadResult:
|
||||
"""Map a client-level exception to a loud, typed failed DownloadResult —
|
||||
never a silent zero-download "success". SubscribeStarAuthError and
|
||||
SubscribeStarDriftError both subclass SubscribeStarAPIError, so match them
|
||||
before the generic HTTP/transport fallthrough."""
|
||||
message = str(exc)
|
||||
if isinstance(exc, SubscribeStarAuthError):
|
||||
error_type = ErrorType.AUTH_ERROR
|
||||
elif isinstance(exc, SubscribeStarDriftError):
|
||||
error_type = ErrorType.API_DRIFT
|
||||
message = f"SubscribeStar markup changed — scraper needs update: {message}"
|
||||
else:
|
||||
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("SubscribeStar ingest failed (%s): %s", error_type.value, message)
|
||||
result = _result(
|
||||
success=False, return_code=1,
|
||||
error_type=error_type, error_message=message,
|
||||
)
|
||||
if error_type == ErrorType.RATE_LIMITED:
|
||||
result.retry_after_seconds = getattr(exc, "retry_after", None)
|
||||
return result
|
||||
|
||||
|
||||
async def verify_subscribestar_credential(
|
||||
url: str,
|
||||
|
||||
Reference in New Issue
Block a user