fix(frontend): return JSON errors on the remaining OpenAI routes - #13685
fix(frontend): return JSON errors on the remaining OpenAI routes#13685ayaangazali wants to merge 1 commit into
Conversation
ai-dynamo#13102 gave `/v1/responses`, `/v1/chat/completions` and `/v1/completions` a JSON error envelope for a bad `Content-Type`, an oversized body and malformed JSON, by reading the body through `read_json_request_body` and `parse_json_request`. Eight handlers were left on the plain `Json<T>` extractor and still return Axum's `text/plain` rejection, so the envelope depends on which endpoint the client happened to call: POST /v1/responses + Content-Type: text/plain 415 application/json {"message":"Expected request with Content-Type application/json",...} POST /v1/embeddings + Content-Type: text/plain 415 text/plain Expected request with `Content-Type: application/json` A client that parses the error body works against one endpoint and fails against the other, for the same mistake and the same status code. Route the remaining handlers through the same two helpers: embeddings, classify, pooling, images, image edits, videos, video stream and audio speech. They already took `HeaderMap` and already returned `Result<Response, ErrorResponse>`, so each is a signature swap plus the two lines ai-dynamo#13102 established. `images_edits` validates `input_reference` and then delegates to `images`, which now consumes a `Body` it has already read. The part of `images` after parsing moves into `images_with_request`, called by both. No change to any success path, and no change to the error text; only the envelope and content type on the eight endpoints that were inconsistent. Signed-off-by: ayaangazali <ayaangazali.work@gmail.com>
|
👋 Hi ayaangazali! Thank you for contributing to ai-dynamo/dynamo. Just a reminder: The 🚀 |
github-actions
Bot
added
fix
external-contribution
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 12 included reviews per hour; 11 remain after this review. WalkthroughOpenAI service handlers now read bounded request bodies, validate JSON content types, and parse requests through the tolerant JSON parser. Image generation and editing share request processing. An embeddings integration test verifies the JSON-formatted unsupported-media-type response. ChangesOpenAI JSON request handling
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The change standardizes malformed-request errors across the remaining OpenAI routes without changing successful request handling; no actionable merge-blocking risk remains after normal checks and review. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Comment |
![dynamo-review-agent[bot]](https://avatars.githubusercontent.com/u/201626793?s=60&v=4){kind=small}
Summary
#13102 gave
/v1/responses,/v1/chat/completionsand/v1/completionsa JSON error envelope for a badContent-Type, an oversized body and malformed JSON, by reading the body throughread_json_request_bodyandparse_json_request.Eight handlers were left on the plain
Json<T>extractor and still return Axum'stext/plainrejection. The envelope therefore depends on which endpoint the client happened to call. Measured onmain, same mistake, same status code:A client that parses the error body succeeds against one endpoint and throws against the other. That is the defect: not the status, which is already right, but the shape.
This routes the remaining handlers through the same two helpers: embeddings, classify, pooling, images, image edits, videos, video stream, audio speech. They already took
HeaderMapand already returnedResult<Response, ErrorResponse>, so each one is a signature swap plus the two lines #13102 established. No new abstraction and no new error text.One handler needed more than the swap.
images_editsvalidatesinput_referenceand then delegates toimages, which now consumes aBodyit has already read. The part ofimagesafter parsing moves intoimages_with_request, which both call. Two real callers, so it is not a wrapper for its own sake.Overlap worth flagging
@chanh has #13268 open against
ensure_json_content_type, changing an absentContent-Typeto be treated as JSON. This PR adds callers of that helper rather than touching it, so the changes are complementary and should not conflict textually. If #13268 lands first, these eight endpoints inherit the new semantics automatically, which is the point of routing them through the shared helper. Happy to rebase behind it.Validation
Behaviour checked by driving a real server through the existing
spawn_default_serviceharness, before and after:/v1/responses415 application/json/v1/embeddings415 text/plain415 application/json, identical envelopeAdded
test_embeddings_non_json_content_type_returns_json_error, mirroring the/v1/responsestest from #13102 and additionally asserting the responsecontent-type, since the status was already correct before this change and only the envelope was wrong. Confirmed red first by restoringopenai.rsfromupstream/mainand re-running:cargo test -p dynamo-llm --lib --no-default-features: 2042 passed, 0 failed on a clean run.cargo clippyclean,cargo fmt --all -- --checkclean.Disclosing one flake rather than reporting only the clean run:
test_oversized_body_returns_json_413fails intermittently, 3 of 6 runs in isolation on this branch. That is the same rate I measured on cleanmainand reported on #13624; it is a pre-existing race between the 413 response and the client still sending the body, not something this change introduces. The two full-suite runs that failed failed only on that test.Not verified here: I have no GPU, so the success paths of these eight endpoints were not exercised end to end. The change does not touch them, only how the request body is read and how the read is reported when it fails.
Summary by CodeRabbit
415 Unsupported Media Typeresponse with structured error details.