fix(bedrock): emit SSE error event when invoke Messages stream ends without message_stop - #32159
Conversation
…ithout message_stop
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Greptile SummaryThis PR fixes a silent failure in Bedrock invoke
Confidence Score: 5/5Safe to merge: the change is narrowly scoped to the SSE wrapper's post-stream finalization path, existing complete streams are fully unaffected, and the false-positive/double-error edge cases are explicitly covered by regression tests. The guard is a late-stage yield appended only when the upstream iterator exhausts without a terminal frame; it does not touch the hot path for normal streams. Terminal-event detection uses exact SSE-line matching rather than substring search, closing the previously-reported false-positive path. The intentional exclusion of the synthetic event from collected_chunks (to protect billing) is documented, tested, and explained. No auth, routing, or schema changes are present. No files require special attention.
|
| Filename | Overview |
|---|---|
| litellm/llms/anthropic/experimental_pass_through/messages/streaming_iterator.py | Adds terminal-event detection and synthetic error SSE emission when streams end without message_stop; helper functions are clean and use precise line-level matching to prevent false positives. |
| tests/test_litellm/llms/anthropic/experimental_pass_through/messages/test_streaming_iterator.py | New test file with comprehensive mock-only coverage: truncated/complete/empty streams, bytes passthrough, false-positive substring regression, double-error prevention, and logging exclusion of synthetic events. |
| tests/test_litellm/llms/bedrock/messages/invoke_transformations/test_anthropic_claude3_transformation.py | Adds two integration-level regression tests through bedrock_sse_wrapper for truncated and complete streams; follows the pre-existing pattern in this file. |
Reviews (3): Last reviewed commit: "test(bedrock): lock in that the syntheti..." | Re-trigger Greptile
Greptile SummaryThis PR fixes a silent stream truncation bug in the Bedrock invoke
Confidence Score: 3/5The core fix is sound for the Bedrock dict-chunk path, but the bytes detection path has a correctness gap that could silently suppress the error event on the Anthropic direct-streaming path. The bytes-path _is_message_stop_chunk uses a raw substring match that means any SSE data payload containing the literal text 'message_stop' as content would be treated as a terminal event, setting saw_message_stop=True and causing the injected error sentinel to be skipped for a genuinely truncated stream. The Bedrock dict path is unaffected. The missing collected_chunks.append before logging is a secondary concern affecting observability only. streaming_iterator.py — specifically the _is_message_stop_chunk bytes branch and the missing append of the injected error event into collected_chunks.
|
| Filename | Overview |
|---|---|
| litellm/llms/anthropic/experimental_pass_through/messages/streaming_iterator.py | Adds message_stop detection and injects an Anthropic-protocol error SSE event on truncated streams; bytes detection uses a broad substring match that can false-positive on content containing "message_stop", and the injected error event is not appended to collected_chunks before logging. |
| tests/test_litellm/llms/anthropic/experimental_pass_through/messages/test_streaming_iterator.py | New test file with good coverage: truncated tool_use, complete stream, empty stream, raw-bytes passthrough, and unit tests for the two new helper functions. All mock-only, no network calls. |
| tests/test_litellm/llms/bedrock/messages/invoke_transformations/test_anthropic_claude3_transformation.py | Adds two regression tests for the bedrock_sse_wrapper: truncated mid-tool-use stream and complete stream ending with message_stop; both are dict-based mocks with no real network calls. |
Reviews (2): Last reviewed commit: "fix(bedrock): emit SSE error event when ..." | Re-trigger Greptile
yucheng-berri
left a comment
There was a problem hiding this comment.
gretile comments legit?
|
@coderabbitai review |
|
bugbot run |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 2 potential issues.
Autofix Details
Bugbot Autofix prepared fixes for both issues found in the latest run.
- ✅ Fixed: Substring marks stop falsely
- Replaced the loose
b"message_stop" in chunkcheck with a line-level match on the SSEevent: message_stopheader so payload substrings (e.g. inside a partial_json delta) no longer falsely mark the stream as complete.
- Replaced the loose
- ✅ Fixed: Provider error not terminal
- Introduced
_is_terminal_stream_chunk(message_stop or providererrorevent, for both dict and bytes chunks) and used it in the wrapper so an upstream error terminates the stream without appending a second synthetic incomplete-stream error.
- Introduced
You can send follow-ups to the cloud agent here.
📝 WalkthroughWalkthroughAdds incomplete-stream detection for Anthropic streaming SSE output, emits a synthetic ChangesIncomplete Stream Detection and Error Event Emission
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@litellm/llms/anthropic/experimental_pass_through/messages/streaming_iterator.py`:
- Around line 21-26: The chunk detection in _is_message_stop_chunk is too narrow
and misses raw ModelResponseStream objects coming from
_promote_message_stop_usage, which then causes async_sse_wrapper to treat a
completed stream as incomplete. Update _is_message_stop_chunk and the related
_convert_chunk_to_sse_format path in streaming_iterator.py to recognize and
handle ModelResponseStream chunks as a stop signal, converting them into the
proper SSE stop format instead of passing them through unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1bd4037d-196a-444a-8e69-70ce609effc3
📒 Files selected for processing (3)
litellm/llms/anthropic/experimental_pass_through/messages/streaming_iterator.pytests/test_litellm/llms/anthropic/experimental_pass_through/messages/test_streaming_iterator.pytests/test_litellm/llms/bedrock/messages/invoke_transformations/test_anthropic_claude3_transformation.py
| def _is_message_stop_chunk(chunk: object) -> bool: | ||
| if isinstance(chunk, dict): | ||
| return chunk.get("type") == "message_stop" | ||
| if isinstance(chunk, (bytes, bytearray)): | ||
| return b"message_stop" in chunk | ||
| return False |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check whether GenericStreamingChunk/ModelResponseStream instances are ever fed into async_sse_wrapper directly
rg -n 'async_sse_wrapper\(' -A5 --type=py
rg -nP 'class\s+(GenericStreamingChunk|ModelResponseStream)\b' --type=py -A5Repository: BerriAI/litellm
Length of output: 153
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the relevant file and find the surrounding symbols.
ast-grep outline litellm/llms/anthropic/experimental_pass_through/messages/streaming_iterator.py --view expanded
# Inspect the exact implementation around the helper and wrapper.
nl -ba litellm/llms/anthropic/experimental_pass_through/messages/streaming_iterator.py | sed -n '1,260p'
# Find the type definitions and all direct call sites/usages.
rg -n --hidden --glob '*.py' '\b(GenericStreamingChunk|ModelResponseStream)\b|async_sse_wrapper\(' litellmRepository: BerriAI/litellm
Length of output: 822
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the implementation around the helper, converter, and wrapper.
sed -n '1,220p' litellm/llms/anthropic/experimental_pass_through/messages/streaming_iterator.py
# Find the concrete definitions/usages of the hinted stream types.
rg -n --hidden --glob '*.py' 'class\s+(GenericStreamingChunk|ModelResponseStream)\b|GenericStreamingChunk|ModelResponseStream|async_sse_wrapper\(' litellmRepository: BerriAI/litellm
Length of output: 50371
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Show the call site and the upstream iterator that feeds async_sse_wrapper.
rg -n -A20 -B20 'async_sse_wrapper\(' litellm/llms/bedrock/messages/invoke_transformations/anthropic_claude3_transformation.py
# Inspect the chunk parser / patched stream path for the actual yielded runtime types.
rg -n -A25 -B10 'def _chunk_parser|def _patch|yield .*chunk|return .*chunk|async def .*stream' \
litellm/llms/bedrock/messages/invoke_transformations/anthropic_claude3_transformation.py
# Narrowly inspect the stream type definitions.
sed -n '270,330p' litellm/types/utils.py
sed -n '1760,1835p' litellm/types/utils.pyRepository: BerriAI/litellm
Length of output: 13913
Handle raw ModelResponseStream chunks here. async_sse_wrapper can receive non-dict chunks from _promote_message_stop_usage, but _is_message_stop_chunk only checks dict/bytes. A stream ending on a ModelResponseStream will still trigger the incomplete-stream error, and _convert_chunk_to_sse_format will pass that object through unchanged.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@litellm/llms/anthropic/experimental_pass_through/messages/streaming_iterator.py`
around lines 21 - 26, The chunk detection in _is_message_stop_chunk is too
narrow and misses raw ModelResponseStream objects coming from
_promote_message_stop_usage, which then causes async_sse_wrapper to treat a
completed stream as incomplete. Update _is_message_stop_chunk and the related
_convert_chunk_to_sse_format path in streaming_iterator.py to recognize and
handle ModelResponseStream chunks as a stop signal, converting them into the
proper SSE stop format instead of passing them through unchanged.
…ves and double errors The bytes branch of _is_message_stop_chunk used a plain substring match, so a content_block_delta whose partial_json contained the literal text message_stop would look like a real terminal event and suppress the synthetic incomplete-stream error. Match the SSE event header line instead. Also treat a provider-emitted error event as terminal so a stream that ends with an upstream error is not followed by a second, contradictory synthetic incomplete-stream error.
|
|
…xcluded from logged chunks
|
bugbot run |
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 fe243df. Configure here.
|
@coderabbitai review |
✅ Action performedReview finished.
|
…ithout message_stop (BerriAI#32159) * fix(bedrock): emit SSE error event when invoke Messages stream ends without message_stop * fix(bedrock): tighten stream-terminal detection to avoid false positives and double errors The bytes branch of _is_message_stop_chunk used a plain substring match, so a content_block_delta whose partial_json contained the literal text message_stop would look like a real terminal event and suppress the synthetic incomplete-stream error. Match the SSE event header line instead. Also treat a provider-emitted error event as terminal so a stream that ends with an upstream error is not followed by a second, contradictory synthetic incomplete-stream error. * test(bedrock): lock in that the synthetic truncation error event is excluded from logged chunks --------- Co-authored-by: Cursor Agent <cursoragent@cursor.com>
Relevant issues
Linear ticket
Resolves LIT-3724 (issue 1: silent end-of-iterator)
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@greptileaiand received a Confidence Score of at least 4/5 before requesting a maintainer reviewScreenshots / Proof of Fix
Bedrock cannot be forced to truncate on demand (the customer's truncation was intermittent, ultimately caused by an ALB buffering timeout between their gateway and Bedrock), so the truncation is reproduced end to end with a local mock Bedrock endpoint that streams valid
application/vnd.amazon.eventstreamframes for atool_useblock and then closes the connection midinput_json_delta, exactly like the customer's evidence logs. The proxy config pointsaws_bedrock_runtime_endpointat it:Truncating stream, before this fix (proxy on
litellm_internal_staging): the stream just stops after the partial tool JSON, HTTP 200, no terminal eventSame request, after this fix (proxy on this branch): the client now receives an Anthropic-protocol
errorevent instead of a silent closeReal Bedrock (live API,
us.anthropic.claude-opus-4-1-20250805-v1:0), proving healthy streams are untouched: full tool call streams to completion and ends withmessage_stop, no error event anywhere in the streamQA rerun (e2e, real Bedrock)
Independent end-to-end rerun of both scenarios on a live proxy started from a fresh venv (
pip install -e ".[proxy]"), with the before leg on the base branch at26c0c93dece5182921e11387253a39dd8086db6eand the after leg on this PR's head atfe243dfe27639913382cc813892b56a27d467345, same commands both times. Every call below streams real Opus 4.1 tokens from the real Bedrock API. For the truncation scenario the proxy'saws_bedrock_runtime_endpointpoints at a small local relay that re-signs the incoming request with SigV4 against the realbedrock-runtime.us-east-1.amazonaws.com, streams the real response bytes back to the proxy, and hard-closes the connection after ~12KB of eventstream bytes, which lands midinput_json_deltaof a largewritetool call; the tokens are real Bedrock output and only the connection cut is synthetic, reproducing the customer's middlebox failure. The healthy scenario hits Bedrock directly with no endpoint overrideRequest body used for all four calls (
req_healthy.jsonis identical except"model": "bedrock-healthy"):{ "model": "bedrock-truncating", "max_tokens": 3000, "stream": true, "tools": [ { "name": "write", "description": "Write a document to a path on disk", "input_schema": { "type": "object", "properties": { "path": {"type": "string"}, "content": {"type": "string"} }, "required": ["path", "content"] } } ], "messages": [ { "role": "user", "content": "Use the write tool to save a detailed 1500-word QA runbook about testing streaming LLM proxies to /builder/docs/QUALITY.md. Call the tool exactly once, putting the entire document in the content field." } ] }Truncating stream, before (base
26c0c93dec): HTTP 200, the stream just stops midinput_json_delta, and grep over the full 131-line capture finds 0event: message_stopand 0event: errorRelay log for that call, confirming the upstream was the real Bedrock API and the cut happened mid-stream:
Truncating stream, after (head
fe243dfe27): the identical request now ends with the Anthropic-protocolerroreventHealthy stream direct to real Bedrock, before (base
26c0c93dec): completes a 2068-output-token tool call and ends withmessage_stop; grep over the full 4100-line capture finds 1event: message_stopand 0event: errorHealthy stream, after (head
fe243dfe27): unchanged behavior; grep over the full 4742-line capture finds 1event: message_stopand 0event: errorType
🐛 Bug Fix
Changes
When a Bedrock invoke
/v1/messagesstream ends without amessage_stopevent (Bedrock or a middlebox drops the connection mid tool call),BaseAnthropicMessagesStreamingIterator.async_sse_wrapperused to pass through whatever events had arrived and close the downstream SSE as a clean HTTP 200. Strict clients (Anthropic SDK, Claude Code) then crash parsing unterminatedtool_useinput JSON, and operators cannot tell the failure from a successasync_sse_wrappernow tracks whether a terminal event was seen; if the upstream iterator is exhausted without one, it appends an Anthropic-protocolerrorSSE event ({"type": "error", "error": {"type": "api_error", ...}}) so clients surface the truncation instead of treating the response as complete. Complete streams are unaffected. Bothmessage_stopand provider-senterrorevents count as terminal, so a stream the provider already ended with anerrorevent does not get a second synthetic error appended. For raw-bytes chunks the detection matches only exactevent: message_stop/event: errorSSE lines rather than a substring search, so payload text that merely mentionsmessage_stopcannot suppress the truncation error. The synthetic error event is deliberately excluded from the chunks handed to the logging pipeline because response reconstruction raises on{"type": "error"}events, which would drop the spend log for the tokens that did streamRegression tests cover the truncated tool_use stream (the exact customer scenario), the complete stream, the empty stream, raw-bytes passthrough chunks, the payload-mention false positive, provider-error-terminated streams (dict and bytes), and the logging exclusion, both at the
BaseAnthropicMessagesStreamingIteratorlevel and throughAmazonAnthropicClaudeMessagesConfig.bedrock_sse_wrapperNote
Low Risk
Narrow streaming guardrail with clear success criteria; only affects streams missing message_stop, with broad test coverage and no auth or request-path changes.
Overview
Fixes LIT-3724: when a Bedrock (or proxy)
/v1/messagesstream stops early—e.g. midtool_use/ partialinput_json_delta—clients used to get HTTP 200 with no terminal event, so Anthropic SDK / Claude Code could treat the response as complete and fail on broken tool JSON.BaseAnthropicMessagesStreamingIterator.async_sse_wrappernow tracks whether any chunk is amessage_stop(dicttypeor raw bytes containingmessage_stop). If the upstream iterator finishes without one, it appends an Anthropic-style SSEerrorevent (api_errorwith a fixed incomplete-stream message). Normal streams that end withmessage_stopare unchanged.Regression tests cover truncated tool-use, complete streams, empty streams, and byte passthrough at the base iterator and via
bedrock_sse_wrapper.Reviewed by Cursor Bugbot for commit 478b837. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
errorevent describing the incomplete stream.errorSSE event.