Skip to content

fix: FastAPI-compatible structured 422s for Pydantic model validation errors#178

Open
justrach wants to merge 2 commits into
mainfrom
fix/pydantic-structured-422
Open

fix: FastAPI-compatible structured 422s for Pydantic model validation errors#178
justrach wants to merge 2 commits into
mainfrom
fix/pydantic-structured-422

Conversation

@justrach

Copy link
Copy Markdown
Owner

Summary

Pydantic models already work as request bodies (the model_validate duck-typing path), but their validation failures came back as a stringified 400 blob:

{"error": "Bad Request", "detail": "Validation error for user: 2 validation errors for User\nname\n  String should have at least 1 character [...]"}

while dhi models get proper structured 422s from the Zig-native validator. This PR makes both model layers speak the same FastAPI-compatible error contract:

{"detail": [
  {"loc": ["body", "user", "name"], "msg": "String should have at least 1 character", "type": "string_too_short"},
  {"loc": ["body", "user", "age"], "msg": "Input should be greater than or equal to 0", "type": "greater_than_equal"}
]}

(HTTP 422, matching dhi_validator.zig's output.)

Design constraint: zero hot-path cost

The whole point of TurboAPI is speed, so this is error-path only:

  • RequestParsingError gains an optional errors field — populated only when validation has already failed
  • _validation_error_details() (pydantic e.errors() dicts, or dhi ValidationErrors.errors objects for the pure-Python fallback) runs only inside except blocks
  • Successful requests execute exactly the same instructions as before; the dhi Zig fast path is untouched
  • Non-validation parsing errors keep the legacy 400 shape for backward compatibility

Verification

  • Live server test: pydantic body model now returns structured 422 (above); dhi path byte-identical to before; POST throughput unchanged (~26k req/s with a urllib client, client-bound)
  • tests/test_post_body_parsing.py + test_fastapi_parity.py + test_fastapi_compatibility.py: 92 passed, no new failures
  • Full suite: 404 passed; the 4 failures (test_dhi_model_validation 500, test_satya_0_4_0_compatibility x2, test_binary_responses flaky port) all reproduce on unpatched main with both dhi 1.3.7 and 1.4.1 — pre-existing, unrelated

Test plan

  • pydantic invalid body → 422 {"detail": [{loc, msg, type}]}
  • pydantic valid body → unchanged
  • dhi invalid body → byte-identical Zig 422
  • no new test failures vs baseline

Generated with Devin

… errors

Pydantic models already work as request bodies via the model_validate
duck-typing path, but validation failures were stringified into a
{"error": "Bad Request"} 400 blob instead of FastAPI's structured shape.
dhi models get proper 422s from the Zig-native validator, so the two
model layers disagreed on error contracts.

RequestParsingError now optionally carries structured error details,
extracted from the failing exception:
- pydantic.ValidationError -> e.errors() dicts mapped to {loc, msg, type}
  with loc prefixed by ["body", <param>] (FastAPI convention)
- dhi.ValidationErrors -> .errors list of (field, message) objects
  (covers the pure-Python dhi fallback path)

Catch sites return 422 {"detail": [...]} when structured details exist,
matching the Zig validator's output; everything else keeps the legacy
400 shape for backward compatibility.

Zero hot-path cost: detail extraction only runs after validation has
already failed. Verified no new failures in tests/test_post_body_parsing.py,
test_fastapi_parity.py, test_fastapi_compatibility.py (92 passed; the one
failure, test_dhi_model_validation, pre-exists on main with dhi 1.3.7 and
1.4.1 alike).

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@github-actions

Copy link
Copy Markdown
Contributor

Performance Regression Report

Runner: ci-pr | Duration: 5s per endpoint | Threads: 4 | Connections: 100 | Workers: 24

Endpoint req/s avg latency p99 latency threshold status
GET /health 85,199 0.20ms 0.91ms 80,000 OK
GET / 89,607 0.22ms 1.03ms 80,000 OK
GET /json 83,334 0.24ms 0.88ms 75,000 OK
GET /users/123 85,419 0.22ms 0.84ms 75,000 OK
POST /items 48,463 0.55ms 1.07ms 45,000 OK
GET /status201 79,387 0.24ms 0.84ms 65,000 OK
AVERAGE 78,568 50,000 OK

✅ All endpoints pass regression thresholds

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 21cafad78f

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread python/turboapi/request_handler.py Outdated
if callable(errors_attr): # pydantic.ValidationError
details = []
for err in errors_attr():
loc = ["body", param_name, *(str(p) for p in err.get("loc", ()))]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve numeric loc components in validation errors

When a Pydantic model has nested list validation errors (for example items: list[Child] with an invalid first element), err["loc"] contains integer indexes. Converting every component with str(p) changes FastAPI/Pydantic-style locations like ['body', 'order', 'items', 0, 'name'] into ['body', 'order', 'items', '0', 'name'], so clients or parity tests that rely on the structured error path no longer get the promised FastAPI-compatible shape for array errors.

Useful? React with 👍 / 👎.

Verified against a live FastAPI app with the identical pydantic model:
the 422 payload is now byte-for-byte identical.

- Single body model: loc is ["body", <field>] (no param name), matching
  FastAPI's non-embedded convention; multi-param bodies keep
  ["body", <param>, <field>] via embed=True
- Include "input" and "ctx" from pydantic error dicts when their values
  are JSON-safe (FastAPI runs them through jsonable_encoder; we skip
  exotic values instead of risking a serialization crash on the error path)

Interleaved load test (12 threads, keep-alive): patched vs unpatched
identical within noise on both model layers (~90k req/s dhi, ~85k req/s
pydantic) — the change remains error-path-only.

Generated with [Devin](https://cli.devin.ai/docs)

Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com>
@justrach

Copy link
Copy Markdown
Owner Author

Pushed 0bf3a41 — verified byte-for-byte identical 422s vs a live FastAPI app with the same pydantic model (loc convention, msg, type, input, ctx all match). Also ran an interleaved keep-alive load test (12 threads x 5 rounds): patched vs unpatched within 0.4% on both model layers (~90k req/s dhi, ~85k req/s pydantic) — confirms the change is error-path-only with zero hot-path cost.

@github-actions

Copy link
Copy Markdown
Contributor

Performance Regression Report

Runner: ci-pr | Duration: 5s per endpoint | Threads: 4 | Connections: 100 | Workers: 24

Endpoint req/s avg latency p99 latency threshold status
GET /health 89,177 0.20ms 0.67ms 80,000 OK
GET / 90,460 0.29ms 1.07ms 80,000 OK
GET /json 83,460 0.23ms 1.04ms 75,000 OK
GET /users/123 83,913 0.22ms 0.91ms 75,000 OK
POST /items 48,470 0.58ms 1.20ms 45,000 OK
GET /status201 80,985 0.22ms 0.85ms 65,000 OK
AVERAGE 79,411 50,000 OK

✅ All endpoints pass regression thresholds

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 0bf3a41e20

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +83 to +84
if isinstance(err.get("input"), _JSON_SAFE):
detail["input"] = err["input"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard optional validation input before indexing

When a Pydantic-style errors() result omits the optional input key (for example v1-shaped error dicts or compatible validators), err.get("input") returns None, which passes _JSON_SAFE; the subsequent err["input"] then raises KeyError. Because the helper catches that and returns None, these model validation failures fall back to the legacy 400 response instead of the new structured 422 detail, so check that the key exists before copying it.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant