lukas.pupkalipinski/BanGUI
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Implement centralized exception handling and validation

Browse Source 
- Add custom exception classes for structured error handling
- Implement global exception handlers in FastAPI application
- Add comprehensive request/response validation
- Create exception contract tests for validation
- Update backend development documentation

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Lukas
2026-04-27 18:52:12 +02:00
parent 2e221f6852
commit afc1e44e99
5 changed files with 500 additions and 98 deletions
Download Patch File Download Diff File Expand all files Collapse all files

128
backend/app/exceptions.py
View File

@@ -1,9 +1,81 @@
"""Shared domain exception classes used across routers and services."""
"""Shared domain exception classes used across routers and services.
Exception Taxonomy
==================
All domain exceptions inherit from one of these base categories:
- **NotFoundError** (404): Domain entity not found
- **BadRequestError** (400): Invalid input, validation failure, invalid identifiers
- **ConflictError** (409): State conflict, resource already exists, invalid state transition
- **OperationError** (500): Operation failure, write errors
- **ServiceUnavailableError** (503): Infrastructure/external service issues
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.
Example:
def get_jail(name: str) -> Jail:
# Raises JailNotFoundError (subclass of NotFoundError)
...
@app.exception_handler(NotFoundError)
async def handle_not_found(request, exc):
return JSONResponse(status_code=404, content={"detail": str(exc)})
See Backend-Development.md for the complete exception contract.
"""
from __future__ import annotations
# ---------------------------------------------------------------------------
# Exception Base Classes (Categories)
# ---------------------------------------------------------------------------
class JailNotFoundError(Exception):
class DomainError(Exception):
"""Base class for all domain exceptions."""
pass
class NotFoundError(DomainError):
"""Raised when a requested domain entity is not found. HTTP 404."""
pass
class BadRequestError(DomainError):
"""Raised for invalid input, validation failures, or invalid identifiers. HTTP 400."""
pass
class ConflictError(DomainError):
"""Raised for state conflicts or resource constraints. HTTP 409."""
pass
class OperationError(DomainError):
"""Raised when a domain operation fails (write, update, etc.). HTTP 500."""
pass
class ServiceUnavailableError(DomainError):
"""Raised for infrastructure or external service issues. HTTP 503."""
pass
# ---------------------------------------------------------------------------
# Jail-Specific Exceptions
# ---------------------------------------------------------------------------
class JailNotFoundError(NotFoundError):
"""Raised when a requested jail name does not exist."""
def __init__(self, name: str) -> None:
@@ -11,23 +83,23 @@ class JailNotFoundError(Exception):
super().__init__(f"Jail not found: {name!r}")
class JailOperationError(Exception):
"""Raised when a fail2ban jail operation fails."""
class JailOperationError(ConflictError):
"""Raised when a jail state operation fails (e.g. start/stop already in progress)."""
class ConfigValidationError(Exception):
class ConfigValidationError(BadRequestError):
"""Raised when config values fail validation before applying."""
class ConfigOperationError(Exception):
class ConfigOperationError(BadRequestError):
"""Raised when a config payload update or command fails."""
class ConfigDirError(Exception):
class ConfigDirError(ServiceUnavailableError):
"""Raised when the fail2ban config directory is missing or inaccessible."""
class ConfigFileNotFoundError(Exception):
class ConfigFileNotFoundError(NotFoundError):
"""Raised when a requested config file does not exist."""
def __init__(self, filename: str) -> None:
@@ -40,7 +112,7 @@ class ConfigFileNotFoundError(Exception):
super().__init__(f"Config file not found: {filename!r}")
class ConfigFileExistsError(Exception):
class ConfigFileExistsError(ConflictError):
"""Raised when trying to create a file that already exists."""
def __init__(self, filename: str) -> None:
@@ -53,19 +125,19 @@ class ConfigFileExistsError(Exception):
super().__init__(f"Config file already exists: {filename!r}")
class ConfigFileWriteError(Exception):
class ConfigFileWriteError(OperationError):
"""Raised when a file cannot be written (permissions, disk full, etc.)."""
class ConfigFileNameError(Exception):
class ConfigFileNameError(BadRequestError):
"""Raised when a supplied filename is invalid or unsafe."""
class ServerOperationError(Exception):
class ServerOperationError(BadRequestError):
"""Raised when a server control command (e.g. refresh) fails."""
class Fail2BanConnectionError(Exception):
class Fail2BanConnectionError(ServiceUnavailableError):
"""Raised when the fail2ban socket is unreachable or returns an error."""
def __init__(self, message: str, socket_path: str) -> None:
@@ -79,11 +151,11 @@ class Fail2BanConnectionError(Exception):
super().__init__(f"{message} (socket: {socket_path})")
class Fail2BanProtocolError(Exception):
class Fail2BanProtocolError(ServiceUnavailableError):
"""Raised when the response from fail2ban cannot be parsed."""
class FilterInvalidRegexError(Exception):
class FilterInvalidRegexError(BadRequestError):
"""Raised when a regex pattern fails to compile."""
def __init__(self, pattern: str, error: str) -> None:
@@ -93,7 +165,7 @@ class FilterInvalidRegexError(Exception):
super().__init__(f"Invalid regex {pattern!r}: {error}")
class JailNotFoundInConfigError(Exception):
class JailNotFoundInConfigError(NotFoundError):
"""Raised when the requested jail name is not defined in any config file."""
def __init__(self, name: str) -> None:
@@ -101,7 +173,7 @@ class JailNotFoundInConfigError(Exception):
super().__init__(f"Jail not found in config: {name!r}")
class ConfigWriteError(Exception):
class ConfigWriteError(OperationError):
"""Raised when writing a configuration file modification fails."""
def __init__(self, message: str) -> None:
@@ -109,11 +181,11 @@ class ConfigWriteError(Exception):
super().__init__(message)
class JailNameError(Exception):
class JailNameError(BadRequestError):
"""Raised when a jail name contains invalid characters."""
class JailAlreadyActiveError(Exception):
class JailAlreadyActiveError(ConflictError):
"""Raised when trying to activate a jail that is already active."""
def __init__(self, name: str) -> None:
@@ -121,7 +193,7 @@ class JailAlreadyActiveError(Exception):
super().__init__(f"Jail is already active: {name!r}")
class JailAlreadyInactiveError(Exception):
class JailAlreadyInactiveError(ConflictError):
"""Raised when trying to deactivate a jail that is already inactive."""
def __init__(self, name: str) -> None:
@@ -129,7 +201,7 @@ class JailAlreadyInactiveError(Exception):
super().__init__(f"Jail is already inactive: {name!r}")
class FilterNotFoundError(Exception):
class FilterNotFoundError(NotFoundError):
"""Raised when the requested filter name is not found."""
def __init__(self, name: str) -> None:
@@ -137,7 +209,7 @@ class FilterNotFoundError(Exception):
super().__init__(f"Filter not found: {name!r}")
class FilterAlreadyExistsError(Exception):
class FilterAlreadyExistsError(ConflictError):
"""Raised when trying to create a filter whose `.conf` or `.local` already exists."""
def __init__(self, name: str) -> None:
@@ -145,11 +217,11 @@ class FilterAlreadyExistsError(Exception):
super().__init__(f"Filter already exists: {name!r}")
class FilterNameError(Exception):
class FilterNameError(BadRequestError):
"""Raised when a filter name contains invalid characters."""
class FilterReadonlyError(Exception):
class FilterReadonlyError(ConflictError):
"""Raised when trying to delete a shipped `.conf` filter with no `.local` override."""
def __init__(self, name: str) -> None:
@@ -159,7 +231,7 @@ class FilterReadonlyError(Exception):
)
class ActionNotFoundError(Exception):
class ActionNotFoundError(NotFoundError):
"""Raised when the requested action name is not found."""
def __init__(self, name: str) -> None:
@@ -167,7 +239,7 @@ class ActionNotFoundError(Exception):
super().__init__(f"Action not found: {name!r}")
class ActionAlreadyExistsError(Exception):
class ActionAlreadyExistsError(ConflictError):
"""Raised when trying to create an action whose `.conf` or `.local` already exists."""
def __init__(self, name: str) -> None:
@@ -175,11 +247,11 @@ class ActionAlreadyExistsError(Exception):
super().__init__(f"Action already exists: {name!r}")
class ActionNameError(Exception):
class ActionNameError(BadRequestError):
"""Raised when an action name contains invalid characters."""
class ActionReadonlyError(Exception):
class ActionReadonlyError(ConflictError):
"""Raised when trying to delete a shipped `.conf` action with no `.local` override."""
def __init__(self, name: str) -> None:

