refactor: error handling integration and testing standards

This commit is contained in:
matt 2026-03-17 17:29:14 -07:00
parent f784741416
commit f23c0dbf2c
10 changed files with 1038 additions and 8 deletions

View file

@ -496,14 +496,15 @@ class CommanderValidationError(DeckBuilderError):
missing required fields, or contains inconsistent information.
"""
def __init__(self, message: str, details: dict | None = None):
def __init__(self, message: str, details: dict | None = None, *, code: str = "CMD_VALID"):
"""Initialize commander validation error.
Args:
message: Description of the validation failure
details: Additional context about the error
code: Error code (overridable by subclasses)
"""
super().__init__(message, code="CMD_VALID", details=details)
super().__init__(message, code=code, details=details)
class CommanderTypeError(CommanderValidationError):
"""Raised when commander type validation fails.
@ -1393,4 +1394,30 @@ class ThemePoolError(DeckBuilderError):
message,
code="THEME_POOL_ERR",
details=details
)
)
# --- Web layer exceptions ---
class SessionExpiredError(DeckBuilderError):
"""Raised when a required session is missing or has expired."""
def __init__(self, sid: str | None = None, details: dict | None = None):
message = "Session has expired or could not be found"
super().__init__(message, code="SESSION_EXPIRED", details=details or {"sid": sid})
class BuildNotFoundError(DeckBuilderError):
"""Raised when a requested build result is not found in the session."""
def __init__(self, sid: str | None = None, details: dict | None = None):
message = "Build result not found; please start a new build"
super().__init__(message, code="BUILD_NOT_FOUND", details=details or {"sid": sid})
class FeatureDisabledError(DeckBuilderError):
"""Raised when a feature is accessed but has been disabled via environment config."""
def __init__(self, feature: str, details: dict | None = None):
message = f"Feature '{feature}' is currently disabled"
super().__init__(message, code="FEATURE_DISABLED", details=details or {"feature": feature})

View file

