From be2e0857e48bb8c6e0df97cd85927700d6972e53 Mon Sep 17 00:00:00 2001 From: Florian Date: Wed, 4 Nov 2026 13:02:53 +0100 Subject: [PATCH] - Moved Anilist errors to its own module and gave them status codes and messages that can propagate upstream - Created exception handler to propagate erros --- src/anilist_errors.py | 73 +++++++++++++++++ src/anilist_query.py | 177 +++++++++++++++++++++++++++++++----------- src/handlers.py | 65 ++++++++++++++++ src/main.py | 39 ++-------- src/models.py | 1 + 5 files changed, 280 insertions(+), 75 deletions(-) create mode 100644 src/anilist_errors.py create mode 100644 src/handlers.py diff --git a/src/anilist_errors.py b/src/anilist_errors.py new file mode 100644 index 0000000..41233f8 --- /dev/null +++ b/src/anilist_errors.py @@ -0,0 +1,73 @@ +from typing import ClassVar + + +class AniListError(Exception): + """Base class for every AniList error the API can explain to a client.""" + status_code: ClassVar[int] = 502 + code: ClassVar[str] = "sea_upstream_error" + client_visible: ClassVar[bool] = False + + @property + def client_message(self) -> str: + return str(self) if self.client_visible else "The Anilist entry could not be checked right now, please try again later" + @property + def headers(self) -> dict[str, str]: + """Extra HTTP headers the proxy should send with this error.""" + return {} + +class InvalidAnimeId(AniListError): + """The supplied AniList ID is not a positive integer.""" + status_code = 422 + code = "sea_invalid_anilist_id" + client_visible = True + + def __init__(self, anilist_id: object): + self.anilist_id = anilist_id + super().__init__(f"Invalid AniList ID: {anilist_id!r}") + + +class AnimeNotFound(AniListError): + """No anime exists on AniList for this ID.""" + status_code = 422 + code = "sea_anime_not_found" + client_visible = True + + def __init__(self, anilist_id: int | None): + self.anilist_id = anilist_id + super().__init__(f"Could not find the anime: {anilist_id}") + + +class MissingAnimeName(AniListError): + """The anime exists but has no usable title.""" + status_code = 422 + code = "sea_anime_missing_name" + client_visible = True + + def __init__(self, anilist_id: int): + self.anilist_id = anilist_id + super().__init__(f"Anime exists on AniList but has no name: {anilist_id}") + + +class AniListApiError(AniListError): + """Temporary or unexpected API problem (network, timeout, 5xx, bad response). + + Nothing is wrong with the AniList ID, so retry later. + """ + + def __init__(self, detail: str): + super().__init__(f"API error: {detail}") + + +class RateLimitedError(AniListError): + """AniList asked us to slow down. Stop the run instead of continuing.""" + status_code = 503 + code = "sea_rate_limited" + + def __init__(self, retry_after: int | None = None): + self.retry_after = retry_after + hint = f", retry after {retry_after}s" if retry_after is not None else "" + super().__init__(f"rate limited{hint}") + + @property + def headers(self) -> dict[str, str]: + return {"Retry-After": str(self.retry_after)} if self.retry_after is not None else {} \ No newline at end of file diff --git a/src/anilist_query.py b/src/anilist_query.py index 9634f2c..24967c4 100644 --- a/src/anilist_query.py +++ b/src/anilist_query.py @@ -1,12 +1,24 @@ -"""Look up anime on AniList by ID.""" +"""Look up anime on AniList by ID. + +This module wraps AniList's public GraphQL API (https://graphql.anilist.co) +and exposes one public function, :func:`get_anime_name`, which resolves an +AniList media ID to the best available display title. + +All failures are raised as subclasses of :class:`anilist_errors.AniListError` +so callers (and the proxy) can handle them in one place. Each error carries a +``status_code``, a stable ``code`` and a safe ``client_message``; internal +detail is logged here rather than exposed to clients. +""" + import requests +from anilist_errors import AniListApiError, AnimeNotFound, InvalidAnimeId, MissingAnimeName, RateLimitedError from simple_logger_handler import setup_logger logger = setup_logger(__name__) ANILIST_API_URL = "https://graphql.anilist.co" -DEFAULT_TIMEOUT = 10 # seconds +DEFAULT_TIMEOUT = 10 _ID_QUERY = """ query ($id: Int) { @@ -20,31 +32,50 @@ query ($id: Int) { # Preference order when picking a display name. _TITLE_PREFERENCE = ("english", "romaji", "native") - -class AniListError(RuntimeError): - """The AniList request failed (network, HTTP, or GraphQL error).""" - - -class AnimeNotFoundError(AniListError): - """No anime exists on AniList for this ID or name.""" - - -class MissingAnimeName(AniListError): - """The anime exists but has no usable title.""" - - def _post( query: str, variables: dict, session: requests.Session | None, timeout: float, ) -> dict: - """POST a GraphQL query and return its ``data`` dict. + """POST a GraphQL query to AniList and return the ``data`` part of the reply. + + This is the only function that talks to the network. It translates every + way a request can go wrong into a :class:`AniListError` subclass, in this + order: + + 1. Transport failure (DNS, connection, timeout, ...) -> ``AniListApiError`` + 2. HTTP 404 -> ``AnimeNotFound`` + 3. HTTP 429 -> ``RateLimitedError`` + 4. Other HTTP error status or body that isn't JSON -> ``AniListApiError`` + 5. JSON body that isn't an object -> ``AniListApiError`` + 6. GraphQL ``errors`` array present -> ``AniListApiError`` + 7. ``data`` present but not an object -> ``AniListApiError`` + + A missing or ``null`` ``data`` field is not an error here; it is returned + as an empty dict and the caller decides what that means. + + Args: + query: The GraphQL query string. + variables: Variables for the query. The ``"id"`` key, if present, is + used to identify the anime in a 404 error. + session: Optional ``requests.Session``. When ``None``, a one-off + ``requests.post`` is used. + timeout: Request timeout in seconds. + + Returns: + The ``data`` dict from the GraphQL response, or ``{}`` if AniList + returned no ``data``. Raises: - AnimeNotFoundError: AniList answered HTTP 404 (unknown ID). - AniListError: Network failure, rate limiting, or any other error. + AnimeNotFound: AniList answered HTTP 404 (unknown ID). + RateLimitedError: AniList answered HTTP 429. ``retry_after`` is set + when the ``Retry-After`` header was a valid integer. + AniListApiError: Network failure, non-2xx status, invalid or + unexpected JSON, or a GraphQL-level error. """ + + anime_id = variables.get("id") http = session or requests try: response = http.post( @@ -53,38 +84,69 @@ def _post( timeout=timeout, ) except requests.RequestException as e: - raise AniListError(f"Request to AniList failed: {type(e).__name__}") from e + raise AniListApiError(f"Request to AniList failed: {type(e).__name__}") from e if response.status_code == 404: - raise AnimeNotFoundError("Not found on AniList") + logger.warning("AniList returned 404 for id=%s", anime_id) + raise AnimeNotFound(variables.get("id")) if response.status_code == 429: retry_after = response.headers.get("Retry-After", "unknown") - raise AniListError(f"Rate limited by AniList (retry after {retry_after}s)") + logger.warning("Rate limited by AniList on id=%s (Retry-After=%s)", anime_id, retry_after,) + raise RateLimitedError(int(retry_after) if retry_after.isdigit() else None) try: response.raise_for_status() payload = response.json() except (requests.HTTPError, ValueError) as e: - raise AniListError(f"Bad response from AniList: {e}") from e + logger.warning("Bad response from AniList: %s", e) + raise AniListApiError(f"Bad response from AniList: {e}") from e + + if not isinstance(payload, dict): + logger.warning("Unexpected AniList payload type: %s", type(payload).__name__) + raise AniListApiError(f"Unexpected response shape: {type(payload).__name__}") if errors := payload.get("errors"): messages = "; ".join(str(err.get("message", err)) for err in errors) - raise AniListError(f"AniList returned errors: {messages}") + raise AniListApiError(f"AniList returned errors: {messages}") - return payload.get("data") or {} + data = payload.get("data") + if data is None: + logger.debug("AniList response for id=%s had no 'data'; returning empty dict", anime_id) + return {} + if not isinstance(data, dict): + logger.warning("Unexpected 'data' shape for id=%s: %s", anime_id, type(data).__name__,) + raise AniListApiError(f"Unexpected 'data' shape: {type(data).__name__}") + logger.debug("Parsed AniList data for id=%s: keys=%s", anime_id, list(data)) + return data def _pick_name(title: dict | None) -> str | None: - """Return the best available title (English, romaji, then native).""" + """Choose the best display name from an AniList ``title`` object. + + Tries each key in ``_TITLE_PREFERENCE`` (English, then romaji, then + native) and returns the first one that is non-empty after stripping + whitespace. Missing keys, ``None`` values and blank strings are skipped. + + Args: + title: The ``title`` object from AniList, e.g. + ``{"romaji": "...", "english": None, "native": "..."}``. + ``None`` is treated the same as an empty dict. + + Returns: + The chosen title, or ``None`` if no usable title exists. + """ title = title or {} for key in _TITLE_PREFERENCE: name = (title.get(key) or "").strip() if name: + logger.debug("Picked '%s' title: %r", key, name) return name + logger.debug("No usable title among keys %s in %s", _TITLE_PREFERENCE, title) return None + def get_anime_name( anilist_id: int, *, @@ -93,38 +155,65 @@ def get_anime_name( ) -> str: """Return the best available title for an AniList anime ID. - Prefers the English title, then romaji, then native. + Validates the ID, queries AniList, and picks a title in the order + English, romaji, native. Only media of type ``ANIME`` match; a manga or + other media ID is treated as not found. + + Every failure is raised as an :class:`AniListError` subclass, so a single + ``except AniListError`` is enough to catch them all. Errors with + ``client_visible = True`` are safe to show to end users; the rest carry + internal detail and should only be logged. Args: - anilist_id: AniList media ID (positive integer). + anilist_id: AniList media ID. Must be a positive ``int`` + (``bool`` is rejected even though it is an ``int`` subclass). session: Optional ``requests.Session`` to reuse connections when looking up many IDs. - timeout: Request timeout in seconds. + timeout: Request timeout in seconds. Defaults to ``DEFAULT_TIMEOUT``. + + Returns: + The anime's display title, stripped of surrounding whitespace. Raises: - ValueError: ``anilist_id`` is not a positive integer. - AnimeNotFoundError: No anime exists for this ID. - MissingAnimeName: The anime exists but has no title. - AniListError: Network failure, rate limiting, or any other - AniList/HTTP error. - """ - if not isinstance(anilist_id, int) or isinstance(anilist_id, bool) or anilist_id <= 0: - raise ValueError(f"anilist_id must be a positive integer, got {anilist_id!r}") + InvalidAnimeId: ``anilist_id`` is not a positive integer. No request + is made. Client-visible (422). + AnimeNotFound: No anime exists for this ID, either via HTTP 404 or + an empty ``Media`` result. Client-visible (422). + MissingAnimeName: The anime exists but none of its titles are usable. + Client-visible (422). + RateLimitedError: AniList rate limited the request (503). Callers + running in a loop should stop rather than keep going; see + ``retry_after`` on the exception. + AniListApiError: Network failure, timeout, 5xx, malformed response or + GraphQL error. Not client-visible (502); safe to retry later. - try: - data = _post(_ID_QUERY, {"id": anilist_id}, session, timeout) - except AnimeNotFoundError: - raise AnimeNotFoundError(f"No anime found for AniList ID={anilist_id}") from None + Example: + >>> get_anime_name(1) + 'Cowboy Bebop' + """ + + if not isinstance(anilist_id, int) or isinstance(anilist_id, bool) or anilist_id <= 0: + logger.warning("Rejected invalid anilist_id: %r", anilist_id) + raise InvalidAnimeId(anilist_id) + data = _post(_ID_QUERY, {"id": anilist_id}, session, timeout) media = data.get("Media") if not media: - raise AnimeNotFoundError(f"No anime found for AniList ID={anilist_id}") + logger.warning("AniList returned no Media for anilist_id=%s", anilist_id) + raise AnimeNotFound(anilist_id) name = _pick_name(media.get("title")) if name is None: - raise MissingAnimeName(f"Anime has no title for AniList ID={anilist_id}") + logger.warning("Anime anilist_id=%s exists but has no usable title", anilist_id) + raise MissingAnimeName(anilist_id) + + logger.info("Resolved anilist_id=%s to %r", anilist_id, name) return name if __name__ == "__main__": - asd = get_anime_name(2002927) - print(asd) \ No newline at end of file + from anilist_errors import AniListError + try: + print(get_anime_name(200927)) + print(get_anime_name(999999)) + except AniListError as e: + print(f"{e.code} ({e.status_code}): {e.client_message}") \ No newline at end of file diff --git a/src/handlers.py b/src/handlers.py new file mode 100644 index 0000000..db655e3 --- /dev/null +++ b/src/handlers.py @@ -0,0 +1,65 @@ +from fastapi import FastAPI, Request +from fastapi.exceptions import RequestValidationError +from fastapi.responses import JSONResponse +from starlette.middleware.base import BaseHTTPMiddleware + +from anilist_errors import AniListError +from metrics_server import REQUEST_COUNTER +from simple_logger_handler import setup_logger + +logger = setup_logger(__name__) + + +async def anilist_error_handler(request: Request, exc: AniListError): + log = logger.warning if exc.client_visible else logger.error + log(f"[FastAPI] {request.method} {request.url.path} failed: {exc}") + return JSONResponse( + status_code=exc.status_code, + content={"code": exc.code, "detail": exc.client_message}, + ) + +async def validation_error_handler(request: Request, exc: RequestValidationError): + errors = exc.errors() + + for err in errors: + original = (err.get("ctx") or {}).get("error") + if isinstance(original, AniListError): + return JSONResponse( + status_code=original.status_code, + content={"code": original.code, "detail": original.client_message}, + ) + + # Problems in headers, path or query are the calling service's fault, not the user's + if any(err["loc"][0] != "body" for err in errors): + logger.error(f"[FastAPI] Invalid request from a calling service: {[(e['loc'], e['msg']) for e in errors]}") + return JSONResponse(status_code=400, content={"code": "bad_request", "detail": "The request could not be processed"}) + + messages = [] + for err in errors: + field = ".".join(str(p) for p in err["loc"] if p != "body") + msg = err["msg"].removeprefix("Value error, ") + messages.append(f"{field}: {msg}" if field else msg) + return JSONResponse(status_code=422, content={"code": "validation_error", "detail": "; ".join(messages)}) + + +async def prometheus_middleware(request: Request, call_next): + logger.debug(f"[FastAPI] Incoming request: {request.method} {request.url.path}") + status = 500 + try: + response = await call_next(request) + status = response.status_code + logger.debug(f"[FastAPI] Request processed with status {status}") + except Exception as e: + logger.error(f"[FastAPI] Exception occurred: {e}", exc_info=True) + raise + finally: + REQUEST_COUNTER.labels(request.method, request.url.path, status).inc() + logger.debug(f"[FastAPI] Counter incremented for {request.method} {request.url.path} [{status}]") + return response + + +def register_handlers(app: FastAPI) -> None: + """Attach the error handlers and the metrics middleware to the app.""" + app.add_exception_handler(AniListError, anilist_error_handler) + app.add_exception_handler(RequestValidationError, validation_error_handler) + app.add_middleware(BaseHTTPMiddleware, dispatch=prometheus_middleware) diff --git a/src/main.py b/src/main.py index d8355ac..de55177 100644 --- a/src/main.py +++ b/src/main.py @@ -8,7 +8,7 @@ from fastapi import APIRouter, Depends, FastAPI, HTTPException, Request from seadex import EntryNotFoundError, SeaDexEntry from service_auth import UserId -from anilist_query import AniListError, AnimeNotFoundError, MissingAnimeName, get_anime_name +from anilist_query import get_anime_name from auth_config import Caller, auth from db_module import ( close_connection_pool, @@ -16,11 +16,12 @@ from db_module import ( get_db, start_healthcheck_thread, ) -from metrics_server import REQUEST_COUNTER, metrics_api +from handlers import register_handlers +from metrics_server import metrics_api +from models import AnimeCreate, AnimeEntry, AnimeList from secret_manager import cleanup_secret_files -from send_notifications import NotificationClient, NotificationUnavailableError, NotificationError +from send_notifications import NotificationClient, NotificationError, NotificationUnavailableError from simple_logger_handler import LOG_LEVEL, setup_logger -from models import AnimeCreate,AnimeEntry,AnimeList logger = setup_logger(__name__) @@ -58,26 +59,10 @@ api = FastAPI( dependencies=[Depends(auth.authenticate)], ) +register_handlers(api) user_router = APIRouter(dependencies=[Depends(auth.require(Caller.GATEWAY))]) scheduler_router = APIRouter(dependencies=[Depends(auth.require(Caller.SCHEDULER))]) -@api.middleware("http") -async def prometheus_middleware(request: Request, call_next): - logger.debug(f"[Metrics] Incoming request: {request.method} {request.url.path}") - status = 500 - try: - response = await call_next(request) - status = response.status_code - logger.debug(f"[Metrics] Request processed with status {status}") - except Exception: - logger.exception("[Metrics] Exception occurred:") - raise - finally: - REQUEST_COUNTER.labels(request.method, request.url.path, status).inc() - logger.debug(f"[Metrics] Counter incremented for {request.method} {request.url.path} [{status}]") - return response - - def to_iso(dt: datetime) -> str: return dt.astimezone(UTC).isoformat(timespec="microseconds") @@ -175,16 +160,8 @@ def check_seadex(request:Request, db=Depends(get_db)): @user_router.post("/add/anime") def add_anime(anime: AnimeCreate, user_id: UserId, db=Depends(get_db)): logger.info(f"[FastAPI] Received request to track anilist_id={anime.anilist_id}, user_id={user_id}") - - try: - anime_name = get_anime_name(anime.anilist_id) - except AnimeNotFoundError as e: - raise HTTPException(status_code=404, detail="Not found on AniList") from e - except MissingAnimeName as e: - raise HTTPException(status_code=404, detail="The anime exists but has no usable title") from e - except AniListError as e: - logger.exception("[FastAPI] AniList request failed") - raise HTTPException(status_code=502, detail="The AniList request failed") from e + + anime_name = get_anime_name(anime.anilist_id) logger.debug(f"[FastAPI] Found name={anime_name} for Anilist id={anime.anilist_id}") with db.cursor() as cursor: diff --git a/src/models.py b/src/models.py index 9520cc7..73a1413 100644 --- a/src/models.py +++ b/src/models.py @@ -1,5 +1,6 @@ from pydantic import BaseModel, Field, computed_field + class AnimeCreate(BaseModel): anilist_id: int = Field(gt=0)