Repository navigation
fix(sagemaker): parse every streamed record however SageMaker cuts the payload parts - #47488
mateo-berri wants to merge 1 commit into
Conversation
…e payload parts SageMaker cuts a container's response stream into PayloadParts at arbitrary byte offsets, so one part can hold several SSE events or JSON lines, or only part of one. The decoder treated each part as one record, so vLLM's [DONE] was logged at ERROR on every stream and a split at an unlucky byte dropped the rest of the answer. Peel records off a running buffer with JSONDecoder.raw_decode, skipping SSE framing and [DONE] between them.
litellm-stampbot: risk highNot approved at Rules (
Cursor Bugbot: Medium Risk at Pushes do not re-run this check. Comment |
|
bugbot run |
Merging this PR will not alter performance
|
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit eb9f9e9. Configure here.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
Superseded by #47495, which carries the same fix and tests after a smaller opening commit that stampbot could approve |
TLDR
Problem this solves:
Unparseable JSON data remained: [DONE]at ERROR every timeHow it solves it:
data:,[DONE], whitespace) between recordsUser Flow
Before: an app streaming chat completions from a vLLM model on a SageMaker endpoint gets some answers cut short with no error, and the proxy log fills with ERROR lines
sagemaker_chat/<endpoint>deployment for an endpoint running a vLLM container"stream": trueand"stream_options": {"include_usage": true}LiteLLM:ERROR ... Unparseable JSON data remained: [DONE]to the proxy log..., 23, 24, 25, 2) yet still ends withfinish_reason: "stop", a 200, anddata: [DONE]After: the same streams arrive whole with the container's token counts and nothing in the error log
sagemaker_chat/<endpoint>deployment for an endpoint running a vLLM container"stream": trueand"stream_options": {"include_usage": true}Relevant issues
Replaces #31598 by @twerkmeister, which found the
[DONE]log spam. That PR strips a trailing[DONE]at stream end, but the lost chunks come from SageMaker splitting or merging events across parts, so this PR fixes the parsing itself. Credit for the report and the first fix goes to that PRDesign decisions
The framing skipped between records is whitespace,
data:, and[DONE]only, since the containers on this route (vLLM, TGI, LMI) send data-only SSE or JSON lines. A full SSE parser would also handleevent:,id:, and:comment lines, which stall the stream today as they did before, but no container here sends them, so it stays out of scopeA record that fails validation on the async path is logged and drops the rest of its own part, as before. Decoding each part as UTF-8 on its own is unchanged, since #46884 and #46887 already cover a multi-byte character split across parts
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
uv run pytest tests/unit/<your_test_file>.py -v. Leave the suites (make test-unit-*,make test-unit) to CI: it finishes in ~15 minutes where a laptop takes an hour or moreDelays in PR merge?
If you're seeing a delay in your PR being merged, ping the LiteLLM Team on Slack (#pr-review).
Screenshots / Proof of Fix
Before is the merge base fb555b0 and After is this PR's head eb9f9e9. Each leg ran one proxy with 2 uvicorn workers and no database, from that commit's tree, with the same config and requests against a real SageMaker endpoint (ml.g5.xlarge, AWS vLLM 0.32.0 container, Qwen/Qwen3.5-0.8B) and a JumpStart TGI endpoint for the
sagemaker/route. Raw client output and both proxy consoles are in this gistconfig.yaml and proxy command
python -m litellm.proxy.proxy_cli --config config.yaml --port $P --num_workers 2/v1/chat/completionsstream/v1/responsesstream/v1/messagesstreamsagemaker/TGI streamEvery stream on both legs ended with the container's own usage chunk, so no answer was cut off. Before logged one ERROR per vLLM stream, 183 over 184 requests, and After logged none over 94
Single chat stream
Both legs stream the same answer
1, 2, 3, 4, 5withfinish_reason: stop, the container's usage (23 in, 14 out), anddata: [DONE]. Before, the proxy console printsAfter, it prints
Full client output, Before (fb555b0)
Full client output, After (eb9f9e9)
/v1/responses and /v1/messages streams
Commands and output
Both legs return the same text ending
..., 28, 29, 30./v1/responsesends withresponse.completedcarrying usage 26 in and 110 out, and/v1/messagesends withmessage_deltacarryingstop_reason: end_turnand the same usage. Before, the proxy console printsLiteLLM:ERROR: common_utils.py:262 - Warning: Unparseable JSON data remained: [DONE]after each request. After, it prints only the access linesFull output is in the gist as before-responses, after-responses, before-messages, and after-messages
sagemaker/ TGI stream
Command and output
Both legs return the same answer with
finish_reason: stop, usage 20 in and 26 out, anddata: [DONE], with no ERROR line on either proxy (before, after)30 concurrent streams
Command and output
Output on both legs
Every stream on both legs ends with the container's own usage chunk, so none was cut off. The Before proxy console prints the
[DONE]ERROR line 30 times, and the After console prints 30 access lines and nothing else (before, after)An earlier Before run on a single-worker proxy at d56ff28, whose SageMaker stream decoder is byte-identical to fb555b0, saw 3 of 30 answers stop mid-stream (
..., 23, 24, 25, 2) while still ending infinish_reason: "stop", a 200, anddata: [DONE], with usage LiteLLM estimated from the cut text (prompt 20 instead of the container's 26). Its console showed the lost rest of each answer asUnparseable JSON data remained: data: {"id":"chatcmpl-...","choices":[{"index":0,"delta":{"content":"6"}}]}{"id":...}{...}. The 2-worker runs above did not hit a cut-off on either leg. The unit and integration tests in this PR replay those part boundaries and fail on mainSDK sync stream
Code and output
Before (fb555b0)
After (eb9f9e9)
The short vLLM answer in Before run 2 carries the container's own usage (3 tokens out), so it is the model's answer, not a cut-off
What surprised the run
/v1/messagesstart event hasinput_tokens0, both legs, unchangedType
🐛 Bug Fix
Caveats (if any)
Final Attestation