@ -0,0 +1,224 @@
"""Base test case classes for the MTG Python Deckbuilder web layer.
Provides reusable base classes and mixins that reduce boilerplate in route,
service, and validation tests. Import what you need don't inherit everything.
Usage:
from code.tests.base_test_cases import RouteTestCase, ServiceTestCase
class TestMyRoute(RouteTestCase):
def test_something(self):
resp = self.client.get("/my-route")
self.assert_ok(resp)
"""
from __future__ import annotations
import json
from pathlib import Path
from typing import Any
from unittest.mock import MagicMock
import pytest
from fastapi.testclient import TestClient
# ---------------------------------------------------------------------------
# Route test base
# ---------------------------------------------------------------------------
class RouteTestCase:
"""Base class for route integration tests.
Provides a shared TestClient and assertion helpers. Subclasses can override
`app_fixture` to use a different FastAPI app (e.g., a minimal test app).
Example:
class TestBuildWizard(RouteTestCase):
def test_step1_renders(self):
resp = self.get("/build/step1")
self.assert_ok(resp)
assert "Commander" in resp.text
"""
@pytest.fixture(autouse=True)
def setup_client(self, monkeypatch):
"""Create a TestClient for the full app. Override to customise."""
from code.web.app import app
with TestClient(app) as c:
self.client = c
yield
# --- Shorthand request helpers ---
def get(self, path: str, *, headers: dict | None = None, cookies: dict | None = None, **params) -> Any:
return self.client.get(path, headers=headers or {}, cookies=cookies or {}, params=params or {})
def post(self, path: str, data: dict | None = None, *, json: dict | None = None,
headers: dict | None = None, cookies: dict | None = None) -> Any:
return self.client.post(path, data=data, json=json, headers=headers or {}, cookies=cookies or {})
def htmx_get(self, path: str, *, cookies: dict | None = None, **params) -> Any:
"""GET with HX-Request header set (simulates HTMX fetch)."""
return self.client.get(path, headers={"HX-Request": "true"}, cookies=cookies or {}, params=params or {})
def htmx_post(self, path: str, data: dict | None = None, *, cookies: dict | None = None) -> Any:
"""POST with HX-Request header set (simulates HTMX form submission)."""
return self.client.post(path, data=data, headers={"HX-Request": "true"}, cookies=cookies or {})
# --- Assertion helpers ---
def assert_ok(self, resp, *, contains: str | None = None) -> None:
assert resp.status_code == 200, f"Expected 200, got {resp.status_code}: {resp.text[:200]}"
if contains:
assert contains in resp.text, f"Expected {contains!r} in response"
def assert_status(self, resp, status: int) -> None:
assert resp.status_code == status, f"Expected {status}, got {resp.status_code}: {resp.text[:200]}"
def assert_json_error(self, resp, *, status: int, error_type: str | None = None) -> dict:
"""Assert a standardized error JSON response (M4 format)."""
assert resp.status_code == status
data = resp.json()
assert data.get("error") is True, f"Expected error=True in: {data}"
assert "request_id" in data
if error_type:
assert data.get("error_type") == error_type, f"Expected error_type={error_type!r}, got {data.get('error_type')!r}"
return data
def assert_redirect(self, resp, *, to: str | None = None) -> None:
assert resp.status_code in (301, 302, 303, 307, 308), f"Expected redirect, got {resp.status_code}"
if to:
assert to in resp.headers.get("location", "")
def with_session(self, commander: str = "Atraxa, Praetors' Voice", **extra) -> tuple[str, dict]:
"""Create a session with basic commander state. Returns (sid, session_dict)."""
from code.web.services.tasks import get_session, new_sid
sid = new_sid()
sess = get_session(sid)
sess["commander"] = {"name": commander, "ok": True}
sess.update(extra)
return sid, sess
# ---------------------------------------------------------------------------
# Error handler test base
# ---------------------------------------------------------------------------
class ErrorHandlerTestCase:
"""Base for tests targeting the DeckBuilderError → HTTP response pipeline.
Spins up a minimal FastAPI app with only the error handler no routes from
the real app, so tests are fast and isolated.
Example:
class TestMyErrors(ErrorHandlerTestCase):
def test_custom_error(self):
from code.exceptions import ThemeSelectionError
self._register_raiser("/raise", ThemeSelectionError("bad theme"))
resp = self.error_client.get("/raise")
self.assert_json_error(resp, status=400, error_type="ThemeSelectionError")
"""
@pytest.fixture(autouse=True)
def setup_error_app(self):
from fastapi import FastAPI, Request
from code.exceptions import DeckBuilderError
from code.web.utils.responses import deck_builder_error_response
self._mini_app = FastAPI()
self._raisers: dict[str, Exception] = {}
app = self._mini_app
@app.exception_handler(Exception)
async def handler(request: Request, exc: Exception):
if isinstance(exc, DeckBuilderError):
return deck_builder_error_response(request, exc)
from fastapi.responses import JSONResponse
return JSONResponse({"error": "unhandled", "detail": str(exc)}, status_code=500)
with TestClient(app, raise_server_exceptions=False) as c:
self.error_client = c
yield
def _register_raiser(self, path: str, exc: Exception) -> None:
"""Add a GET endpoint that raises `exc` when called."""
from fastapi import Request
@self._mini_app.get(path)
async def _raiser(request: Request):
raise exc
# Rebuild the client after adding the route
with TestClient(self._mini_app, raise_server_exceptions=False) as c:
self.error_client = c
def assert_json_error(self, resp, *, status: int, error_type: str | None = None) -> dict:
assert resp.status_code == status, f"Expected {status}, got {resp.status_code}: {resp.text[:200]}"
data = resp.json()
assert data.get("error") is True
if error_type:
assert data.get("error_type") == error_type
return data
# ---------------------------------------------------------------------------
# Service test base
# ---------------------------------------------------------------------------
class ServiceTestCase:
"""Base class for service unit tests.
Provides helpers for creating mock dependencies and asserting common
service behaviors. No TestClient services are tested directly.
Example:
class TestSessionManager(ServiceTestCase):
def test_new_session_is_empty(self):
from code.web.services.tasks import SessionManager
mgr = SessionManager()
sess = mgr.get("new-key")
assert sess == {}
"""
def make_mock(self, **attrs) -> MagicMock:
"""Create a MagicMock with the given attributes pre-set."""
m = MagicMock()
for k, v in attrs.items():
setattr(m, k, v)
return m
def assert_raises(self, exc_type, fn, *args, **kwargs):
"""Assert that fn(*args, **kwargs) raises exc_type."""
with pytest.raises(exc_type):
fn(*args, **kwargs)
# ---------------------------------------------------------------------------
# Validation test mixin
# ---------------------------------------------------------------------------
class ValidationTestMixin:
"""Mixin for Pydantic model validation tests.
Example:
class TestBuildRequest(ValidationTestMixin):
MODEL = BuildRequest
def test_commander_required(self):
self.assert_validation_error(commander="")
"""
MODEL = None # Set in subclass
def build(self, **kwargs) -> Any:
"""Instantiate MODEL with kwargs. Raises ValidationError on invalid input."""
assert self.MODEL is not None, "Set MODEL in your test class"
return self.MODEL(**kwargs)
def assert_validation_error(self, **kwargs) -> None:
"""Assert that MODEL(**kwargs) raises a Pydantic ValidationError."""
from pydantic import ValidationError
assert self.MODEL is not None
with pytest.raises(ValidationError):
self.MODEL(**kwargs)

