diff --git a/Docs/Backend-Development.md b/Docs/Backend-Development.md index 7d10404..99a4c4e 100644 --- a/Docs/Backend-Development.md +++ b/Docs/Backend-Development.md @@ -521,6 +521,129 @@ total_pages = compute_total_pages(total, page_size) --- +## 4.2 Error Response Schema + +All error responses use a consistent machine-readable format that enables frontend code to branch reliably on error conditions without string-parsing error detail text. + +### Error Response Format + +Every non-2xx HTTP response body is a JSON object with this structure: + +```json +{ + "code": "jail_not_found", + "detail": "Jail 'example' not found", + "metadata": { + "jail_name": "example" + } +} +``` + +**Fields:** +- **`code`** (string, required): Machine-readable error code for client-side branching. Examples: `jail_not_found`, `rate_limit_exceeded`, `authentication_required`. +- **`detail`** (string, required): Human-readable error message. Safe for displaying to users. +- **`metadata`** (object, optional): Structured context data relevant to the error. Only includes data safe for client consumption (no sensitive internal state). Examples: offending parameter names, resource identifiers, time windows. + +### Exception Hierarchy & Error Codes + +All domain exceptions inherit from `DomainError` (defined in `backend/app/exceptions.py`) and are organized by HTTP status category: + +| HTTP Status | Category Class | Error Codes | Use Case | +|---|---|---|---| +| **404** | `NotFoundError` | `not_found`, `jail_not_found`, `filter_not_found`, `action_not_found`, `config_file_not_found`, `blocklist_source_not_found`, `history_not_found` | Requested resource does not exist | +| **400** | `BadRequestError` | `invalid_input`, `config_validation_failed`, `config_operation_failed`, `jail_name_invalid`, `filter_name_invalid`, `action_name_invalid`, `config_file_name_invalid`, `filter_invalid_regex` | Invalid input, validation failure, malformed request | +| **409** | `ConflictError` | `conflict`, `jail_operation_failed`, `jail_already_active`, `jail_already_inactive`, `jail_not_in_config`, `action_already_exists`, `filter_already_exists`, `config_file_exists` | State conflict, resource already exists, invalid state transition | +| **500** | `OperationError` | `operation_failed`, `config_write_failed`, `config_file_write_failed`, `server_operation_failed`, `fail2ban_protocol_error` | Operation failure, write errors, unexpected failures | +| **503** | `ServiceUnavailableError` | `service_unavailable`, `config_dir_unavailable`, `fail2ban_unreachable` | Infrastructure/external service issues, temporary unavailability | +| **401** | `AuthenticationError` | `authentication_required` | Authentication or authorization failure, invalid/expired credentials | +| **429** | `RateLimitError` | `rate_limit_exceeded` | Rate limit exceeded, too many requests | + +### Implementing Error Handlers + +Every exception category has a corresponding exception handler registered in `backend/app/main.py`. When a domain exception is raised: + +1. FastAPI's exception handling middleware catches it. +2. The registered handler converts it to an `ErrorResponse` with HTTP status code. +3. The response is serialized as JSON with `code`, `detail`, and `metadata` fields. + +**Pattern for service code:** + +```python +from app.exceptions import JailNotFoundError, ConfigValidationError + +async def get_jail(name: str) -> Jail: + """Raises JailNotFoundError if jail not found.""" + jail = await db.fetchone("SELECT * FROM jails WHERE name = ?", (name,)) + if jail is None: + raise JailNotFoundError(name) # HTTP 404, code='jail_not_found' + return jail + +async def apply_config(config: JailConfig) -> None: + """Raises ConfigValidationError if invalid.""" + if not config.filter_name: + raise ConfigValidationError("filter_name is required") # HTTP 400, code='config_validation_failed' +``` + +### Adding New Exception Types + +1. **Choose the appropriate category** based on the HTTP status (NotFoundError for 404, BadRequestError for 400, etc.). +2. **Create a subclass** in `backend/app/exceptions.py`: + +```python +class MySpecificError(BadRequestError): + """Raised when X happens.""" + + error_code: str = "my_specific_error" + + def __init__(self, detail_msg: str, **context) -> None: + self.context = context + super().__init__(detail_msg) + + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + """Return only safe, relevant metadata.""" + return {k: v for k, v in self.context.items() if k in ("offending_value", "constraint")} +``` + +3. **Use explicit error codes** — Don't derive them from the class name. This makes them self-documenting and prevents breakage on class renames. +4. **Implement `get_error_metadata()`** — Return only data safe for client consumption. Never leak internal state, file paths, or system details. +5. **Raise from service code** — Never from repositories or utils. Exceptions represent business logic violations, not infrastructure errors. + +**What NOT to do:** +- ❌ Don't raise `HTTPException` from service code (bypass the ErrorResponse format). +- ❌ Don't put sensitive information in `metadata` (database paths, SQL, internal IDs). +- ❌ Don't derive error codes from class names using regex (fragile and non-self-documenting). + +### Frontend Error Parsing + +The frontend `ApiError` class parses error responses automatically: + +```typescript +import { api } from "src/api/client"; + +try { + const jail = await api.get("/jails/example"); +} catch (error) { + if (error instanceof ApiError) { + const code = error.errorResponse?.code; + + if (code === "jail_not_found") { + // Handle not found + console.log("Jail does not exist:", error.errorResponse?.metadata?.jail_name); + } else if (code === "rate_limit_exceeded") { + // Handle rate limit + showRateLimitModal(); + } else if (code === "authentication_required") { + // Handle auth — the frontend framework auto-redirects to /login + redirectToLogin(); + } + } +} +``` + +The `errorResponse` field contains the parsed error object with `code`, `detail`, and `metadata` fields, enabling reliable machine-readable branching. + +--- + ## 5. Pydantic Models ### Base Class diff --git a/Docs/Tasks.md b/Docs/Tasks.md index 14a9639..832ac0b 100644 --- a/Docs/Tasks.md +++ b/Docs/Tasks.md @@ -1,24 +1,3 @@ - -## 26) Pagination contract is not standardized across endpoints -- Where found: - - [backend/app/routers/dashboard.py](backend/app/routers/dashboard.py) - - [backend/app/routers/history.py](backend/app/routers/history.py) - - [backend/app/routers/blocklist.py](backend/app/routers/blocklist.py) -- Why this is needed: - - Different pagination patterns increase frontend complexity. -- Goal: - - Use one shared paginated response model. -- What to do: - - Introduce common pagination schema and query parameter policy. -- Possible traps and issues: - - Existing endpoint consumers may require transition period. -- Docs changes needed: - - Add pagination API spec. -- Doc references: - - [Docs/Backend-Development.md](Docs/Backend-Development.md) - ---- - ## 27) Error response body shape is inconsistent - Where found: - [backend/app/main.py](backend/app/main.py) diff --git a/backend/app/exceptions.py b/backend/app/exceptions.py index fea06c2..349182c 100644 --- a/backend/app/exceptions.py +++ b/backend/app/exceptions.py @@ -10,11 +10,17 @@ All domain exceptions inherit from one of these base categories: - **ConflictError** (409): State conflict, resource already exists, invalid state transition - **OperationError** (500): Operation failure, write errors - **ServiceUnavailableError** (503): Infrastructure/external service issues +- **AuthenticationError** (401): Authentication or authorization failure +- **RateLimitError** (429): Rate limit exceeded Service exceptions inherit from the appropriate category, allowing routers to handle categories rather than individual exception types. Exception handlers in main.py register only base category types. +Every exception class has: +- **error_code**: A machine-readable error code for client-side branching +- **get_error_metadata()**: Returns structured metadata for the API response + Example: def get_jail(name: str) -> Jail: # Raises JailNotFoundError (subclass of NotFoundError) @@ -22,52 +28,84 @@ Example: @app.exception_handler(NotFoundError) async def handle_not_found(request, exc): - return JSONResponse(status_code=404, content={"detail": str(exc)}) + return JSONResponse(status_code=404, content=ErrorResponse( + code=exc.error_code, + detail=str(exc), + metadata=exc.get_error_metadata() + ).model_dump()) See Backend-Development.md for the complete exception contract. """ from __future__ import annotations + # --------------------------------------------------------------------------- # Exception Base Classes (Categories) # --------------------------------------------------------------------------- class DomainError(Exception): - """Base class for all domain exceptions.""" + """Base class for all domain exceptions. + + All domain exceptions must: + 1. Define an `error_code` class attribute (machine-readable error code) + 2. Implement `get_error_metadata()` to return structured error context + """ - pass + error_code: str = "internal_error" + + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + """Return structured metadata for the API error response. + + Subclasses should override to expose only safe, relevant metadata. + + Returns: + A dictionary of metadata key-value pairs safe for client consumption. + """ + return {} class NotFoundError(DomainError): """Raised when a requested domain entity is not found. HTTP 404.""" - pass + error_code: str = "not_found" class BadRequestError(DomainError): """Raised for invalid input, validation failures, or invalid identifiers. HTTP 400.""" - pass + error_code: str = "invalid_input" class ConflictError(DomainError): """Raised for state conflicts or resource constraints. HTTP 409.""" - pass + error_code: str = "conflict" class OperationError(DomainError): """Raised when a domain operation fails (write, update, etc.). HTTP 500.""" - pass + error_code: str = "operation_failed" class ServiceUnavailableError(DomainError): """Raised for infrastructure or external service issues. HTTP 503.""" - pass + error_code: str = "service_unavailable" + + +class AuthenticationError(DomainError): + """Raised for authentication or authorization failures. HTTP 401.""" + + error_code: str = "authentication_required" + + +class RateLimitError(DomainError): + """Raised when a client exceeds rate limits. HTTP 429.""" + + error_code: str = "rate_limit_exceeded" # --------------------------------------------------------------------------- @@ -78,30 +116,45 @@ class ServiceUnavailableError(DomainError): class JailNotFoundError(NotFoundError): """Raised when a requested jail name does not exist.""" + error_code: str = "jail_not_found" + def __init__(self, name: str) -> None: self.name = name super().__init__(f"Jail not found: {name!r}") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"jail_name": self.name} + class JailOperationError(ConflictError): """Raised when a jail state operation fails (e.g. start/stop already in progress).""" + error_code: str = "jail_operation_failed" + class ConfigValidationError(BadRequestError): """Raised when config values fail validation before applying.""" + error_code: str = "config_validation_failed" + class ConfigOperationError(BadRequestError): """Raised when a config payload update or command fails.""" + error_code: str = "config_operation_failed" + class ConfigDirError(ServiceUnavailableError): """Raised when the fail2ban config directory is missing or inaccessible.""" + error_code: str = "config_dir_unavailable" + class ConfigFileNotFoundError(NotFoundError): """Raised when a requested config file does not exist.""" + error_code: str = "config_file_not_found" + def __init__(self, filename: str) -> None: """Initialize with the filename that was not found. @@ -111,10 +164,15 @@ class ConfigFileNotFoundError(NotFoundError): self.filename = filename super().__init__(f"Config file not found: {filename!r}") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"filename": self.filename} + class ConfigFileExistsError(ConflictError): """Raised when trying to create a file that already exists.""" + error_code: str = "config_file_exists" + def __init__(self, filename: str) -> None: """Initialize with the filename that already exists. @@ -124,22 +182,33 @@ class ConfigFileExistsError(ConflictError): self.filename = filename super().__init__(f"Config file already exists: {filename!r}") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"filename": self.filename} + class ConfigFileWriteError(OperationError): """Raised when a file cannot be written (permissions, disk full, etc.).""" + error_code: str = "config_file_write_failed" + class ConfigFileNameError(BadRequestError): """Raised when a supplied filename is invalid or unsafe.""" + error_code: str = "config_file_name_invalid" + class ServerOperationError(BadRequestError): """Raised when a server control command (e.g. refresh) fails.""" + error_code: str = "server_operation_failed" + class Fail2BanConnectionError(ServiceUnavailableError): """Raised when the fail2ban socket is unreachable or returns an error.""" + error_code: str = "fail2ban_unreachable" + def __init__(self, message: str, socket_path: str) -> None: """Initialize with a human-readable message and the socket path. @@ -150,112 +219,215 @@ class Fail2BanConnectionError(ServiceUnavailableError): self.socket_path: str = socket_path super().__init__(f"{message} (socket: {socket_path})") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"socket_path": self.socket_path} + class Fail2BanProtocolError(ServiceUnavailableError): """Raised when the response from fail2ban cannot be parsed.""" + error_code: str = "fail2ban_protocol_error" + class FilterInvalidRegexError(BadRequestError): """Raised when a regex pattern fails to compile.""" + error_code: str = "filter_invalid_regex" + def __init__(self, pattern: str, error: str) -> None: """Initialize with the invalid pattern and compile error.""" self.pattern = pattern self.error = error super().__init__(f"Invalid regex {pattern!r}: {error}") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"pattern": self.pattern, "error": self.error} + class JailNotFoundInConfigError(NotFoundError): """Raised when the requested jail name is not defined in any config file.""" + error_code: str = "jail_not_in_config" + def __init__(self, name: str) -> None: self.name = name super().__init__(f"Jail not found in config: {name!r}") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"jail_name": self.name} + class ConfigWriteError(OperationError): """Raised when writing a configuration file modification fails.""" + error_code: str = "config_write_failed" + def __init__(self, message: str) -> None: self.message = message super().__init__(message) + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"message": self.message} + class JailNameError(BadRequestError): """Raised when a jail name contains invalid characters.""" + error_code: str = "jail_name_invalid" + class JailAlreadyActiveError(ConflictError): """Raised when trying to activate a jail that is already active.""" + error_code: str = "jail_already_active" + def __init__(self, name: str) -> None: self.name = name super().__init__(f"Jail is already active: {name!r}") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"jail_name": self.name} + class JailAlreadyInactiveError(ConflictError): """Raised when trying to deactivate a jail that is already inactive.""" + error_code: str = "jail_already_inactive" + def __init__(self, name: str) -> None: self.name = name super().__init__(f"Jail is already inactive: {name!r}") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"jail_name": self.name} + class FilterNotFoundError(NotFoundError): """Raised when the requested filter name is not found.""" + error_code: str = "filter_not_found" + def __init__(self, name: str) -> None: self.name = name super().__init__(f"Filter not found: {name!r}") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"filter_name": self.name} + class FilterAlreadyExistsError(ConflictError): """Raised when trying to create a filter whose `.conf` or `.local` already exists.""" + error_code: str = "filter_already_exists" + def __init__(self, name: str) -> None: self.name = name super().__init__(f"Filter already exists: {name!r}") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"filter_name": self.name} + class FilterNameError(BadRequestError): """Raised when a filter name contains invalid characters.""" + error_code: str = "filter_name_invalid" + class FilterReadonlyError(ConflictError): """Raised when trying to delete a shipped `.conf` filter with no `.local` override.""" + error_code: str = "filter_readonly" + def __init__(self, name: str) -> None: self.name = name super().__init__( f"Filter {name!r} is a shipped default (.conf only); only user-created .local files can be deleted." ) + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"filter_name": self.name} + class ActionNotFoundError(NotFoundError): """Raised when the requested action name is not found.""" + error_code: str = "action_not_found" + def __init__(self, name: str) -> None: self.name = name super().__init__(f"Action not found: {name!r}") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"action_name": self.name} + class ActionAlreadyExistsError(ConflictError): """Raised when trying to create an action whose `.conf` or `.local` already exists.""" + error_code: str = "action_already_exists" + def __init__(self, name: str) -> None: self.name = name super().__init__(f"Action already exists: {name!r}") + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"action_name": self.name} + class ActionNameError(BadRequestError): """Raised when an action name contains invalid characters.""" + error_code: str = "action_name_invalid" + class ActionReadonlyError(ConflictError): """Raised when trying to delete a shipped `.conf` action with no `.local` override.""" + error_code: str = "action_readonly" + def __init__(self, name: str) -> None: self.name = name super().__init__( f"Action {name!r} is a shipped default (.conf only); only user-created .local files can be deleted." ) + + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"action_name": self.name} + + + +class SetupAlreadyCompleteError(ConflictError): + """Raised when attempting to run setup when it has already been completed.""" + + error_code: str = "setup_already_complete" + + def __init__(self) -> None: + super().__init__("Setup has already been completed.") + + + +class BlocklistSourceNotFoundError(NotFoundError): + """Raised when a blocklist source is not found.""" + + error_code: str = "blocklist_source_not_found" + + def __init__(self, source_id: int) -> None: + self.source_id = source_id + super().__init__(f"Blocklist source not found: {source_id}") + + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"source_id": self.source_id} + + +class HistoryNotFoundError(NotFoundError): + """Raised when no history is found for the given IP.""" + + error_code: str = "history_not_found" + + def __init__(self, ip: str) -> None: + self.ip = ip + super().__init__(f"No history found for IP: {ip}") + + def get_error_metadata(self) -> dict[str, str | int | float | bool | None]: + return {"ip": self.ip} diff --git a/backend/app/main.py b/backend/app/main.py index 17a34f0..31621f6 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -22,7 +22,7 @@ if TYPE_CHECKING: from starlette.responses import Response as StarletteResponse import structlog -from fastapi import FastAPI, Request, status +from fastapi import FastAPI, HTTPException, Request, status from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse, RedirectResponse from starlette.middleware.base import BaseHTTPMiddleware @@ -30,12 +30,14 @@ from starlette.middleware.base import BaseHTTPMiddleware from app import __version__ from app.config import Settings, get_settings from app.exceptions import ( + AuthenticationError, BadRequestError, ConflictError, Fail2BanConnectionError, Fail2BanProtocolError, NotFoundError, OperationError, + RateLimitError, ServiceUnavailableError, ) from app.middleware.csrf import CsrfMiddleware @@ -54,6 +56,7 @@ from app.routers import ( setup, ) from app.startup import startup_shared_resources +from app.models.response import ErrorResponse from app.utils.rate_limiter import RateLimiter from app.utils.runtime_state import ApplicationState, RuntimeState from app.utils.session_cache import InMemorySessionCache, NoOpSessionCache @@ -163,6 +166,43 @@ async def _lifespan(app: FastAPI) -> AsyncGenerator[None, None]: # --------------------------------------------------------------------------- +def _get_error_code(exc: Exception) -> str: + """Get the machine-readable error code from an exception. + + First checks if the exception has an error_code class attribute. + Falls back to converting the exception class name to snake_case. + + Args: + exc: The exception instance. + + Returns: + A snake_case error code string. + """ + if hasattr(exc, "error_code"): + return exc.error_code + + exc_name = exc.__class__.__name__ + import re + snake_case = re.sub(r"(? dict[str, str | int | float | bool | None]: + """Get structured metadata from an exception. + + Calls the exception's get_error_metadata() method if available. + + Args: + exc: The exception instance. + + Returns: + A dictionary of metadata safe for API responses. + """ + if hasattr(exc, "get_error_metadata") and callable(exc.get_error_metadata): + return exc.get_error_metadata() + return {} + + async def _unhandled_exception_handler( request: Request, exc: Exception, @@ -185,9 +225,14 @@ async def _unhandled_exception_handler( method=request.method, exc_info=exc, ) + error_response = ErrorResponse( + code="internal_error", + detail="An unexpected error occurred. Please try again later.", + metadata={}, + ) return JSONResponse( status_code=500, - content={"detail": "An unexpected error occurred. Please try again later."}, + content=error_response.model_dump(), ) @@ -210,9 +255,14 @@ async def _fail2ban_connection_handler( method=request.method, error=str(exc), ) + error_response = ErrorResponse( + code="fail2ban_unreachable", + detail="Cannot reach the fail2ban service. Check the server status page.", + metadata={"socket_path": exc.socket_path}, + ) return JSONResponse( status_code=502, - content={"detail": "Cannot reach the fail2ban service. Check the server status page."}, + content=error_response.model_dump(), ) @@ -235,9 +285,14 @@ async def _fail2ban_protocol_handler( method=request.method, error=str(exc), ) + error_response = ErrorResponse( + code="fail2ban_protocol_error", + detail="Cannot reach the fail2ban service. Check the server status page.", + metadata={}, + ) return JSONResponse( status_code=502, - content={"detail": "Cannot reach the fail2ban service. Check the server status page."}, + content=error_response.model_dump(), ) @@ -260,9 +315,14 @@ async def _not_found_handler( method=request.method, error=str(exc), ) + error_response = ErrorResponse( + code=_get_error_code(exc), + detail=str(exc), + metadata=_get_error_metadata(exc), + ) return JSONResponse( status_code=status.HTTP_404_NOT_FOUND, - content={"detail": str(exc)}, + content=error_response.model_dump(), ) @@ -285,9 +345,14 @@ async def _bad_request_handler( method=request.method, error=str(exc), ) + error_response = ErrorResponse( + code=_get_error_code(exc), + detail=str(exc), + metadata=_get_error_metadata(exc), + ) return JSONResponse( status_code=status.HTTP_400_BAD_REQUEST, - content={"detail": str(exc)}, + content=error_response.model_dump(), ) @@ -302,9 +367,14 @@ async def _conflict_handler( method=request.method, error=str(exc), ) + error_response = ErrorResponse( + code=_get_error_code(exc), + detail=str(exc), + metadata=_get_error_metadata(exc), + ) return JSONResponse( status_code=status.HTTP_409_CONFLICT, - content={"detail": str(exc)}, + content=error_response.model_dump(), ) @@ -320,9 +390,14 @@ async def _domain_error_handler( error=str(exc), exc_info=exc, ) + error_response = ErrorResponse( + code=_get_error_code(exc), + detail=str(exc), + metadata=_get_error_metadata(exc), + ) return JSONResponse( status_code=status.HTTP_500_INTERNAL_SERVER_ERROR, - content={"detail": str(exc)}, + content=error_response.model_dump(), ) @@ -345,9 +420,14 @@ async def _value_error_handler( method=request.method, error=str(exc), ) + error_response = ErrorResponse( + code="invalid_input", + detail=str(exc), + metadata={}, + ) return JSONResponse( status_code=status.HTTP_400_BAD_REQUEST, - content={"detail": str(exc)}, + content=error_response.model_dump(), ) @@ -370,9 +450,126 @@ async def _service_unavailable_handler( method=request.method, error=str(exc), ) + error_response = ErrorResponse( + code=_get_error_code(exc), + detail=str(exc), + metadata=_get_error_metadata(exc), + ) return JSONResponse( status_code=status.HTTP_503_SERVICE_UNAVAILABLE, - content={"detail": str(exc)}, + content=error_response.model_dump(), + ) + + +async def _authentication_error_handler( + request: Request, + exc: AuthenticationError, +) -> JSONResponse: + """Return a ``401 Unauthorized`` response for authentication failures. + + Args: + request: The incoming FastAPI request. + exc: The :class:`~app.exceptions.AuthenticationError`. + + Returns: + A :class:`fastapi.responses.JSONResponse` with status 401. + """ + log.warning( + "authentication_error", + path=request.url.path, + method=request.method, + error=str(exc), + ) + error_response = ErrorResponse( + code=_get_error_code(exc), + detail=str(exc), + metadata=_get_error_metadata(exc), + ) + return JSONResponse( + status_code=status.HTTP_401_UNAUTHORIZED, + content=error_response.model_dump(), + ) + + +async def _rate_limit_error_handler( + request: Request, + exc: RateLimitError, +) -> JSONResponse: + """Return a ``429 Too Many Requests`` response for rate limit exceeded errors. + + Args: + request: The incoming FastAPI request. + exc: The :class:`~app.exceptions.RateLimitError`. + + Returns: + A :class:`fastapi.responses.JSONResponse` with status 429 and Retry-After header. + """ + log.warning( + "rate_limit_exceeded", + path=request.url.path, + method=request.method, + error=str(exc), + ) + error_response = ErrorResponse( + code=_get_error_code(exc), + detail=str(exc), + metadata=_get_error_metadata(exc), + ) + return JSONResponse( + status_code=status.HTTP_429_TOO_MANY_REQUESTS, + content=error_response.model_dump(), + headers={"Retry-After": "60"}, + ) + + +async def _http_exception_handler( + request: Request, + exc: HTTPException, +) -> JSONResponse: + """Return a standardized error response for FastAPI HTTPException. + + This handler standardizes responses from FastAPI validation errors, + path parameter mismatches, and other built-in validation failures + to use the ErrorResponse envelope with a machine-readable error code. + + Args: + request: The incoming FastAPI request. + exc: The :class:`fastapi.HTTPException`. + + Returns: + A :class:`fastapi.responses.JSONResponse` with the original status code. + """ + log.warning( + "http_exception", + path=request.url.path, + method=request.method, + status_code=exc.status_code, + error=exc.detail, + ) + + error_code_map = { + status.HTTP_400_BAD_REQUEST: "invalid_input", + status.HTTP_401_UNAUTHORIZED: "authentication_required", + status.HTTP_403_FORBIDDEN: "forbidden", + status.HTTP_404_NOT_FOUND: "not_found", + status.HTTP_409_CONFLICT: "conflict", + status.HTTP_422_UNPROCESSABLE_ENTITY: "invalid_input", + status.HTTP_429_TOO_MANY_REQUESTS: "rate_limit_exceeded", + status.HTTP_500_INTERNAL_SERVER_ERROR: "internal_error", + status.HTTP_503_SERVICE_UNAVAILABLE: "service_unavailable", + } + + error_code = error_code_map.get(exc.status_code, "internal_error") + error_response = ErrorResponse( + code=error_code, + detail=exc.detail, + metadata={}, + ) + + return JSONResponse( + status_code=exc.status_code, + content=error_response.model_dump(), + headers=exc.headers or {}, ) @@ -509,11 +706,14 @@ def create_app(settings: Settings | None = None) -> FastAPI: # rather than falling through to the generic 500 handler. app.add_exception_handler(Fail2BanConnectionError, _fail2ban_connection_handler) # type: ignore[arg-type] app.add_exception_handler(Fail2BanProtocolError, _fail2ban_protocol_handler) # type: ignore[arg-type] + app.add_exception_handler(AuthenticationError, _authentication_error_handler) # type: ignore[arg-type] + app.add_exception_handler(RateLimitError, _rate_limit_error_handler) # type: ignore[arg-type] app.add_exception_handler(NotFoundError, _not_found_handler) app.add_exception_handler(BadRequestError, _bad_request_handler) app.add_exception_handler(ConflictError, _conflict_handler) app.add_exception_handler(OperationError, _domain_error_handler) app.add_exception_handler(ServiceUnavailableError, _service_unavailable_handler) + app.add_exception_handler(HTTPException, _http_exception_handler) # type: ignore[arg-type] app.add_exception_handler(ValueError, _value_error_handler) # type: ignore[arg-type] app.add_exception_handler(Exception, _unhandled_exception_handler) diff --git a/backend/app/models/response.py b/backend/app/models/response.py index 9ac5ed9..addca03 100644 --- a/backend/app/models/response.py +++ b/backend/app/models/response.py @@ -205,3 +205,48 @@ class CommandResponse(BanGuiBaseModel): default=True, description="Whether the command succeeded (false for errors in non-exception handlers).", ) + + +class ErrorResponse(BanGuiBaseModel): + """Standardized error response envelope for all API errors. + + Use this for all error responses to ensure consistent client-side error handling. + The error code enables machine-readable branching, while detail provides + human-readable context. Metadata offers optional structured context. + + Fields: + code: Machine-readable error code (e.g., "jail_not_found", "invalid_input"). + detail: Human-readable error description for display to users. + metadata: Optional structured context (e.g., field names, constraint violations). + + Example: + ```python + # 404 Not Found + { + "code": "jail_not_found", + "detail": "Jail 'sshd' not found", + "metadata": {"jail_name": "sshd"} + } + + # 400 Bad Request - Validation Error + { + "code": "invalid_input", + "detail": "Invalid IP address format", + "metadata": {"field": "ip", "value": "999.999.999.999"} + } + + # 409 Conflict + { + "code": "jail_already_active", + "detail": "Jail is already active: 'sshd'", + "metadata": {"jail_name": "sshd", "current_status": "active"} + } + ``` + """ + + code: str = Field(..., description="Machine-readable error code for client-side branching.") + detail: str = Field(..., description="Human-readable error description.") + metadata: dict[str, str | int | float | bool | None] = Field( + default_factory=dict, + description="Optional structured context for the error.", + ) diff --git a/backend/app/routers/auth.py b/backend/app/routers/auth.py index 27ab22e..0cf93b6 100644 --- a/backend/app/routers/auth.py +++ b/backend/app/routers/auth.py @@ -22,7 +22,7 @@ from __future__ import annotations import asyncio import structlog -from fastapi import APIRouter, HTTPException, Request, Response, status +from fastapi import APIRouter, Request, Response, status from app.dependencies import ( AuthDep, @@ -31,6 +31,7 @@ from app.dependencies import ( SessionServiceContextDep, SettingsDep, ) +from app.exceptions import AuthenticationError, RateLimitError from app.models.auth import LoginRequest, LoginResponse, LogoutResponse from app.services import auth_service from app.utils.client_ip import get_client_ip @@ -79,18 +80,14 @@ async def login( :class:`~app.models.auth.LoginResponse` containing the token. Raises: - HTTPException: 401 if the password is incorrect. - HTTPException: 429 if the rate limit is exceeded. + AuthenticationError: if the password is incorrect. + RateLimitError: if the rate limit is exceeded. """ client_ip = get_client_ip(request, trusted_proxies=_TRUSTED_PROXIES) if not rate_limiter.is_allowed(client_ip): log.warning("login_rate_limit_exceeded", client_ip=client_ip) - raise HTTPException( - status_code=status.HTTP_429_TOO_MANY_REQUESTS, - detail="Too many login attempts. Please try again later.", - headers={"Retry-After": "60"}, - ) + raise RateLimitError("Too many login attempts. Please try again later.") try: signed_token, expires_at = await auth_service.login( @@ -106,10 +103,7 @@ async def login( # but an extra 10 seconds makes automation much less feasible. await asyncio.sleep(10.0) log.warning("login_failed", client_ip=client_ip, error=str(exc)) - raise HTTPException( - status_code=status.HTTP_401_UNAUTHORIZED, - detail=str(exc), - ) from exc + raise AuthenticationError(str(exc)) from exc response.set_cookie( key=SESSION_COOKIE_NAME, diff --git a/backend/app/routers/blocklist.py b/backend/app/routers/blocklist.py index 12ee74d..e9b7fba 100644 --- a/backend/app/routers/blocklist.py +++ b/backend/app/routers/blocklist.py @@ -22,7 +22,7 @@ registered *before* the ``/{id}`` routes so FastAPI resolves them correctly. from __future__ import annotations -from fastapi import APIRouter, HTTPException, Query, status +from fastapi import APIRouter, Query, status from app.dependencies import ( AuthDep, @@ -33,6 +33,7 @@ from app.dependencies import ( SchedulerDep, SettingsDep, ) +from app.exceptions import BadRequestError, BlocklistSourceNotFoundError from app.models.blocklist import ( BlocklistListResponse, BlocklistSource, @@ -107,7 +108,7 @@ async def create_blocklist( blocklist_ctx.db, payload.name, str(payload.url), enabled=payload.enabled ) except ValueError as exc: - raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc + raise BadRequestError(str(exc)) from exc # --------------------------------------------------------------------------- @@ -271,7 +272,7 @@ async def get_blocklist( """ source = await blocklist_service.get_source(blocklist_ctx.db, source_id) if source is None: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Blocklist source not found.") + raise BlocklistSourceNotFoundError(source_id) return source @@ -307,9 +308,9 @@ async def update_blocklist( enabled=payload.enabled, ) except ValueError as exc: - raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc + raise BadRequestError(str(exc)) from exc if updated is None: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Blocklist source not found.") + raise BlocklistSourceNotFoundError(source_id) return updated @@ -335,7 +336,7 @@ async def delete_blocklist( """ deleted = await blocklist_service.delete_source(blocklist_ctx.db, source_id) if not deleted: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Blocklist source not found.") + raise BlocklistSourceNotFoundError(source_id) @router.get( @@ -366,12 +367,9 @@ async def preview_blocklist( """ source = await blocklist_service.get_source(blocklist_ctx.db, source_id) if source is None: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="Blocklist source not found.") + raise BlocklistSourceNotFoundError(source_id) try: return await blocklist_service.preview_source(source.url, http_session) except ValueError as exc: - raise HTTPException( - status_code=status.HTTP_502_BAD_GATEWAY, - detail=f"Could not fetch blocklist: {exc}", - ) from exc + raise BadRequestError(f"Could not fetch blocklist: {exc}") from exc diff --git a/backend/app/routers/config_misc.py b/backend/app/routers/config_misc.py index 0e1827a..cee10e6 100644 --- a/backend/app/routers/config_misc.py +++ b/backend/app/routers/config_misc.py @@ -4,7 +4,7 @@ import shlex from typing import Annotated import structlog -from fastapi import APIRouter, HTTPException, Query, Request, status +from fastapi import APIRouter, Query, Request, status from app.dependencies import ( AuthDep, @@ -12,6 +12,7 @@ from app.dependencies import ( Fail2BanStartCommandDep, SettingsServiceContextDep, ) +from app.exceptions import OperationError from app.models.config import ( Fail2BanLogResponse, GlobalConfigResponse, @@ -158,15 +159,12 @@ async def restart_fail2ban( ) if not restarted: - raise HTTPException( - status_code=status.HTTP_503_SERVICE_UNAVAILABLE, - detail=( - "fail2ban was stopped but did not come back " - "online within 10 seconds. " - "Check the fail2ban log for initialisation errors. " - "Use POST /api/config/jails/{name}/rollback if a " - "specific jail is suspect." - ), + raise OperationError( + "fail2ban was stopped but did not come back " + "online within 10 seconds. " + "Check the fail2ban log for initialisation errors. " + "Use POST /api/config/jails/{name}/rollback if a " + "specific jail is suspect." ) log.info("fail2ban_restarted") diff --git a/backend/app/routers/health.py b/backend/app/routers/health.py index 0c1dd9b..77ea839 100644 --- a/backend/app/routers/health.py +++ b/backend/app/routers/health.py @@ -6,7 +6,7 @@ state so monitoring tools and Docker health checks can observe daemon status without probing the socket directly. """ -from fastapi import APIRouter +from fastapi import APIRouter, status from fastapi.responses import JSONResponse from app.dependencies import ServerStatusDep diff --git a/backend/app/routers/history.py b/backend/app/routers/history.py index f930578..3b75484 100644 --- a/backend/app/routers/history.py +++ b/backend/app/routers/history.py @@ -17,7 +17,7 @@ from __future__ import annotations from typing import Literal -from fastapi import APIRouter, HTTPException, Query, Request +from fastapi import APIRouter, Query, Request from app.dependencies import ( AuthDep, @@ -26,6 +26,7 @@ from app.dependencies import ( HistoryServiceContextDep, HttpSessionDep, ) +from app.exceptions import HistoryNotFoundError from app.models.ban import BanOrigin, TimeRange from app.models.history import HistoryListResponse, IpDetailResponse from app.services import history_service @@ -188,6 +189,6 @@ async def get_ip_history( ) if detail is None: - raise HTTPException(status_code=404, detail=f"No history found for IP {ip!r}.") + raise HistoryNotFoundError(ip) return detail diff --git a/backend/app/routers/jail_config.py b/backend/app/routers/jail_config.py index 95ff674..68aaac7 100644 --- a/backend/app/routers/jail_config.py +++ b/backend/app/routers/jail_config.py @@ -3,7 +3,7 @@ from __future__ import annotations import shlex from typing import Annotated -from fastapi import APIRouter, HTTPException, Path, Query, Request, status +from fastapi import APIRouter, Path, Query, Request, status from app.dependencies import ( AppDep, @@ -14,6 +14,7 @@ from app.dependencies import ( HealthProbeDep, PendingRecoveryDep, ) +from app.exceptions import BadRequestError from app.models.config import ( ActivateJailRequest, AddLogPathRequest, @@ -258,10 +259,7 @@ async def delete_log_path( try: validate_log_path(log_path) except ValueError as e: - raise HTTPException( - status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, - detail=str(e), - ) from e + raise BadRequestError(str(e)) from e await config_service.delete_log_path(socket_path, name, log_path) diff --git a/backend/app/routers/jails.py b/backend/app/routers/jails.py index 0e6f008..40776bd 100644 --- a/backend/app/routers/jails.py +++ b/backend/app/routers/jails.py @@ -22,7 +22,7 @@ from __future__ import annotations import asyncio from typing import Annotated -from fastapi import APIRouter, Body, HTTPException, Path, status +from fastapi import APIRouter, Body, Path, status from app.dependencies import ( AuthDep, @@ -32,6 +32,7 @@ from app.dependencies import ( HttpSessionDep, JailServiceStateDep, ) +from app.exceptions import BadRequestError from app.models.ban import JailBannedIpsResponse from app.models.jail import ( IgnoreIpRequest, @@ -469,15 +470,9 @@ async def get_jail_banned_ips( HTTPException: 502 when fail2ban is unreachable. """ if page < 1: - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="page must be >= 1.", - ) + raise BadRequestError("page must be >= 1.") if not (1 <= page_size <= 100): - raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail="page_size must be between 1 and 100.", - ) + raise BadRequestError("page_size must be between 1 and 100.") return await jail_service.get_jail_banned_ips( socket_path=socket_path, diff --git a/backend/app/routers/setup.py b/backend/app/routers/setup.py index 96ff0f4..6d37c24 100644 --- a/backend/app/routers/setup.py +++ b/backend/app/routers/setup.py @@ -8,9 +8,10 @@ return ``409 Conflict``. from __future__ import annotations import structlog -from fastapi import APIRouter, HTTPException, status +from fastapi import APIRouter, status from app.dependencies import AppDep, SettingsDep, SettingsServiceContextDep +from app.exceptions import SetupAlreadyCompleteError from app.models.setup import SetupRequest, SetupResponse, SetupStatusResponse, SetupTimezoneResponse from app.services import setup_service from app.utils.runtime_state import update_app_settings @@ -59,13 +60,10 @@ async def post_setup( :class:`~app.models.setup.SetupResponse` on success. Raises: - HTTPException: 409 if setup has already been completed. + SetupAlreadyCompleteError: if setup has already been completed. """ if is_setup_complete_cached(app) or await setup_service.is_setup_complete(settings_ctx.db): - raise HTTPException( - status_code=status.HTTP_409_CONFLICT, - detail="Setup has already been completed.", - ) + raise SetupAlreadyCompleteError() await setup_service.run_setup( settings_ctx.db, diff --git a/frontend/src/api/client.ts b/frontend/src/api/client.ts index 96465dc..23a01ac 100644 --- a/frontend/src/api/client.ts +++ b/frontend/src/api/client.ts @@ -10,6 +10,7 @@ * to guarantee type safety at the API boundary. */ +import { ErrorResponse } from "../types/response"; import { ENDPOINTS } from "./endpoints"; /** Base URL for all API calls. Falls back to `/api` in production. */ @@ -27,15 +28,20 @@ export class ApiError extends Error { /** Raw response body text as returned by the server. */ public readonly body: string; + /** Parsed error response (if response was a valid ErrorResponse), undefined otherwise. */ + public readonly errorResponse: ErrorResponse | undefined; + /** * @param status - The HTTP status code. * @param body - The raw response body text. + * @param errorResponse - Parsed ErrorResponse if available. */ - constructor(status: number, body: string) { - super(`API error ${String(status)}: ${body}`); + constructor(status: number, body: string, errorResponse?: ErrorResponse) { + super(`API error ${String(status)}: ${errorResponse?.detail || body}`); this.name = "ApiError"; this.status = status; this.body = body; + this.errorResponse = errorResponse; } } @@ -79,7 +85,7 @@ export function setUnauthorizedHandler(handler: (() => void) | null): void { * @param url - Fully-qualified URL. * @param options - Standard `RequestInit` options. * @returns Parsed JSON response cast to `T`. - * @throws {FetchError} When the request fails or server returns non-2xx status. + * @throws {ApiError} When the request fails or server returns non-2xx status. */ async function request(url: string, options: RequestInit = {}): Promise { try { @@ -100,7 +106,18 @@ async function request(url: string, options: RequestInit = {}): Promise { unauthorizedHandler?.(); } - throw new ApiError(response.status, body); + // Try to parse as ErrorResponse + let errorResponse: ErrorResponse | undefined; + try { + const parsed = JSON.parse(body); + if (parsed && typeof parsed === "object" && "code" in parsed && "detail" in parsed) { + errorResponse = parsed as ErrorResponse; + } + } catch { + // If parsing fails, errorResponse remains undefined + } + + throw new ApiError(response.status, body, errorResponse); } // 204 No Content — return undefined cast to T. diff --git a/frontend/src/types/api.ts b/frontend/src/types/api.ts index dacac6b..f6588c8 100644 --- a/frontend/src/types/api.ts +++ b/frontend/src/types/api.ts @@ -19,6 +19,12 @@ export interface ApiErrorPayload { body: string; /** User-friendly error message derived from status and body. */ message: string; + /** Machine-readable error code for client-side branching (e.g., "jail_not_found"). */ + code?: string; + /** Human-readable error description from the server. */ + detail?: string; + /** Optional structured context for the error (e.g., field names, constraint violations). */ + metadata?: Record; } /** diff --git a/frontend/src/types/response.ts b/frontend/src/types/response.ts index 1f189e3..7afa3e0 100644 --- a/frontend/src/types/response.ts +++ b/frontend/src/types/response.ts @@ -25,3 +25,12 @@ export interface CommandResponse { /** Whether the command succeeded. */ success: boolean; } + +export interface ErrorResponse { + /** Machine-readable error code for client-side branching (e.g., "jail_not_found", "rate_limit_exceeded"). */ + code: string; + /** Human-readable error description for display to users. */ + detail: string; + /** Optional structured context for the error (field names, constraint violations, etc.). */ + metadata: Record; +}