fix(api): declare the real v1 error envelope and the SSE content type (#116) - #136
Merged
Conversation
…#116) Two response contracts in the committed spec did not match what the app returns. Since that spec bootstraps the Kong gateway and is what clients would be generated from, both produce clients that mis-parse real responses. 422: app/core/errors installs a RequestValidationError handler returning the Range42 envelope (error/message/code/details/trace_id/timestamp), but every v1 operation advertised FastAPI's default {"detail": [...]}. Added ErrorEnvelope mirroring _envelope() and declared it once on the aggregate v1 router, so all 48 operations inherit it rather than each route repeating itself. SSE: /v1/deployments/{id}/events returns EventSourceResponse, so its body is text/event-stream. It advertised application/json, which would have a generated consumer awaiting a body that never completes instead of opening a stream. The first test asserts a real 422 body and the declared schema have the same keys — asserting on the spec alone would let both drift together. v0 still advertises the default 422; those routes do not go through the v1 handlers and are decom-track anyway, so they are deliberately left.
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.
Closes #116. Both findings originally from Codex review on #115.
The committed spec bootstraps the Kong gateway and is what clients would be generated from, so a wrong declaration is not cosmetic — it produces clients that mis-parse real responses.
422 shape
app/core/errorsinstalls aRequestValidationErrorhandler returning the Range42 envelope:{"error": "...", "message": "...", "code": "...", "details": [...], "trace_id": "...", "timestamp": "..."}…while all 48 v1 operations advertised FastAPI's default
{"detail": [...]}.Added
ErrorEnvelope(mirroring_envelope()) and declared it once on the aggregate v1 router viaresponses=, so every operation inherits it rather than 48 routes each repeating the same block — and a new v1 route gets it automatically.SSE content type
/v1/deployments/{deployment_id}/eventsreturnsEventSourceResponse, so its body istext/event-stream. The spec saidapplication/json, which would have a generated consumer awaiting a body that never completes instead of opening a stream. Now declared withresponse_classplus an explicittext/event-streamresponse.Tests
Three, all failing against
dev:The first is the important one: it POSTs an invalid body, reads the real 422, and asserts its keys match the declared schema's properties. Asserting on the spec alone would let the declaration and the handler drift together — which is exactly how this bug existed in the first place.
Deliberately not changed
The 48 v0 operations still advertise the default 422. They do not go through the v1 handlers, and they are on the decom track (#113 removed a large chunk already), so aligning them would be churn on surface we intend to delete. Verified the remaining
HTTPValidationErrorreferences are v0-only.491 passed, ruff clean, spec regenerated — and #134's drift guard now enforces that.