View file

@ -0,0 +1,252 @@
"""Tests for M4 error handling integration.
Covers:
- DeckBuilderError HTTP response conversion
- HTMX vs JSON response detection
- Status code mapping for exception hierarchy
- app.py DeckBuilderError exception handler
- Web-specific exceptions (SessionExpiredError, BuildNotFoundError, FeatureDisabledError)
"""
from __future__ import annotations
import pytest
from unittest.mock import MagicMock
from fastapi.testclient import TestClient
from fastapi import FastAPI, Request
from code.exceptions import (
DeckBuilderError,
CommanderValidationError,
CommanderTypeError,
ThemeSelectionError,
SessionExpiredError,
BuildNotFoundError,
FeatureDisabledError,
CSVFileNotFoundError,
PriceAPIError,
)
from code.web.utils.responses import (
deck_error_to_status,
deck_builder_error_response,
is_htmx_request,
)
# ---------------------------------------------------------------------------
# Unit tests: exception → status mapping
# ---------------------------------------------------------------------------
class TestDeckErrorToStatus:
def test_commander_validation_400(self):
assert deck_error_to_status(CommanderValidationError("bad")) == 400
def test_commander_type_400(self):
assert deck_error_to_status(CommanderTypeError("bad type")) == 400
def test_theme_selection_400(self):
assert deck_error_to_status(ThemeSelectionError("bad theme")) == 400
def test_session_expired_401(self):
assert deck_error_to_status(SessionExpiredError()) == 401
def test_build_not_found_404(self):
assert deck_error_to_status(BuildNotFoundError()) == 404
def test_feature_disabled_404(self):
assert deck_error_to_status(FeatureDisabledError("test_feature")) == 404
def test_csv_file_not_found_503(self):
assert deck_error_to_status(CSVFileNotFoundError("cards.csv")) == 503
def test_price_api_error_503(self):
assert deck_error_to_status(PriceAPIError("http://x", 500)) == 503
def test_base_deck_builder_error_500(self):
assert deck_error_to_status(DeckBuilderError("generic")) == 500
def test_subclass_not_in_map_falls_back_via_mro(self):
# CommanderTypeError is a subclass of CommanderValidationError
exc = CommanderTypeError("oops")
assert deck_error_to_status(exc) == 400
# ---------------------------------------------------------------------------
# Unit tests: HTMX detection
# ---------------------------------------------------------------------------
class TestIsHtmxRequest:
def _make_request(self, hx_header: str | None = None) -> Request:
scope = {
"type": "http",
"method": "GET",
"path": "/test",
"query_string": b"",
"headers": [],
}
if hx_header is not None:
scope["headers"] = [(b"hx-request", hx_header.encode())]
return Request(scope)
def test_htmx_true_header(self):
req = self._make_request("true")
assert is_htmx_request(req) is True
def test_no_htmx_header(self):
req = self._make_request()
assert is_htmx_request(req) is False
def test_htmx_false_header(self):
req = self._make_request("false")
assert is_htmx_request(req) is False
# ---------------------------------------------------------------------------
# Unit tests: deck_builder_error_response
# ---------------------------------------------------------------------------
class TestDeckBuilderErrorResponse:
def _make_request(self, htmx: bool = False) -> Request:
headers = []
if htmx:
headers.append((b"hx-request", b"true"))
scope = {
"type": "http",
"method": "GET",
"path": "/build/new",
"query_string": b"",
"headers": headers,
}
req = Request(scope)
req.state.request_id = "test-rid-123"
return req
def test_json_response_structure(self):
from fastapi.responses import JSONResponse
req = self._make_request(htmx=False)
exc = CommanderValidationError("Invalid commander")
resp = deck_builder_error_response(req, exc)
assert isinstance(resp, JSONResponse)
assert resp.status_code == 400
import json
body = json.loads(resp.body)
assert body["error"] is True
assert body["status"] == 400
assert body["error_type"] == "CommanderValidationError"
assert body["message"] == "Invalid commander"
assert body["request_id"] == "test-rid-123"
assert "timestamp" in body
assert "path" in body
def test_htmx_response_is_html(self):
from fastapi.responses import HTMLResponse
req = self._make_request(htmx=True)
exc = ThemeSelectionError("Invalid theme")
resp = deck_builder_error_response(req, exc)
assert isinstance(resp, HTMLResponse)
assert resp.status_code == 400
assert "Invalid theme" in resp.body.decode()
assert resp.headers.get("X-Request-ID") == "test-rid-123"
def test_request_id_in_response_header(self):
req = self._make_request(htmx=False)
exc = BuildNotFoundError()
resp = deck_builder_error_response(req, exc)
assert resp.headers.get("X-Request-ID") == "test-rid-123"
# ---------------------------------------------------------------------------
# Integration tests: app exception handler
# ---------------------------------------------------------------------------
@pytest.fixture(scope="module")
def test_app():
"""Minimal FastAPI app that raises DeckBuilderErrors for testing."""
app = FastAPI()
@app.get("/raise/commander")
async def raise_commander(request: Request):
raise CommanderValidationError("Commander 'Foo' not found")
@app.get("/raise/session")
async def raise_session(request: Request):
raise SessionExpiredError(sid="abc123")
@app.get("/raise/feature")
async def raise_feature(request: Request):
raise FeatureDisabledError("partner_suggestions")
@app.get("/raise/generic")
async def raise_generic(request: Request):
raise DeckBuilderError("Something went wrong")
# Wire the same handler as app.py
from code.exceptions import DeckBuilderError as DBE
from code.web.utils.responses import deck_builder_error_response
@app.exception_handler(Exception)
async def handler(request: Request, exc: Exception):
if isinstance(exc, DBE):
return deck_builder_error_response(request, exc)
from fastapi.responses import JSONResponse
return JSONResponse({"error": "unhandled"}, status_code=500)
return app
@pytest.fixture(scope="module")
def client(test_app):
return TestClient(test_app, raise_server_exceptions=False)
class TestAppExceptionHandler:
def test_commander_validation_returns_400(self, client):
resp = client.get("/raise/commander")
assert resp.status_code == 400
data = resp.json()
assert data["error"] is True
assert data["error_type"] == "CommanderValidationError"
assert "Commander 'Foo' not found" in data["message"]
def test_session_expired_returns_401(self, client):
resp = client.get("/raise/session")
assert resp.status_code == 401
data = resp.json()
assert data["error_type"] == "SessionExpiredError"
def test_feature_disabled_returns_404(self, client):
resp = client.get("/raise/feature")
assert resp.status_code == 404
data = resp.json()
assert data["error_type"] == "FeatureDisabledError"
def test_generic_deck_builder_error_returns_500(self, client):
resp = client.get("/raise/generic")
assert resp.status_code == 500
data = resp.json()
assert data["error_type"] == "DeckBuilderError"
def test_htmx_commander_error_returns_html(self, client):
resp = client.get("/raise/commander", headers={"HX-Request": "true"})
assert resp.status_code == 400
assert "text/html" in resp.headers.get("content-type", "")
assert "Commander 'Foo' not found" in resp.text
# ---------------------------------------------------------------------------
# Web-specific exception constructors
# ---------------------------------------------------------------------------
class TestWebExceptions:
def test_session_expired_has_code(self):
exc = SessionExpiredError(sid="xyz")
assert exc.code == "SESSION_EXPIRED"
assert "xyz" in str(exc.details)
def test_build_not_found_has_code(self):
exc = BuildNotFoundError(sid="abc")
assert exc.code == "BUILD_NOT_FOUND"
def test_feature_disabled_has_feature_name(self):
exc = FeatureDisabledError("partner_suggestions")
assert exc.code == "FEATURE_DISABLED"
assert "partner_suggestions" in exc.message

