Repository navigation
Conversation
… is answered
FastAPI raises `RequestValidationError` before a route runs, so it never reached the
`AMPError` handler: a body the schema rejects answered `{"detail": [...]}` while
every other error answered `{"error": {"code", "message", "details"}}`. The API
reference documented `422 VALIDATION_ERROR` all along, and both SDKs read
`error.code` - so a caller that sent a bad field got a generic "HTTP error 422" and
no idea which field was wrong. That is the exact failure the single error shape was
introduced to remove in the first place; it just had a hole where the framework
replies before the application does.
- One handler for `RequestValidationError`, returning the protocol envelope with
`code: VALIDATION_ERROR`. The field errors move into `error.details.errors`,
which is what `details` is for.
- `jsonable_encoder` on those errors, so a rejected value that is not JSON
serialisable (a bytes body, say) cannot turn this handler into a 500 - the error
path must not be the thing that breaks.
- **The contract is corrected too.** FastAPI documents its own validation body on
every route that can reject input, so the committed `openapi.json` told client
generators to expect `HTTPValidationError` while the server sent something else.
`document_the_error_envelope` rewrites each 422 to `ErrorResponse` and drops the
now-unreferenced `HTTPValidationError`/`ValidationError` components, because a
contract is what somebody generates a client from.
- The conformance suite gained a vector, so any implementation is held to the same
shape rather than only this one (39 vectors now, 39/39 against a live server).
The first version of the schema cleanup deleted *every* component, including
`ErrorResponse`, because it searched for references inside the components block
instead of the whole document - most refs live in the paths. The openapi contract
test caught it on the first run, which is the argument for that test existing.
Owner
Author
|
Landed on master in the v0.1.0 chain: the branch was fast-forward merged as part of |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
fix(server): answer a rejected request body the way every other error is answered
FastAPI raises
RequestValidationErrorbefore a route runs, so it never reached theAMPErrorhandler: a body the schema rejects answered{"detail": [...]}whileevery other error answered
{"error": {"code", "message", "details"}}. The APIreference documented
422 VALIDATION_ERRORall along, and both SDKs readerror.code- so a caller that sent a bad field got a generic "HTTP error 422" andno idea which field was wrong. That is the exact failure the single error shape was
introduced to remove in the first place; it just had a hole where the framework
replies before the application does.
RequestValidationError, returning the protocol envelope withcode: VALIDATION_ERROR. The field errors move intoerror.details.errors,which is what
detailsis for.jsonable_encoderon those errors, so a rejected value that is not JSONserialisable (a bytes body, say) cannot turn this handler into a 500 - the error
path must not be the thing that breaks.
every route that can reject input, so the committed
openapi.jsontold clientgenerators to expect
HTTPValidationErrorwhile the server sent something else.document_the_error_enveloperewrites each 422 toErrorResponseand drops thenow-unreferenced
HTTPValidationError/ValidationErrorcomponents, because acontract is what somebody generates a client from.
shape rather than only this one (39 vectors now, 39/39 against a live server).
The first version of the schema cleanup deleted every component, including
ErrorResponse, because it searched for references inside the components blockinstead of the whole document - most refs live in the paths. The openapi contract
test caught it on the first run, which is the argument for that test existing.