diff --git a/.env.example b/.env.example index 0dca713..1e7669f 100644 --- a/.env.example +++ b/.env.example @@ -229,6 +229,10 @@ SHOP_TIMEZONE=Europe/Berlin # ingest endpoint returns 404. STOREFRONT_ANALYTICS_ENABLED=false +# Accept uncaught error reports from the frontends. Off by default, for the same +# reason. While false the report endpoint returns 404. +FRONTEND_ERRORS_ENABLED=false + # ---------------------------------- # OpenTelemetry # ---------------------------------- diff --git a/docs/observability.md b/docs/observability.md index 8385428..6878c6f 100644 --- a/docs/observability.md +++ b/docs/observability.md @@ -131,3 +131,78 @@ which looks like a working panel. That mistake is already made and fixed here. output and Prometheus, not against the fact that setup was called. That distinction caught the real bug: instrumenting the app before configuring telemetry logged a clean start and produced no HTTP metrics at all. + +--- + +# Frontend Errors + +Uncaught errors from the storefront and the admin UI. + +Traces and metrics see nothing that happens in a browser. A component that +throws leaves the server returning 200 with healthy metrics while the shop is +broken for a real customer — and for a shop, by the time someone reports it the +sale is gone. + +``` +POST /v1/telemetry/errors report (public, rate limited, opt-in) +GET /v1/admin/telemetry/errors read, grouped (admin) +``` + +## Public, and treated as such + +Storefront visitors are not signed in, and an error that happens before login is +exactly the one worth catching — so the report endpoint takes no token. It is +therefore hostile-input territory: + +| Guard | Why | +|---|---| +| 30 requests/minute per address | A component throwing in a render loop reports as fast as the browser can loop | +| 10 errors per batch | One request cannot be a bulk insert | +| Closed `app` vocabulary | The field cannot become free text | +| `extra="forbid"` | Unknown fields are refused, not ignored | +| Stack truncated at 4000 chars | Unbounded input, and the top frames are where the fault is | +| Off unless `FRONTEND_ERRORS_ENABLED` | Returns 404 while off | + +## The user agent is reduced, never stored + +A raw user-agent string is a fingerprint. But "which browser?" is genuinely +diagnostic — a large share of frontend bugs are one engine behaving differently +— so discarding it entirely makes the reports much weaker. + +The compromise: reduce it at the boundary to a family and major version, and +store only that. `Safari 18` reproduces a bug; it does not recognise anyone. + +The reduction is also a filter. Whatever a client sends, the output is a known +family name and an integer — never a fragment of the input: + +``` +"Mozilla/5.0 Chrome/140 user=alice@example.com token=abc123" → "Chrome 140" +``` + +There is no column for an IP address, an email or a customer id, and a test +fails if one appears. + +## Errors are grouped + +One bug produces thousands of identical rows, so the read endpoint groups by +application, error class and message, ordered by frequency. + +Deliberately **not** grouped by stack: the same fault reached from two routes +produces two different stacks and is one bug. `affected_paths` shows the spread +instead, and one representative stack is returned for debugging. + +## What this cannot tell you + +It reports only what browsers managed to send. An error that breaks a page badly +enough to stop the reporter is precisely the one that will not appear — so a +quiet report is weaker evidence than a noisy one. Read silence as "no news", +never as "no errors". + +## Testing + +- `tests/test_frontend_errors_unit.py` — the user-agent reduction, including + Edge not being reported as Chrome, and that nothing from the input string + survives it. +- `tests/test_frontend_errors_integration.py` — reporting without auth, the + PII-column assertion, the raw agent never reaching storage, query-string + stripping, stale timestamps, and grouping across routes. diff --git a/src/app/db_models.py b/src/app/db_models.py index 041253a..5ec7cdc 100644 --- a/src/app/db_models.py +++ b/src/app/db_models.py @@ -42,6 +42,11 @@ # Returns (RMA) from app.services.returns.models.returns_db_models import ReturnDB # noqa: F401 +# Frontend errors (S4) +from app.services.frontend_errors.models.frontend_errors_db_models import ( # noqa: F401 + FrontendErrorDB, +) + # Storefront analytics (S2) from app.services.storefront_analytics.models.storefront_events_db_models import ( # noqa: F401 StorefrontEventDB, diff --git a/src/app/main.py b/src/app/main.py index ab0f014..5dab56e 100644 --- a/src/app/main.py +++ b/src/app/main.py @@ -12,6 +12,10 @@ from app.services.crud_item_store import router as item_store_router from app.services.customers import customers_api_router from app.services.fulfillment import fulfillment_api_router +from app.services.frontend_errors import ( + admin_frontend_errors_api_router, + frontend_errors_api_router, +) from app.services.health import health_api_router from app.services.inventory import inventory_api_router from app.services.mail import mail_api_router @@ -170,6 +174,10 @@ async def generic_exception_handler(request: Request, exc: Exception) -> JSONRes # Include analytics service router (S1) app.include_router(analytics_api_router, prefix="/v1") +# Include frontend error reporting (S4) — public ingest plus admin read +app.include_router(frontend_errors_api_router, prefix="/v1") +app.include_router(admin_frontend_errors_api_router, prefix="/v1") + # Include storefront analytics ingest (S2) — public, rate limited, opt-in app.include_router(storefront_analytics_api_router, prefix="/v1") diff --git a/src/app/services/frontend_errors/__init__.py b/src/app/services/frontend_errors/__init__.py new file mode 100644 index 0000000..2b9e6b1 --- /dev/null +++ b/src/app/services/frontend_errors/__init__.py @@ -0,0 +1,35 @@ +""" +Frontend Errors Service + +Uncaught browser errors from the storefront and the admin UI. + +S3 gave the API traces and metrics; neither can see a component that throws in +somebody's browser. The server returns 200, the metrics look healthy, and the +shop is broken for a real customer. + +Endpoints: + POST /telemetry/errors — report (public, rate limited, opt-in) + GET /admin/telemetry/errors — read, grouped (admin) +""" + +from fastapi import APIRouter + +from app.authorize import require_admin +from fastapi import Depends + +from .routers import admin_router, report_router + +frontend_errors_api_router = APIRouter( + prefix="/telemetry", + tags=["Frontend Errors"], +) +frontend_errors_api_router.include_router(report_router) + +admin_frontend_errors_api_router = APIRouter( + prefix="/admin/telemetry", + tags=["Frontend Errors"], + dependencies=[Depends(require_admin)], +) +admin_frontend_errors_api_router.include_router(admin_router) + +__all__ = ["admin_frontend_errors_api_router", "frontend_errors_api_router"] diff --git a/src/app/services/frontend_errors/functions/__init__.py b/src/app/services/frontend_errors/functions/__init__.py new file mode 100644 index 0000000..b0f57e4 --- /dev/null +++ b/src/app/services/frontend_errors/functions/__init__.py @@ -0,0 +1,5 @@ +"""Frontend error helpers.""" + +from .user_agent import coarse_browser + +__all__ = ["coarse_browser"] diff --git a/src/app/services/frontend_errors/functions/user_agent.py b/src/app/services/frontend_errors/functions/user_agent.py new file mode 100644 index 0000000..6cb5c07 --- /dev/null +++ b/src/app/services/frontend_errors/functions/user_agent.py @@ -0,0 +1,49 @@ +""" +Coarse browser identification. + +A raw user-agent string is a fingerprint: combined with a handful of other +signals it identifies a device, which is exactly what the rest of this system +refuses to collect. But "which browser?" is genuinely diagnostic — half of all +frontend bugs are one engine behaving differently — so throwing it away entirely +makes the error reports much less useful. + +The compromise is to reduce the string to a family and a major version at the +boundary and store only that. "Safari 18" is enough to reproduce a bug and not +enough to recognise anyone. +""" + +from __future__ import annotations + +import re + +_UNKNOWN = "unknown" + +# Order matters. Edge and Opera both claim to be Chrome, and Chrome claims to be +# Safari, so the most specific pattern has to win. +_PATTERNS: tuple[tuple[str, re.Pattern[str]], ...] = ( + ("Edge", re.compile(r"Edg(?:e|A|iOS)?/(\d+)")), + ("Opera", re.compile(r"OPR/(\d+)")), + ("Samsung Internet", re.compile(r"SamsungBrowser/(\d+)")), + ("Firefox", re.compile(r"Firefox/(\d+)")), + ("Chrome", re.compile(r"Chrome/(\d+)")), + ("Safari", re.compile(r"Version/(\d+).*Safari")), +) + + +def coarse_browser(user_agent: str | None) -> str: + """ + Reduce a user-agent string to "Family Major", or "unknown". + + Never returns anything derived from the original string beyond the family + name and an integer, so no amount of unusual input can smuggle a fingerprint + through. + """ + if not user_agent: + return _UNKNOWN + + for family, pattern in _PATTERNS: + match = pattern.search(user_agent) + if match: + return f"{family} {match.group(1)}" + + return _UNKNOWN diff --git a/src/app/services/frontend_errors/models/__init__.py b/src/app/services/frontend_errors/models/__init__.py new file mode 100644 index 0000000..80aae17 --- /dev/null +++ b/src/app/services/frontend_errors/models/__init__.py @@ -0,0 +1,25 @@ +"""Frontend error models.""" + +from .frontend_errors_db_models import FrontendErrorDB +from .frontend_errors_models import ( + MAX_ERRORS_PER_BATCH, + MAX_STACK_CHARS, + ErrorGroup, + FrontendApp, + FrontendErrorBatch, + FrontendErrorIngestResponse, + FrontendErrorInput, + FrontendErrorsResponse, +) + +__all__ = [ + "MAX_ERRORS_PER_BATCH", + "MAX_STACK_CHARS", + "ErrorGroup", + "FrontendApp", + "FrontendErrorBatch", + "FrontendErrorDB", + "FrontendErrorIngestResponse", + "FrontendErrorInput", + "FrontendErrorsResponse", +] diff --git a/src/app/services/frontend_errors/models/frontend_errors_db_models.py b/src/app/services/frontend_errors/models/frontend_errors_db_models.py new file mode 100644 index 0000000..7d9a2a9 --- /dev/null +++ b/src/app/services/frontend_errors/models/frontend_errors_db_models.py @@ -0,0 +1,71 @@ +""" +Frontend Error Database Model + +One row per reported uncaught error from a browser. + +As with storefront_events, what is absent matters: no IP address, no raw user +agent, no customer id, no email. `browser` holds a coarse family and major +version parsed server-side — enough to reproduce a bug, not enough to recognise +anyone. +""" + +from datetime import datetime +from uuid import UUID, uuid4 + +from sqlalchemy import DateTime, Index, String, Text, text +from sqlalchemy.dialects.postgresql import UUID as PGUUID +from sqlalchemy.orm import Mapped, mapped_column + +from app.shared.database.base import Base + + +class FrontendErrorDB(Base): + """An uncaught error reported by one of the frontends.""" + + __tablename__ = "frontend_errors" + + id: Mapped[UUID] = mapped_column( + PGUUID(as_uuid=True), + primary_key=True, + default=uuid4, + server_default=text("gen_random_uuid()"), + ) + + app: Mapped[str] = mapped_column( + String(20), nullable=False, doc="storefront | admin" + ) + name: Mapped[str] = mapped_column( + String(120), nullable=False, doc="Error class, e.g. TypeError" + ) + message: Mapped[str] = mapped_column(String(500), nullable=False) + stack: Mapped[str | None] = mapped_column( + Text, nullable=True, doc="Truncated at ingest — a full trace is unbounded input" + ) + path: Mapped[str | None] = mapped_column( + String(255), nullable=True, doc="Route only; query strings are stripped" + ) + browser: Mapped[str] = mapped_column( + String(60), + nullable=False, + server_default=text("'unknown'"), + doc="Coarse family and major version. Never the raw user agent.", + ) + + occurred_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False + ) + created_at: Mapped[datetime] = mapped_column( + DateTime(timezone=True), nullable=False, server_default=text("now()") + ) + + __table_args__ = ( + Index("ix_frontend_errors_occurred_at", "occurred_at"), + Index("ix_frontend_errors_app_occurred", "app", "occurred_at"), + # Grouping is the only way anyone reads this table: one bug in a render + # loop produces thousands of identical rows, and the useful question is + # "which distinct errors, how often". + Index("ix_frontend_errors_grouping", "app", "name", "message"), + ) + + def __repr__(self) -> str: + return f"FrontendErrorDB(id={self.id}, app={self.app!r}, name={self.name!r})" diff --git a/src/app/services/frontend_errors/models/frontend_errors_models.py b/src/app/services/frontend_errors/models/frontend_errors_models.py new file mode 100644 index 0000000..2da0a8d --- /dev/null +++ b/src/app/services/frontend_errors/models/frontend_errors_models.py @@ -0,0 +1,113 @@ +""" +Frontend Error Schemas + +The report endpoint is public, so this schema is a boundary against hostile and +merely broken input alike. A component erroring inside a render loop is not an +attack, and it will still send thousands of reports a second if nothing stops +it. +""" + +from __future__ import annotations + +from datetime import datetime +from enum import Enum + +from pydantic import BaseModel, ConfigDict, Field, field_validator + +from app.shared.responses import BaseResponse + +MAX_ERRORS_PER_BATCH = 10 +MAX_STACK_CHARS = 4000 + + +class FrontendApp(str, Enum): + """Which application reported. Closed, so it cannot become a free-text field.""" + + STOREFRONT = "storefront" + ADMIN = "admin" + + +class FrontendErrorInput(BaseModel): + """One uncaught error.""" + + model_config = ConfigDict(extra="forbid") + + app: FrontendApp + name: str = Field(max_length=120) + message: str = Field(max_length=500) + stack: str | None = Field(default=None) + path: str | None = Field(default=None, max_length=255) + occurred_at: datetime + + @field_validator("path", mode="before") + @classmethod + def strip_query_string(cls, value: object) -> object: + """Same reasoning as storefront events: query strings carry accidents.""" + if not isinstance(value, str): + return value + for separator in ("?", "#"): + value = value.split(separator, 1)[0] + return value[:255] + + @field_validator("stack", mode="before") + @classmethod + def bound_stack(cls, value: object) -> object: + """ + Truncate rather than reject. + + A stack trace is unbounded input from a public endpoint, but it is also + the single most useful field here. Cutting it keeps the top frames, + which is where the fault is. + """ + if not isinstance(value, str): + return value + if len(value) <= MAX_STACK_CHARS: + return value + return value[:MAX_STACK_CHARS] + "\n… truncated" + + +class FrontendErrorBatch(BaseModel): + """A batch from one browser.""" + + model_config = ConfigDict(extra="forbid") + + errors: list[FrontendErrorInput] = Field( + min_length=1, max_length=MAX_ERRORS_PER_BATCH + ) + + +class FrontendErrorIngestResponse(BaseResponse): + """How many reports were kept.""" + + accepted: int + rejected: int + + +class ErrorGroup(BaseModel): + """Distinct errors, with how often and how recently they happened.""" + + app: str + name: str + message: str + occurrences: int + affected_paths: int = Field(description="Distinct routes this error occurred on") + browsers: list[str] = Field( + default_factory=list, description="Coarse browser labels that hit it" + ) + first_seen: datetime + last_seen: datetime + sample_stack: str | None = None + + +class FrontendErrorsResponse(BaseResponse): + """ + Errors grouped, because one bug produces thousands of identical rows. + + Reports only what browsers managed to send. An error that breaks the page + badly enough to stop the reporter is the one you will not see here, so a + quiet report is weaker evidence than a noisy one. + """ + + enabled: bool + total_occurrences: int + groups: list[ErrorGroup] = Field(default_factory=list) diff --git a/src/app/services/frontend_errors/routers/__init__.py b/src/app/services/frontend_errors/routers/__init__.py new file mode 100644 index 0000000..eefd7ad --- /dev/null +++ b/src/app/services/frontend_errors/routers/__init__.py @@ -0,0 +1,5 @@ +"""Frontend error routers.""" + +from .frontend_errors_router import admin_router, report_router + +__all__ = ["admin_router", "report_router"] diff --git a/src/app/services/frontend_errors/routers/frontend_errors_router.py b/src/app/services/frontend_errors/routers/frontend_errors_router.py new file mode 100644 index 0000000..d6acdf7 --- /dev/null +++ b/src/app/services/frontend_errors/routers/frontend_errors_router.py @@ -0,0 +1,138 @@ +""" +Frontend Error Router + + POST /v1/telemetry/errors — report uncaught browser errors (public) + GET /v1/admin/telemetry/errors — read them, grouped (admin) + +The report endpoint is public because the storefront's visitors are not signed +in, and an error that happens before login is exactly the one worth catching. +""" + +from __future__ import annotations + +from datetime import date + +from fastapi import APIRouter, Depends, Query, Request, status +from sqlalchemy.ext.asyncio import AsyncSession + +from app.services.analytics.functions import build_period +from app.shared.config import get_settings +from app.shared.database.session import get_session_dependency +from app.shared.exceptions import NotFoundError +from app.shared.logger import get_logger +from app.shared.rate_limit import limiter + +from ..functions import coarse_browser +from ..models import ( + ErrorGroup, + FrontendErrorBatch, + FrontendErrorIngestResponse, + FrontendErrorsResponse, +) +from ..services import FrontendErrorRepository + +logger = get_logger(__name__) + +report_router = APIRouter() +admin_router = APIRouter() + +# Deliberately tighter than the analytics ingest. A component throwing inside a +# render loop is the normal failure mode here, and it will report as fast as the +# browser can loop. +_REPORT_RATE_LIMIT = "30/minute" + + +@report_router.post( + "/errors", + response_model=FrontendErrorIngestResponse, + status_code=status.HTTP_202_ACCEPTED, + summary="Report uncaught frontend errors", + description=( + "Accepts uncaught errors from the storefront or admin UI.\n\n" + "Public, because storefront visitors are not signed in and an error " + "before login is exactly the one worth catching. Records no IP, no raw " + "user agent and no identity — the user agent is reduced server-side to " + "a coarse family and major version, which is enough to reproduce a bug " + "and not enough to recognise anyone.\n\n" + "Disabled unless `FRONTEND_ERRORS_ENABLED` is set, returning 404 while off." + ), + responses={ + 404: {"description": "Frontend error reporting is not enabled."}, + 429: {"description": "Rate limit exceeded."}, + }, +) +@limiter.limit(_REPORT_RATE_LIMIT) +async def report_errors( + request: Request, + batch: FrontendErrorBatch, + session: AsyncSession = Depends(get_session_dependency), +) -> FrontendErrorIngestResponse: + settings = get_settings() + + if not settings.frontend_errors_enabled: + raise NotFoundError( + message="Frontend error reporting is not enabled on this deployment", + context={"setting": "FRONTEND_ERRORS_ENABLED"}, + ) + + # Read here and reduced immediately. The raw string never reaches storage. + browser = coarse_browser(request.headers.get("user-agent")) + + repository = FrontendErrorRepository(session) + accepted, rejected = await repository.record(batch.errors, browser) + + if accepted: + logger.warning( + "Frontend errors reported", + extra={ + "count": accepted, + "app": batch.errors[0].app.value, + "first_message": batch.errors[0].message[:200], + "browser": browser, + }, + ) + + return FrontendErrorIngestResponse( + success=True, + message="Errors recorded", + accepted=accepted, + rejected=rejected, + ) + + +@admin_router.get( + "/errors", + response_model=FrontendErrorsResponse, + summary="Frontend errors, grouped (admin)", + description=( + "Distinct errors with occurrence counts, ordered by frequency.\n\n" + "Grouped by application, error class and message rather than by stack: " + "the same fault reached from two routes produces two stacks and is one " + "bug.\n\n" + "Reports only what browsers managed to send. An error that breaks a page " + "badly enough to stop the reporter is the one that will not appear here, " + "so a quiet report is weaker evidence than a noisy one." + ), + dependencies=[Depends(get_settings)], +) +async def list_errors( + date_from: date | None = Query(None, alias="from"), + date_to: date | None = Query(None, alias="to"), + app: str | None = Query(None, pattern="^(storefront|admin)$"), + limit: int = Query(25, ge=1, le=200), + session: AsyncSession = Depends(get_session_dependency), +) -> FrontendErrorsResponse: + settings = get_settings() + period = build_period(date_from, date_to, settings.shop_timezone, default_days=7) + + repository = FrontendErrorRepository(session) + groups = await repository.grouped(period.start, period.end, app, limit) + total = await repository.total(period.start, period.end, app) + + return FrontendErrorsResponse( + success=True, + message="Frontend errors retrieved successfully", + enabled=settings.frontend_errors_enabled, + total_occurrences=total, + groups=[ErrorGroup(**group) for group in groups], + ) diff --git a/src/app/services/frontend_errors/services/__init__.py b/src/app/services/frontend_errors/services/__init__.py new file mode 100644 index 0000000..cc5016b --- /dev/null +++ b/src/app/services/frontend_errors/services/__init__.py @@ -0,0 +1,13 @@ +"""Frontend error services.""" + +from .frontend_errors_db_service import ( + MAX_CLOCK_SKEW, + FrontendErrorRepository, + get_frontend_error_repository, +) + +__all__ = [ + "MAX_CLOCK_SKEW", + "FrontendErrorRepository", + "get_frontend_error_repository", +] diff --git a/src/app/services/frontend_errors/services/frontend_errors_db_service.py b/src/app/services/frontend_errors/services/frontend_errors_db_service.py new file mode 100644 index 0000000..90d5041 --- /dev/null +++ b/src/app/services/frontend_errors/services/frontend_errors_db_service.py @@ -0,0 +1,135 @@ +""" +Frontend Error Database Service + +Writing browser error reports, and reading them back grouped. +""" + +from __future__ import annotations + +from datetime import UTC, datetime, timedelta + +from sqlalchemy import Select, and_, distinct, func, select +from sqlalchemy.ext.asyncio import AsyncSession + +from app.shared.logger import get_logger + +from ..models import FrontendErrorDB, FrontendErrorInput + +logger = get_logger(__name__) + +# Same reasoning as storefront events: a browser clock can be wrong, but it must +# not be able to write into a period already reviewed. +MAX_CLOCK_SKEW = timedelta(hours=24) + + +class FrontendErrorRepository: + """Persistence for browser error reports.""" + + def __init__(self, session: AsyncSession) -> None: + self._session = session + + async def record( + self, errors: list[FrontendErrorInput], browser: str + ) -> tuple[int, int]: + """Store a batch, discarding implausible timestamps. Returns (accepted, rejected).""" + now = datetime.now(UTC) + earliest, latest = now - MAX_CLOCK_SKEW, now + MAX_CLOCK_SKEW + + rows: list[FrontendErrorDB] = [] + rejected = 0 + + for error in errors: + occurred = error.occurred_at + if occurred.tzinfo is None: + occurred = occurred.replace(tzinfo=UTC) + if not (earliest <= occurred <= latest): + rejected += 1 + continue + + rows.append( + FrontendErrorDB( + app=error.app.value, + name=error.name, + message=error.message, + stack=error.stack, + path=error.path, + browser=browser, + occurred_at=occurred, + ) + ) + + if rows: + self._session.add_all(rows) + await self._session.commit() + + return len(rows), rejected + + async def grouped( + self, start: datetime, end: datetime, app: str | None, limit: int + ) -> list[dict]: + """ + Distinct errors with counts, ordered by how often they happen. + + Grouped by (app, name, message) rather than by stack: the same fault + reached from two routes produces two stacks and is one bug. + """ + conditions = [ + FrontendErrorDB.occurred_at >= start, + FrontendErrorDB.occurred_at < end, + ] + if app: + conditions.append(FrontendErrorDB.app == app) + + statement: Select = ( + select( + FrontendErrorDB.app, + FrontendErrorDB.name, + FrontendErrorDB.message, + func.count(FrontendErrorDB.id).label("occurrences"), + func.count(distinct(FrontendErrorDB.path)).label("affected_paths"), + func.array_agg(distinct(FrontendErrorDB.browser)).label("browsers"), + func.min(FrontendErrorDB.occurred_at).label("first_seen"), + func.max(FrontendErrorDB.occurred_at).label("last_seen"), + # Any one stack is representative; they differ only by frame + # addresses within a group. + func.min(FrontendErrorDB.stack).label("sample_stack"), + ) + .where(and_(*conditions)) + .group_by( + FrontendErrorDB.app, FrontendErrorDB.name, FrontendErrorDB.message + ) + .order_by(func.count(FrontendErrorDB.id).desc()) + .limit(limit) + ) + + result = await self._session.execute(statement) + return [ + { + "app": row.app, + "name": row.name, + "message": row.message, + "occurrences": int(row.occurrences), + "affected_paths": int(row.affected_paths), + "browsers": sorted(b for b in (row.browsers or []) if b), + "first_seen": row.first_seen, + "last_seen": row.last_seen, + "sample_stack": row.sample_stack, + } + for row in result + ] + + async def total(self, start: datetime, end: datetime, app: str | None) -> int: + conditions = [ + FrontendErrorDB.occurred_at >= start, + FrontendErrorDB.occurred_at < end, + ] + if app: + conditions.append(FrontendErrorDB.app == app) + result = await self._session.execute( + select(func.count(FrontendErrorDB.id)).where(and_(*conditions)) + ) + return int(result.scalar_one_or_none() or 0) + + +def get_frontend_error_repository(session: AsyncSession) -> FrontendErrorRepository: + return FrontendErrorRepository(session) diff --git a/src/app/shared/config/settings.py b/src/app/shared/config/settings.py index 76f2202..4ab900a 100644 --- a/src/app/shared/config/settings.py +++ b/src/app/shared/config/settings.py @@ -323,6 +323,14 @@ class Settings(BaseSettings): "endpoint returns 404 while this is false." ), ) + frontend_errors_enabled: bool = Field( + default=False, + description=( + "Accept uncaught error reports from the frontends. Off by default, " + "for the same reason as storefront analytics. The report endpoint " + "returns 404 while this is false." + ), + ) shop_timezone: str = Field( default="Europe/Berlin", description=( diff --git a/tests/test_frontend_errors_integration.py b/tests/test_frontend_errors_integration.py new file mode 100644 index 0000000..511fc22 --- /dev/null +++ b/tests/test_frontend_errors_integration.py @@ -0,0 +1,281 @@ +""" +Integration tests for frontend error reporting (S4). + +Runs against the live stack with FRONTEND_ERRORS_ENABLED=true. + + POST /v1/telemetry/errors + GET /v1/admin/telemetry/errors +""" + +import os +import subprocess +import uuid +from datetime import UTC, datetime, timedelta + +import pytest +import requests + +from auth_helpers import admin_headers + +_BASE = os.getenv("TEST_API_URL", "http://localhost:8000") +REPORT_URL = f"{_BASE}/v1/telemetry/errors" +ADMIN_URL = f"{_BASE}/v1/admin/telemetry/errors" + +_HEADERS = admin_headers() +RUN = uuid.uuid4().hex[:8] + +SAFARI = ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 " + "(KHTML, like Gecko) Version/18.3 Safari/605.1.15" +) + + +def _psql(sql: str) -> str: + result = subprocess.run( + [ + "docker", + "exec", + "opentaberna-db", + "psql", + "-U", + "opentaberna", + "-d", + "opentaberna", + "-t", + "-A", + "-c", + sql, + ], + check=True, + capture_output=True, + text=True, + ) + return result.stdout.strip() + + +def _now() -> str: + return datetime.now(UTC).isoformat() + + +def _report(errors: list[dict], user_agent: str = SAFARI) -> requests.Response: + return requests.post( + REPORT_URL, json={"errors": errors}, headers={"User-Agent": user_agent} + ) + + +def _message(suffix: str) -> str: + return f"it-{RUN}-{suffix}" + + +@pytest.fixture(scope="module", autouse=True) +def enabled_or_skip(): + response = requests.get(ADMIN_URL, headers=_HEADERS) + response.raise_for_status() + if not response.json()["enabled"]: + pytest.skip("FRONTEND_ERRORS_ENABLED is false on this deployment") + yield + _psql(f"DELETE FROM frontend_errors WHERE message LIKE 'it-{RUN}-%';") + + +# --------------------------------------------------------------------------- +# Access +# --------------------------------------------------------------------------- + + +def test_reporting_needs_no_authentication(): + """Storefront visitors are not signed in; their errors matter most.""" + response = _report( + [ + { + "app": "storefront", + "name": "TypeError", + "message": _message("anon"), + "occurred_at": _now(), + } + ] + ) + assert response.status_code == 202 + assert response.json()["accepted"] == 1 + + +def test_reading_errors_requires_admin(): + assert requests.get(ADMIN_URL).status_code in (401, 403) + + +# --------------------------------------------------------------------------- +# What must never be stored +# --------------------------------------------------------------------------- + + +def test_the_table_cannot_hold_an_ip_or_a_raw_user_agent(): + """ + The privacy posture is structural. `browser` holds a reduced label; there is + nowhere to put a fingerprint. + """ + columns = set( + _psql( + "SELECT column_name FROM information_schema.columns " + "WHERE table_name = 'frontend_errors';" + ).splitlines() + ) + + forbidden = { + "ip", + "ip_address", + "remote_addr", + "user_agent", + "email", + "customer_id", + "keycloak_user_id", + "user_id", + } + assert not (columns & forbidden), f"PII column present: {columns & forbidden}" + assert "browser" in columns + + +def test_the_raw_user_agent_is_reduced_before_storage(): + message = _message("ua") + _report( + [ + { + "app": "storefront", + "name": "TypeError", + "message": message, + "occurred_at": _now(), + } + ], + user_agent=SAFARI, + ) + + stored = _psql(f"SELECT browser FROM frontend_errors WHERE message = '{message}';") + assert stored == "Safari 18" + assert "AppleWebKit" not in stored + assert "Macintosh" not in stored + + +def test_a_query_string_carrying_a_token_is_not_stored(): + message = _message("qs") + _report( + [ + { + "app": "storefront", + "name": "TypeError", + "message": message, + "path": "/checkout?token=supersecret", + "occurred_at": _now(), + } + ] + ) + + stored = _psql(f"SELECT path FROM frontend_errors WHERE message = '{message}';") + assert stored == "/checkout" + assert "supersecret" not in stored + + +# --------------------------------------------------------------------------- +# Hostile and merely broken input +# --------------------------------------------------------------------------- + + +def test_an_unknown_app_is_refused(): + response = _report( + [ + { + "app": "wordpress", + "name": "TypeError", + "message": _message("bad"), + "occurred_at": _now(), + } + ] + ) + assert response.status_code == 422 + + +def test_stale_timestamps_are_discarded(): + old = (datetime.now(UTC) - timedelta(days=400)).isoformat() + response = _report( + [ + { + "app": "storefront", + "name": "TypeError", + "message": _message("stale"), + "occurred_at": old, + }, + { + "app": "storefront", + "name": "TypeError", + "message": _message("fresh"), + "occurred_at": _now(), + }, + ] + ) + body = response.json() + assert body["accepted"] == 1 + assert body["rejected"] == 1 + + +# --------------------------------------------------------------------------- +# Grouping +# --------------------------------------------------------------------------- + + +def test_the_same_fault_on_two_routes_is_one_group(): + """ + One bug produces thousands of identical rows. Reading them ungrouped is + useless, and grouping by stack would split one bug into many. + """ + message = _message("grouped") + _report( + [ + { + "app": "storefront", + "name": "TypeError", + "message": message, + "path": "/shop/1", + "stack": "at Product.render", + "occurred_at": _now(), + }, + { + "app": "storefront", + "name": "TypeError", + "message": message, + "path": "/shop/2", + "stack": "at Product.render (other frame)", + "occurred_at": _now(), + }, + ] + ) + + payload = requests.get(ADMIN_URL, headers=_HEADERS, params={"limit": 200}).json() + group = next(g for g in payload["groups"] if g["message"] == message) + + assert group["occurrences"] == 2 + assert group["affected_paths"] == 2 + assert group["browsers"] == ["Safari 18"] + assert group["sample_stack"] + + +def test_groups_are_ordered_by_how_often_they_happen(): + payload = requests.get(ADMIN_URL, headers=_HEADERS, params={"limit": 200}).json() + counts = [g["occurrences"] for g in payload["groups"]] + assert counts == sorted(counts, reverse=True) + + +def test_errors_can_be_filtered_to_one_application(): + _report( + [ + { + "app": "admin", + "name": "HttpErrorResponse", + "message": _message("adminonly"), + "occurred_at": _now(), + } + ] + ) + + payload = requests.get( + ADMIN_URL, headers=_HEADERS, params={"app": "admin", "limit": 200} + ).json() + + assert payload["groups"] + assert all(g["app"] == "admin" for g in payload["groups"]) diff --git a/tests/test_frontend_errors_unit.py b/tests/test_frontend_errors_unit.py new file mode 100644 index 0000000..9610296 --- /dev/null +++ b/tests/test_frontend_errors_unit.py @@ -0,0 +1,139 @@ +""" +Unit tests for frontend error reporting — schema and user-agent reduction. + +The report endpoint is public, so its schema is a boundary. And the whole point +of reducing the user agent is that a fingerprint never reaches storage, which is +a property worth pinning rather than trusting. +""" + +from datetime import UTC, datetime + +import pytest +from pydantic import ValidationError as PydanticValidationError + +from app.services.frontend_errors.functions import coarse_browser +from app.services.frontend_errors.models import ( + MAX_ERRORS_PER_BATCH, + MAX_STACK_CHARS, + FrontendErrorBatch, + FrontendErrorInput, +) + + +def _error(**overrides) -> dict: + base = { + "app": "storefront", + "name": "TypeError", + "message": "undefined is not a function", + "occurred_at": datetime.now(UTC), + } + base.update(overrides) + return base + + +# --------------------------------------------------------------------------- +# The user agent is reduced, never stored +# --------------------------------------------------------------------------- + + +@pytest.mark.parametrize( + "user_agent,expected", + [ + ( + "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 " + "(KHTML, like Gecko) Version/18.3 Safari/605.1.15", + "Safari 18", + ), + ( + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 " + "(KHTML, like Gecko) Chrome/140.0.0.0 Safari/537.36", + "Chrome 140", + ), + ( + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) Gecko/20100101 Firefox/133.0", + "Firefox 133", + ), + ], +) +def test_common_browsers_are_reduced_to_family_and_major(user_agent, expected): + assert coarse_browser(user_agent) == expected + + +def test_edge_is_not_reported_as_chrome(): + """ + Edge, Opera and Chrome all claim to be one another. The most specific + pattern has to win, or every error looks like it came from Chrome. + """ + edge = ( + "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 " + "(KHTML, like Gecko) Chrome/140.0.0.0 Safari/537.36 Edg/140.0.0.0" + ) + assert coarse_browser(edge) == "Edge 140" + + +def test_nothing_from_the_original_string_survives(): + """ + The reduction is the privacy guarantee. Whatever a client sends, the result + is a known family name and an integer — never a fragment of the input. + """ + hostile = "Mozilla/5.0 Chrome/140 user=alice@example.com token=abc123" + result = coarse_browser(hostile) + + assert result == "Chrome 140" + assert "alice@example.com" not in result + assert "abc123" not in result + + +def test_an_unrecognised_agent_is_unknown_not_stored_verbatim(): + assert coarse_browser("something entirely made up") == "unknown" + assert coarse_browser(None) == "unknown" + assert coarse_browser("") == "unknown" + + +# --------------------------------------------------------------------------- +# The schema as a boundary +# --------------------------------------------------------------------------- + + +def test_query_strings_are_stripped_from_the_path(): + error = FrontendErrorInput(**_error(path="/checkout?token=secret&email=a@b.c")) + assert error.path == "/checkout" + + +def test_a_huge_stack_is_truncated_rather_than_rejected(): + """ + A stack trace is unbounded input from a public endpoint, and also the most + useful field here. Cutting it keeps the top frames, where the fault is. + """ + error = FrontendErrorInput(**_error(stack="x" * (MAX_STACK_CHARS * 3))) + + assert len(error.stack) < MAX_STACK_CHARS + 100 + assert error.stack.endswith("truncated") + + +def test_unknown_apps_are_refused(): + """A closed set, so the field cannot become free text from a public endpoint.""" + with pytest.raises(PydanticValidationError): + FrontendErrorInput(**_error(app="not-an-app")) + + +def test_extra_fields_are_refused(): + with pytest.raises(PydanticValidationError): + FrontendErrorInput(**_error(user_agent="Mozilla/5.0")) + + with pytest.raises(PydanticValidationError): + FrontendErrorInput(**_error(customer_id="123")) + + +def test_batch_size_is_capped(): + """ + A component throwing inside a render loop is the normal failure mode, and + it will report as fast as the browser can loop. + """ + with pytest.raises(PydanticValidationError): + FrontendErrorBatch(errors=[_error() for _ in range(MAX_ERRORS_PER_BATCH + 1)]) + + +def test_message_length_is_bounded(): + with pytest.raises(PydanticValidationError): + FrontendErrorInput(**_error(message="x" * 5000))