View file

@ -22,6 +22,8 @@ from .services.combo_utils import detect_all as _detect_all
from .services.theme_catalog_loader import prewarm_common_filters, load_index
from .services.commander_catalog_loader import load_commander_catalog
from .services.tasks import get_session, new_sid, set_session_value
from code.exceptions import DeckBuilderError
from .utils.responses import deck_builder_error_response
# Logger for app-level logging
logger = logging.getLogger(__name__)
@ -2403,6 +2405,13 @@ async def starlette_http_exception_handler(request: Request, exc: StarletteHTTPE
@app.exception_handler(Exception)
async def unhandled_exception_handler(request: Request, exc: Exception):
# Handle DeckBuilderError subclasses with structured responses before falling to 500
if isinstance(exc, DeckBuilderError):
rid = getattr(request.state, "request_id", None) or uuid.uuid4().hex
logging.getLogger("web").warning(
f"DeckBuilderError [rid={rid}] {type(exc).__name__} {request.method} {request.url.path}: {exc}"
)
return deck_builder_error_response(request, exc)
rid = getattr(request.state, "request_id", None) or uuid.uuid4().hex
logging.getLogger("web").error(
f"Unhandled exception [rid={rid}] {request.method} {request.url.path}", exc_info=True

View file

@ -2,7 +2,7 @@ from __future__ import annotations
from typing import Iterable, List, Optional
from fastapi import APIRouter, HTTPException, Query, Request
from fastapi import APIRouter, Query, Request
from fastapi.responses import JSONResponse
from deck_builder.combined_commander import PartnerMode
@ -10,6 +10,7 @@ from deck_builder.combined_commander import PartnerMode
from ..app import ENABLE_PARTNER_MECHANICS
from ..services.partner_suggestions import get_partner_suggestions
from ..services.telemetry import log_partner_suggestions_generated
from code.exceptions import CommanderValidationError, FeatureDisabledError
router = APIRouter(prefix="/api/partner", tags=["partner suggestions"])
@ -65,11 +66,11 @@ async def partner_suggestions_api(
refresh: bool = Query(False, description="When true, force a dataset refresh before scoring"),
):
if not ENABLE_PARTNER_MECHANICS:
raise HTTPException(status_code=404, detail="Partner suggestions are disabled")
raise FeatureDisabledError("partner_suggestions")
commander_name = (commander or "").strip()
if not commander_name:
raise HTTPException(status_code=400, detail="Commander name is required")
raise CommanderValidationError("Commander name is required")
include_modes = _parse_modes(mode)
result = get_partner_suggestions(
@ -79,7 +80,7 @@ async def partner_suggestions_api(
refresh_dataset=refresh,
)
if result is None:
raise HTTPException(status_code=503, detail="Partner suggestion dataset is unavailable")
raise FeatureDisabledError("partner_suggestion_dataset")
partner_names = _coerce_name_list(partner)
background_names = _coerce_name_list(background)

View file

@ -156,3 +156,97 @@ def merge_hx_trigger(response: HTMLResponse, events: Dict[str, Any]) -> None:
response.headers["HX-Trigger"] = json.dumps(events)
else:
response.headers["HX-Trigger"] = json.dumps(events)
# --- DeckBuilderError integration ---
def is_htmx_request(request: Request) -> bool:
"""Return True if the request was made by HTMX."""
try:
return request.headers.get("HX-Request") == "true"
except Exception:
return False
# Map DeckBuilderError subclass names to HTTP status codes.
# More specific subclasses should appear before their parents.
_EXCEPTION_STATUS_MAP: list[tuple[str, int]] = [
# Web-specific
("SessionExpiredError", 401),
("BuildNotFoundError", 404),
("FeatureDisabledError", 404),
# Commander
("CommanderValidationError", 400),
("CommanderTypeError", 400),
("CommanderColorError", 400),
("CommanderTagError", 400),
("CommanderPartnerError", 400),
("CommanderSelectionError", 400),
("CommanderLoadError", 503),
# Theme
("ThemeSelectionError", 400),
("ThemeWeightError", 400),
("ThemeError", 400),
# Price
("PriceLimitError", 400),
("PriceValidationError", 400),
("PriceAPIError", 503),
("PriceError", 400),
# CSV / setup data unavailable
("CSVFileNotFoundError", 503),
("MTGJSONDownloadError", 503),
("EmptyDataFrameError", 503),
("CSVError", 503),
("MTGSetupError", 503),
]
def deck_error_to_status(exc: Exception) -> int:
"""Return the appropriate HTTP status code for a DeckBuilderError."""
exc_type = type(exc).__name__
for name, status in _EXCEPTION_STATUS_MAP:
if exc_type == name:
return status
# Walk MRO for inexact matches (subclasses not listed above)
for cls in type(exc).__mro__:
for name, status in _EXCEPTION_STATUS_MAP:
if cls.__name__ == name:
return status
return 500
def deck_builder_error_response(request: Request, exc: Exception) -> JSONResponse | HTMLResponse:
"""Convert a DeckBuilderError to an appropriate HTTP response.
Returns an HTML error fragment for HTMX requests, JSON otherwise.
Includes request_id and standardized structure.
"""
import time
status = deck_error_to_status(exc)
request_id = getattr(getattr(request, "state", None), "request_id", None) or "unknown"
# User-safe message: use .message attribute if present, else str()
message = getattr(exc, "message", None) or str(exc)
error_type = type(exc).__name__
code = getattr(exc, "code", error_type)
if is_htmx_request(request):
html = (
f'<div class="error-banner" role="alert">'
f'<strong>{status}</strong> {message}'
f'</div>'
)
return HTMLResponse(content=html, status_code=status, headers={"X-Request-ID": request_id})
payload: Dict[str, Any] = {
"error": True,
"status": status,
"error_type": error_type,
"code": code,
"message": message,
"path": str(request.url.path),
"request_id": request_id,
"timestamp": time.strftime("%Y-%m-%dT%H:%M:%SZ", time.gmtime()),
}
return JSONResponse(content=payload, status_code=status, headers={"X-Request-ID": request_id})