58
backend/app/main.py
View File

@@ -30,32 +30,13 @@ from starlette.middleware.base import BaseHTTPMiddleware
from app import __version__
from app.config import Settings, get_settings
from app.exceptions import (
ActionAlreadyExistsError,
ActionNameError,
ActionNotFoundError,
ActionReadonlyError,
ConfigDirError,
ConfigFileExistsError,
ConfigFileNameError,
ConfigFileNotFoundError,
ConfigFileWriteError,
ConfigOperationError,
ConfigValidationError,
ConfigWriteError,
BadRequestError,
ConflictError,
Fail2BanConnectionError,
Fail2BanProtocolError,
FilterAlreadyExistsError,
FilterInvalidRegexError,
FilterNameError,
FilterNotFoundError,
FilterReadonlyError,
JailAlreadyActiveError,
JailAlreadyInactiveError,
JailNameError,
JailNotFoundError,
JailNotFoundInConfigError,
JailOperationError,
ServerOperationError,
NotFoundError,
OperationError,
ServiceUnavailableError,
)
from app.middleware.csrf import CsrfMiddleware
from app.routers import (
@@ -528,31 +509,12 @@ 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(JailNotFoundError, _not_found_handler) # type: ignore[arg-type]
app.add_exception_handler(JailNotFoundInConfigError, _not_found_handler) # type: ignore[arg-type]
app.add_exception_handler(FilterNotFoundError, _not_found_handler) # type: ignore[arg-type]
app.add_exception_handler(ActionNotFoundError, _not_found_handler) # type: ignore[arg-type]
app.add_exception_handler(ConfigFileNotFoundError, _not_found_handler) # type: ignore[arg-type]
app.add_exception_handler(ConfigValidationError, _bad_request_handler) # type: ignore[arg-type]
app.add_exception_handler(ConfigFileNameError, _bad_request_handler) # type: ignore[arg-type]
app.add_exception_handler(ConfigOperationError, _bad_request_handler) # type: ignore[arg-type]
app.add_exception_handler(ServerOperationError, _bad_request_handler) # type: ignore[arg-type]
app.add_exception_handler(ActionNameError, _bad_request_handler) # type: ignore[arg-type]
app.add_exception_handler(FilterNameError, _bad_request_handler) # type: ignore[arg-type]
app.add_exception_handler(JailNameError, _bad_request_handler) # type: ignore[arg-type]
app.add_exception_handler(FilterInvalidRegexError, _bad_request_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(ValueError, _value_error_handler) # type: ignore[arg-type]
app.add_exception_handler(JailOperationError, _conflict_handler) # type: ignore[arg-type]
app.add_exception_handler(JailAlreadyActiveError, _conflict_handler) # type: ignore[arg-type]
app.add_exception_handler(JailAlreadyInactiveError, _conflict_handler) # type: ignore[arg-type]
app.add_exception_handler(FilterAlreadyExistsError, _conflict_handler) # type: ignore[arg-type]
app.add_exception_handler(ActionAlreadyExistsError, _conflict_handler) # type: ignore[arg-type]
app.add_exception_handler(FilterReadonlyError, _conflict_handler) # type: ignore[arg-type]
app.add_exception_handler(ActionReadonlyError, _conflict_handler) # type: ignore[arg-type]
app.add_exception_handler(ConfigFileExistsError, _conflict_handler) # type: ignore[arg-type]
app.add_exception_handler(ConfigWriteError, _domain_error_handler) # type: ignore[arg-type]
app.add_exception_handler(ConfigDirError, _service_unavailable_handler) # type: ignore[arg-type]
app.add_exception_handler(ConfigFileWriteError, _bad_request_handler) # type: ignore[arg-type]
app.add_exception_handler(Exception, _unhandled_exception_handler)
# --- Routers ---

308
backend/tests/test_exception_contract.py Normal file
View File

@@ -0,0 +1,308 @@
"""Test exception contract: categories map to HTTP status codes.
This test suite verifies that domain exceptions inherit from the correct
base categories and that main.py registers handlers that map those categories
to the expected HTTP status codes.
The exception taxonomy ensures:
1. Routers don't need to know about individual exception types.
2. Exception mapping is consistent and centralized in main.py.
3. Existing clients see predictable HTTP status codes.
"""
from __future__ import annotations
import pytest
from httpx import ASGITransport, AsyncClient
from app.config import Settings
from app.exceptions import (
ActionAlreadyExistsError,
ActionNameError,
ActionNotFoundError,
ActionReadonlyError,
BadRequestError,
ConfigDirError,
ConfigFileExistsError,
ConfigFileNameError,
ConfigFileNotFoundError,
ConfigFileWriteError,
ConfigOperationError,
ConfigValidationError,
ConfigWriteError,
ConflictError,
Fail2BanConnectionError,
Fail2BanProtocolError,
FilterAlreadyExistsError,
FilterInvalidRegexError,
FilterNameError,
FilterNotFoundError,
FilterReadonlyError,
JailAlreadyActiveError,
JailAlreadyInactiveError,
JailNameError,
JailNotFoundError,
JailNotFoundInConfigError,
JailOperationError,
NotFoundError,
OperationError,
ServerOperationError,
ServiceUnavailableError,
)
from app.main import create_app
# ---------------------------------------------------------------------------
# Exception Taxonomy Tests
# ---------------------------------------------------------------------------
class TestExceptionTaxonomy:
"""Verify all domain exceptions inherit from the correct category."""
def test_jail_not_found_error_inherits_from_not_found_error(self) -> None:
exc = JailNotFoundError("test")
assert isinstance(exc, NotFoundError)
def test_jail_not_found_in_config_error_inherits_from_not_found_error(self) -> None:
exc = JailNotFoundInConfigError("test")
assert isinstance(exc, NotFoundError)
def test_filter_not_found_error_inherits_from_not_found_error(self) -> None:
exc = FilterNotFoundError("test")
assert isinstance(exc, NotFoundError)
def test_action_not_found_error_inherits_from_not_found_error(self) -> None:
exc = ActionNotFoundError("test")
assert isinstance(exc, NotFoundError)
def test_config_file_not_found_error_inherits_from_not_found_error(self) -> None:
exc = ConfigFileNotFoundError("test.conf")
assert isinstance(exc, NotFoundError)
def test_config_validation_error_inherits_from_bad_request_error(self) -> None:
exc = ConfigValidationError()
assert isinstance(exc, BadRequestError)
def test_config_operation_error_inherits_from_bad_request_error(self) -> None:
exc = ConfigOperationError()
assert isinstance(exc, BadRequestError)
def test_config_file_name_error_inherits_from_bad_request_error(self) -> None:
exc = ConfigFileNameError()
assert isinstance(exc, BadRequestError)
def test_server_operation_error_inherits_from_bad_request_error(self) -> None:
exc = ServerOperationError()
assert isinstance(exc, BadRequestError)
def test_jail_name_error_inherits_from_bad_request_error(self) -> None:
exc = JailNameError()
assert isinstance(exc, BadRequestError)
def test_filter_name_error_inherits_from_bad_request_error(self) -> None:
exc = FilterNameError()
assert isinstance(exc, BadRequestError)
def test_action_name_error_inherits_from_bad_request_error(self) -> None:
exc = ActionNameError()
assert isinstance(exc, BadRequestError)
def test_filter_invalid_regex_error_inherits_from_bad_request_error(self) -> None:
exc = FilterInvalidRegexError("[invalid", "error")
assert isinstance(exc, BadRequestError)
def test_jail_operation_error_inherits_from_conflict_error(self) -> None:
exc = JailOperationError()
assert isinstance(exc, ConflictError)
def test_jail_already_active_error_inherits_from_conflict_error(self) -> None:
exc = JailAlreadyActiveError("test")
assert isinstance(exc, ConflictError)
def test_jail_already_inactive_error_inherits_from_conflict_error(self) -> None:
exc = JailAlreadyInactiveError("test")
assert isinstance(exc, ConflictError)
def test_filter_already_exists_error_inherits_from_conflict_error(self) -> None:
exc = FilterAlreadyExistsError("test")
assert isinstance(exc, ConflictError)
def test_filter_readonly_error_inherits_from_conflict_error(self) -> None:
exc = FilterReadonlyError("test")
assert isinstance(exc, ConflictError)
def test_action_already_exists_error_inherits_from_conflict_error(self) -> None:
exc = ActionAlreadyExistsError("test")
assert isinstance(exc, ConflictError)
def test_action_readonly_error_inherits_from_conflict_error(self) -> None:
exc = ActionReadonlyError("test")
assert isinstance(exc, ConflictError)
def test_config_file_exists_error_inherits_from_conflict_error(self) -> None:
exc = ConfigFileExistsError("test.conf")
assert isinstance(exc, ConflictError)
def test_config_write_error_inherits_from_operation_error(self) -> None:
exc = ConfigWriteError("test")
assert isinstance(exc, OperationError)
def test_config_file_write_error_inherits_from_operation_error(self) -> None:
exc = ConfigFileWriteError()
assert isinstance(exc, OperationError)
def test_fail2ban_connection_error_inherits_from_service_unavailable_error(
self,
) -> None:
exc = Fail2BanConnectionError("test", "/tmp/socket")
assert isinstance(exc, ServiceUnavailableError)
def test_fail2ban_protocol_error_inherits_from_service_unavailable_error(
self,
) -> None:
exc = Fail2BanProtocolError()
assert isinstance(exc, ServiceUnavailableError)
def test_config_dir_error_inherits_from_service_unavailable_error(self) -> None:
exc = ConfigDirError()
assert isinstance(exc, ServiceUnavailableError)
# ---------------------------------------------------------------------------
# HTTP Status Code Mapping Tests
# ---------------------------------------------------------------------------
class TestExceptionHandlerMapping:
"""Verify exception handlers map to the correct HTTP status codes."""
@pytest.mark.asyncio
async def test_not_found_error_returns_404(self, test_settings: Settings) -> None:
"""NotFoundError handlers return HTTP 404."""
app = create_app(test_settings)
transport = ASGITransport(app=app)
async with AsyncClient(transport=transport, base_url="http://test") as client:
@app.get("/test-404")
async def test_404() -> None:
raise JailNotFoundError("test_jail")
response = await client.get("/test-404")
assert response.status_code == 404
data = response.json()
assert "detail" in data
assert "Jail not found" in data["detail"]
@pytest.mark.asyncio
async def test_bad_request_error_returns_400(self, test_settings: Settings) -> None:
"""BadRequestError handlers return HTTP 400."""
app = create_app(test_settings)
transport = ASGITransport(app=app)
async with AsyncClient(transport=transport, base_url="http://test") as client:
@app.get("/test-400")
async def test_400() -> None:
raise ConfigValidationError()
response = await client.get("/test-400")
assert response.status_code == 400
@pytest.mark.asyncio
async def test_conflict_error_returns_409(self, test_settings: Settings) -> None:
"""ConflictError handlers return HTTP 409."""
app = create_app(test_settings)
transport = ASGITransport(app=app)
async with AsyncClient(transport=transport, base_url="http://test") as client:
@app.get("/test-409")
async def test_409() -> None:
raise JailAlreadyActiveError("test_jail")
response = await client.get("/test-409")
assert response.status_code == 409
data = response.json()
assert "detail" in data
assert "already active" in data["detail"]
@pytest.mark.asyncio
async def test_operation_error_returns_500(self, test_settings: Settings) -> None:
"""OperationError handlers return HTTP 500."""
app = create_app(test_settings)
transport = ASGITransport(app=app)
async with AsyncClient(transport=transport, base_url="http://test") as client:
@app.get("/test-500")
async def test_500() -> None:
raise ConfigWriteError("test error")
response = await client.get("/test-500")
assert response.status_code == 500
data = response.json()
assert "detail" in data
@pytest.mark.asyncio
async def test_service_unavailable_error_returns_503(self, test_settings: Settings) -> None:
"""ServiceUnavailableError handlers return HTTP 503."""
app = create_app(test_settings)
transport = ASGITransport(app=app)
async with AsyncClient(transport=transport, base_url="http://test") as client:
@app.get("/test-503")
async def test_503() -> None:
raise ConfigDirError()
response = await client.get("/test-503")
assert response.status_code == 503
data = response.json()
assert "detail" in data
@pytest.mark.asyncio
async def test_fail2ban_connection_error_returns_502(self, test_settings: Settings) -> None:
"""Fail2BanConnectionError is specially mapped to HTTP 502 (not 503)."""
app = create_app(test_settings)
transport = ASGITransport(app=app)
async with AsyncClient(transport=transport, base_url="http://test") as client:
@app.get("/test-502")
async def test_502() -> None:
raise Fail2BanConnectionError("test", "/tmp/socket")
response = await client.get("/test-502")
assert response.status_code == 502
data = response.json()
assert "detail" in data
# ---------------------------------------------------------------------------
# Exception Detail Preservation Tests
# ---------------------------------------------------------------------------
class TestExceptionDetails:
"""Verify exception details are preserved in responses."""
def test_jail_not_found_error_preserves_name(self) -> None:
exc = JailNotFoundError("my_jail")
assert exc.name == "my_jail"
assert "my_jail" in str(exc)
def test_jail_already_active_error_preserves_name(self) -> None:
exc = JailAlreadyActiveError("my_jail")
assert exc.name == "my_jail"
assert "my_jail" in str(exc)
def test_config_file_not_found_error_preserves_filename(self) -> None:
exc = ConfigFileNotFoundError("test.conf")
assert exc.filename == "test.conf"
assert "test.conf" in str(exc)
def test_config_write_error_preserves_message(self) -> None:
exc = ConfigWriteError("Permission denied")
assert exc.message == "Permission denied"
assert "Permission denied" in str(exc)
def test_fail2ban_connection_error_preserves_socket_path(self) -> None:
exc = Fail2BanConnectionError(
"Connection refused", "/var/run/fail2ban/fail2ban.sock"
)
assert exc.socket_path == "/var/run/fail2ban/fail2ban.sock"
def test_filter_invalid_regex_error_preserves_pattern_and_error(self) -> None:
exc = FilterInvalidRegexError("[invalid", "unterminated character set")
assert exc.pattern == "[invalid"
assert exc.error == "unterminated character set"
assert "[invalid" in str(exc)