docs(app_controller+guidelines): add Exception Handling section + audit script cross-reference

This commit is contained in:
ed
2026-06-16 09:07:24 -04:00
parent b447f66818
commit 4209523228
2 changed files with 94 additions and 0 deletions
+10
View File
@@ -71,6 +71,16 @@ tracks will apply it to the remaining `src/` files
see `conductor/tracks/data_oriented_error_handling_20260606/spec.md` §12.2
for the prioritized list).
**Audit:** the convention is enforced via
[`scripts/audit_exception_handling.py`](../../scripts/audit_exception_handling.py)
(static analyzer; file-presence = enabled per
[`feature_flags.md`](code_styleguides/feature_flags.md)). Run
`uv run python scripts/audit_exception_handling.py` for a human-readable
report or `--json` for machine-readable output. The audit classifies each
`try/except/finally/raise` site against 10 categories (5 compliant + 3
violation + 1 suspicious + 1 unclear); see the styleguide's "Audit Script"
section for the full taxonomy.
### `Optional[T]` ban (return types only)
In the 3 refactored files (`src/mcp_client.py`, `src/ai_client.py`,
+84
View File
@@ -391,6 +391,90 @@ def test_apply_persona(live_gui):
---
## Exception Handling in `app_controller.py`
Per the data-oriented error handling convention
(`conductor/code_styleguides/error_handling.md`), `app_controller.py` is a
**migration-target file**: it has not been fully refactored to the
`Result[T]` pattern. The convention is applied to 3 of 65 source files
(`src/mcp_client.py`, `src/ai_client.py`, `src/rag_engine.py`); the rest of
`src/` is in the migration-target state.
The exception-handling audit (`scripts/audit_exception_handling.py`)
classifies 56 exception-handling sites in this file (per the 2026-06-16
audit run):
| Category | Count | Convention status |
|---|---|---|
| `BOUNDARY_FASTAPI` | 13 | Compliant |
| `INTERNAL_BROAD_CATCH` | 28 | **Violation** (migration target) |
| `INTERNAL_SILENT_SWALLOW` | 6 | **Violation** (migration target) |
| `INTERNAL_RETHROW` | 3 | Suspicious |
| `UNCLEAR` | 2 | Manual review needed |
| `INTERNAL_OPTIONAL_RETURN` | 1 | **Violation** (migration target) |
### The 13 FastAPI boundary sites (compliant)
`app_controller.py` is the FastAPI handler layer (`--enable-test-hooks`
mode). The 11 `HTTPException` raises in `_api_*` handlers (lines 96, 99,
213, 215, 312, 320, 341, 369, 380, 402) and the 2 `except Exception +
raise HTTPException` sites (lines 309, 401) are the **framework boundary**
`HTTPException` is the FastAPI-idiomatic way to signal HTTP errors;
FastAPI converts it to a JSON response at the framework level. These
sites are compliant per the convention's "Framework boundaries (FastAPI)"
section.
Example (line 96):
```python
if not target_key:
raise HTTPException(status_code=403, detail="API Key not configured on server")
```
Example (line 309):
```python
try:
result = ai_client.send_result(...)
return result.data
except Exception as e:
raise HTTPException(status_code=500, detail=f"AI call failed: {e}")
```
### The 40 migration-target sites (violations)
The remaining ~40 sites (mostly `except Exception + log/print`, `except
Exception + return None`) are **migration-target** — they would benefit
from a future track that migrates the controller to the convention.
**Recommended future track:** `app_controller_result_migration_20260616`
(not in scope of the audit track; the user decides). The work would be:
1. Convert `Optional[T]` return types to `Result[T]`.
2. Convert `except Exception + log/print` to `except Exception +
return Result(data=NIL_T, errors=[ErrorInfo(...)])`.
3. Convert `except Exception + return None` to `except Exception +
return Result(data=NIL_T, errors=[ErrorInfo(...)])`.
4. Keep the 13 FastAPI boundary sites as-is (they're the framework contract).
5. Add tests that verify the success and failure paths of each migrated
function return the right `Result`.
This is a 2-3 day Tier 2 effort. The audit's per-site hints (in the JSON
output via `--json`) tell the implementer what the fix should look like
for each site.
### Quick check
To see the current state of the file's exception handling:
```bash
uv run python scripts/audit_exception_handling.py --top 1 --verbose | head -50
```
To see only the violations (not the compliant FastAPI boundary sites):
```bash
uv run python scripts/audit_exception_handling.py --json | \
python -c "import json,sys; r=json.load(sys.stdin); print([f for f in r['files'][0]['findings'] if f['category'] in ('INTERNAL_BROAD_CATCH', 'INTERNAL_SILENT_SWALLOW', 'INTERNAL_OPTIONAL_RETURN')])"
```
---
## See Also
- **[guide_architecture.md](guide_architecture.md)** — Threading and event flow