Fix exception handler overlap issue - add DomainError catch-all handler
**Problem:** Broad exception handlers created fragility where adding a new DomainError subclass without explicit registration would silently fall through to the generic exception handler, losing the specific error_code and metadata. **Solution:** 1. Import DomainError in main.py for explicit handler registration 2. Fix type hints in exception handlers from 'Exception' to specific types - NotFoundError handler now typed as 'NotFoundError' - BadRequestError handler now typed as 'BadRequestError' - ConflictError handler now typed as 'ConflictError' - DomainError handler now typed as 'DomainError' - ServiceUnavailableError handler now typed as 'ServiceUnavailableError' 3. Add DomainError as an explicit catch-all handler in the registration chain - Positioned after specific handlers, before HTTPException - Any unregistered DomainError subclass now gets correct error_code + metadata 4. Document the exception handler hierarchy with detailed comments 5. Update Backend-Development.md with handler hierarchy documentation 6. Update Architekture.md section 2.2 with exception handler details 7. Fix test expectations in test_main.py to verify ErrorResponse format **Impact:** Any new DomainError subclass now automatically gets correct HTTP 500 status, error_code, and metadata - even if developer forgets explicit handler. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
@@ -1,62 +1,3 @@
|
||||
## [Backend] Pydantic validators execute at import time
|
||||
|
||||
**Status:** ✅ **COMPLETE** — Codebase is fully compliant
|
||||
|
||||
**Verification:**
|
||||
- ✓ Audited all 14 model files in `backend/app/models/`
|
||||
- ✓ No model files import from `app.config`, `app.services`, `app.utils`, or `app.routers`
|
||||
- ✓ No validators, field defaults, or computed fields call runtime-dependent functions
|
||||
- ✓ `BanGuiBaseModel` is pure with no side-effects
|
||||
- ✓ Path validation correctly placed in `app.utils.path_utils` and called from routers/services only
|
||||
|
||||
**Current Architecture (Correct):**
|
||||
|
||||
The codebase follows the proper pattern:
|
||||
1. **Models** (`app/models/`) contain only pure data classes and stateless validators (if any)
|
||||
2. **Validation requiring app state** (settings, file I/O, database) happens in **routers or services** at request time
|
||||
3. **Path validation helper** (`app.utils.path_utils.validate_log_path()`) is called from routers/services, never imported in models
|
||||
|
||||
Example (correct pattern in place):
|
||||
```python
|
||||
# ✅ Correct: Validation in router with access to settings
|
||||
from app.utils.path_utils import validate_log_path
|
||||
|
||||
@router.post("/jails/{name}/logpath")
|
||||
async def add_log_path(name: str, body: AddLogPathRequest) -> None:
|
||||
validate_log_path(body.log_path) # Called at request time
|
||||
await config_service.add_log_path(name, body)
|
||||
```
|
||||
|
||||
**Documentation Updates (Completed):**
|
||||
- ✓ `Docs/Architekture.md` § 2.1: Updated Models section with explicit constraints on I/O and side effects
|
||||
- ✓ `Docs/Backend-Development.md`: Enhanced validator documentation with import-time execution explanation
|
||||
|
||||
**Important Note on Task Description:**
|
||||
|
||||
The original task suggested using `@model_validator` with imports inside the method. This approach is **incorrect** and would not solve the problem:
|
||||
```python
|
||||
# ❌ WRONG — Still executes at import time:
|
||||
@model_validator(mode="after")
|
||||
def validate_range(self):
|
||||
from app.config import get_settings # ← Decorator still runs at class definition time
|
||||
...
|
||||
```
|
||||
|
||||
The **correct solution** (already in place) is to perform validation in the **router or service layer** where settings and services are available at request/execution time.
|
||||
|
||||
**Regression Prevention:**
|
||||
|
||||
To prevent future violations, run this check before committing:
|
||||
```bash
|
||||
# Verify no app-layer imports in model files
|
||||
find backend/app/models -name "*.py" -exec grep -l "from app\.\(config\|services\|utils\|routers\)" {} \;
|
||||
# Should return: 0 files (empty)
|
||||
```
|
||||
|
||||
Consider adding this as a pre-commit hook or CI check for long-term prevention.
|
||||
|
||||
---
|
||||
|
||||
## [Backend] Exception handler overlap — broad handlers catching everything
|
||||
|
||||
**Where found**
|
||||
|
||||
Reference in New Issue
Block a user