From ebe6ab9741ab091533f4b543f413c62aaf461713 Mon Sep 17 00:00:00 2001
From: Bryan Van Deusen <bryan.vandeusen@fabledsword.com>
Date: Wed, 17 Jun 2026 11:40:36 -0400
Subject: [PATCH] refactor(native-ingest): shared exception trio + base
 _failure_result (#899 DRY 2/3)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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>
---
 backend/app/services/ingest_core.py           | 44 +++++++++++++--
 backend/app/services/native_ingest_common.py  | 35 ++++++++++++
 backend/app/services/patreon_client.py        | 31 +++--------
 backend/app/services/patreon_ingester.py      | 55 ++-----------------
 backend/app/services/subscribestar_client.py  | 25 +++------
 .../app/services/subscribestar_ingester.py    | 44 ++-------------
 6 files changed, 98 insertions(+), 136 deletions(-)

diff --git a/backend/app/services/ingest_core.py b/backend/app/services/ingest_core.py
index ba628f7..5cfaf5c 100644
--- a/backend/app/services/ingest_core.py
+++ b/backend/app/services/ingest_core.py
@@ -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) --------------------------------
 
diff --git a/backend/app/services/native_ingest_common.py b/backend/app/services/native_ingest_common.py
index 4317e9a..e0c9bda 100644
--- a/backend/app/services/native_ingest_common.py
+++ b/backend/app/services/native_ingest_common.py
@@ -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(
diff --git a/backend/app/services/patreon_client.py b/backend/app/services/patreon_client.py
index b5264b7..c9cd6a1 100644
--- a/backend/app/services/patreon_client.py
+++ b/backend/app/services/patreon_client.py
@@ -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
diff --git a/backend/app/services/patreon_ingester.py b/backend/app/services/patreon_ingester.py
index c4e8d73..5acc086 100644
--- a/backend/app/services/patreon_ingester.py
+++ b/backend/app/services/patreon_ingester.py
@@ -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,
diff --git a/backend/app/services/subscribestar_client.py b/backend/app/services/subscribestar_client.py
index 2565089..fad3dc2 100644
--- a/backend/app/services/subscribestar_client.py
+++ b/backend/app/services/subscribestar_client.py
@@ -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
diff --git a/backend/app/services/subscribestar_ingester.py b/backend/app/services/subscribestar_ingester.py
index e28d1d0..26138b5 100644
--- a/backend/app/services/subscribestar_ingester.py
+++ b/backend/app/services/subscribestar_ingester.py
@@ -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,