diff --git a/Makefile b/Makefile index 02271608..2f84cb35 100644 --- a/Makefile +++ b/Makefile @@ -430,7 +430,7 @@ prune: # Clean Cache Files .PHONY: clean clean: - @find . | grep -E "(__pycache__|\.pyc|\.pyo|\.pytest_cache|\.ruff_cache|\.mypy_cache|./data/)" | xargs rm -rf + @find . | grep -E "(__pycache__|\.pyc|\.pyo|\.pytest_cache|\.ruff_cache|\.mypy_cache|data/|site/)" | xargs rm -rf @cd panel && rm -rf node_modules/ && rm -rf .next/ && rm -rf logs/ && rm -rf coverage/ @cd .. diff --git a/roboco/api/middleware.py b/roboco/api/middleware.py index fbbcc499..a97cfc58 100644 --- a/roboco/api/middleware.py +++ b/roboco/api/middleware.py @@ -12,6 +12,7 @@ from typing import Any, cast import structlog from fastapi import FastAPI, HTTPException, Request, Response from fastapi import status as http_status +from fastapi.encoders import jsonable_encoder from fastapi.exceptions import RequestValidationError from fastapi.responses import JSONResponse from starlette.middleware.base import BaseHTTPMiddleware @@ -431,7 +432,15 @@ async def request_validation_handler(request: Request, exc: Exception) -> JSONRe body=body_for_log, errors=errors, ) - content: dict[str, Any] = {"detail": errors, "body": body} + # jsonable_encoder is mandatory: Pydantic v2 stashes the raw exception + # object under error['ctx']['error'] for any validator that raises + # ValueError (e.g. blocker_type), and a raw Exception is not JSON + # serializable — without this json.dumps crashes the 422 render into a + # 500. FastAPI's own default validation handler encodes for the same reason. + content: dict[str, Any] = { + "detail": jsonable_encoder(errors), + "body": jsonable_encoder(body), + } remediate = _uuid_field_remediation(errors) if remediate is not None: content["remediate"] = remediate diff --git a/tests/unit/api/test_middleware.py b/tests/unit/api/test_middleware.py index a516d638..baab77b6 100644 --- a/tests/unit/api/test_middleware.py +++ b/tests/unit/api/test_middleware.py @@ -12,7 +12,7 @@ from uuid import UUID # noqa: TC003 from fastapi import FastAPI, HTTPException from fastapi.testclient import TestClient -from pydantic import BaseModel +from pydantic import BaseModel, field_validator from roboco.api.middleware import ( _uuid_field_remediation, get_status_code, @@ -278,6 +278,43 @@ def test_request_validation_handler_returns_422_with_details() -> None: assert "body" in body +class _EnumFieldBody(BaseModel): + """Mirrors IAmBlockedRequest: a field_validator that raises ValueError. + + Pydantic v2 stashes the raw ValueError object in error['ctx']['error'], + which is not JSON-serializable — the handler must encode it (jsonable_encoder) + or json.dumps crashes the 422 render into a 500.""" + + kind: str + + @field_validator("kind") + @classmethod + def _one_of(cls, v: str) -> str: + if v not in {"a", "b"}: + raise ValueError(f"kind must be one of: a | b. Got {v!r}.") + return v + + +def test_validator_valueerror_returns_422_not_500() -> None: + """A field_validator ValueError (raw exc in ctx) must render a clean 422, + not crash the handler into a 500. Reproduces the live i_am_blocked + blocker_type='task_complete' crash.""" + app = FastAPI() + setup_middleware(app) + + @app.post("/enum") + async def _e(_data: _EnumFieldBody) -> Any: + return {"ok": True} + + client = TestClient(app, raise_server_exceptions=False) + response = client.post("/enum", json={"kind": "task_complete"}) + assert response.status_code == HTTPStatus.UNPROCESSABLE_ENTITY + body = response.json() + assert "detail" in body + # The human-readable validator message must survive serialization. + assert "kind must be one of" in str(body["detail"]) + + # --------------------------------------------------------------------------- # secret scrubbing in the 422 log line # ---------------------------------------------------------------------------