Repository navigation
fix(aiohttp): reset the upstream connection when a stream closes early, so aborted Bedrock Invoke streams stop - #50657
Conversation
[Medium impact] The PR appears safe to merge, with both numbered findings addressed and no new actionable issues found. SummaryThis PR closes abandoned Bedrock streams through each wrapper and immediately aborts unreleased aiohttp connections. The latest change checks that a provider error also closes the response once.
Reviews (6) · Last reviewed commit: "test(bedrock): assert the converse strea..." · Reviewed by Greptile |
| finally: | ||
| await response.aclose() |
There was a problem hiding this comment.
Required live proof is missing
This changes stream cleanup and billing behavior. Repository guidance requires live before/after proxy proof. Before merging, add reproducible Bedrock disconnect and normal-stream results at afbfaeed500a3d193de8039a18fb05e8b27cae59.
Context Used: AGENTS.md (source)
There was a problem hiding this comment.
Live proof is now in the description. Sonnet 4.5 on Bedrock billed about 1,500 tokens per abort before and 3 after
There was a problem hiding this comment.
Case 4 now covers a full stream too. Three Sonnet 4.5 streams per leg end with end_turn and 126 tokens on both legs
There was a problem hiding this comment.
Thanks—Case 4 closes the full-stream control gap: it demonstrates three completed real Bedrock streams on each leg with matching HTTP 200, end_turn, and 126 output tokens.
One QA detail remains: the before leg is identified as ghcr.io/berriai/litellm:main-latest / v1.111.0-dev.2, but the description doesn’t give the image’s source commit SHA. Without it, I can’t verify that the before run used the PR base or a later default-branch commit. Please add that SHA. The after run is identified as 2eeb586, the latest product commit.
There was a problem hiding this comment.
The before image is main-latest built from ba1257b, 70 commits before the merge base, none touching aiohttp_transport.py or the close path
There was a problem hiding this comment.
Thanks—that identifies the before image and helps show the relevant code was unchanged, but it still falls short of the required baseline: ba1257bda4 is 70 commits before the merge base dc378de207f8e79860d1fd52df26fcce092777e1. The before run must use the merge base or a later default-branch commit; unchanged close-path files don’t replace that requirement. Please rerun the before leg at dc378de207f8e79860d1fd52df26fcce092777e1 (or a later default-branch commit) and include its SHA and results. The after proof remains valid at/after the latest product commit, dccf503fd01a3a9442f654098a3a8bceb440faa7; the later PR commits 2eeb586add1aedd90f1da57daa0c1a063cda5bb9 and aed8398c2f7505f1c6ead73b831668b25502ab47 are test-only.
|
@ryan-crabbe-berri @greptileai Updated the tests in bbdb38b to use Final state holders and fixed datetime(2026, 1, 1), and added live proxy before and after output. |
|
@greptileai re review |
|
bugbot run |
thanks! i'm taking a look |
Merging this PR will not alter performance1 warning
Comparing |
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
…ck_abort_close # Conflicts: # litellm/llms/bedrock/chat/invoke_handler.py
Move SupportsAclose, aclose_if_supported, and aiter_bytes_then_close into litellm/llms/custom_httpx/stream_close.py so the Bedrock invoke paths, the generic HTTP handler, and the Anthropic messages iterator use one copy, and drop the mock-tolerant close in production code
…e its body ends Closing an unfinished aiohttp response on uvloop starts a graceful TLS shutdown that keeps reading the provider's body for up to 30 s. Aborting the transport first ends the upstream read at once.
… exception event ends it
|
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 3dab9ff. Configure here.
mateo-berri
left a comment
There was a problem hiding this comment.
LGTM. Thanks for the contribution!
Handwritten 1-3 sentence description
When
AnthropicMessagesStreamingIterator._abort_upstream_stream()aborts an abandoned/v1/messagesstream onbedrock/invoke/*,self.httpx_responsewasNonebecauseAmazonAnthropicClaudeMessagesConfig.get_async_streaming_response_iteratornever passedhttpx_responseinto the iterator, andAWSEventStreamDecoder.aiter_bytes()did not close its underlying byte iterator on early exit. Passinghttpx_responsethrough and closingbyte_iteratorin afinallyblock ensureshttpx_response.aclose()closes the upstream Bedrock HTTP stream when a client disconnects.Details
TLDR
Problem this solves:
/v1/messagesstreams keep reading from BedrockHow it solves it:
stream_closemoduleUser Flow
Before: a client that drops a long streaming answer leaves Bedrock generating, and CloudWatch counts about 1,500 output tokens for that call
ANTHROPIC_MESSAGES_MAX_DETACHED_STREAM_DRAINS=0and servesbedrock/invoke/eu.anthropic.claude-sonnet-4-5-20250929-v1:0"stream": true,max_tokens16384 and a prompt for a long answercontent_block_deltaOutputTokenCountfor that call reads 1,400 to 1,600 tokensAfter: the same disconnect closes the Bedrock stream at once, and CloudWatch counts 277 to 314 output tokens
ANTHROPIC_MESSAGES_MAX_DETACHED_STREAM_DRAINS=0and servesbedrock/invoke/eu.anthropic.claude-sonnet-4-5-20250929-v1:0"stream": true,max_tokens16384 and a prompt for a long answercontent_block_deltaOutputTokenCountfor that call reads 277 to 314 tokensRelevant issues
Fixes #50514
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 moreScreenshots / Proof of Fix
Last updated: 3dab9ff
Both legs build the repo's
litellm/deploy/docker/Dockerfile(Python 3.13.18, uvloop 0.22.1) and run 2 workers against real Bedrock in eu-central-1. Before is the merge base dc378de and After is the tip 3dab9ff, whose installedaiohttp_transport.pynames_abort_unreleased_connection3 times to Before's 0Shared setup: images, config.yaml, env vars, docker run
Each image is built from its own commit's tree
Tree check inside each container
Cases 1 and 3 run on a pair of proxies with no database and this config.yaml
Env vars, passed with
--env-file:AWS_ACCESS_KEY_ID,AWS_SECRET_ACCESS_KEY,AWS_SESSION_TOKEN,LITELLM_MASTER_KEY,ANTHROPIC_MESSAGES_MAX_DETACHED_STREAM_DRAINS=0Cases 2 and 4 run on a second pair of proxies, each with its own Postgres, using this config.yaml. The
-recmodels go through a recorder that logs and forwards each upstream requestEnv vars: the same five plus
OPENAI_API_KEY,ANTHROPIC_API_KEYandDATABASE_URLCase scripts
Case 1, run as
case3.sh base-dc378de207 29385 eu-central-1and the same forhead-3dab9ffab4. Each abort gets its own 6-minute CloudWatch windowCase 2, run as
sockprobe_ss.sh base-dc378de207 32597 work-e6dbfc97-abbaseand the same forabhead. The sidecar printsbytes_receivedper :443 socket every 2 sCase 3, run as
case4.sh base-dc378de207 29385and the same forhead-3dab9ffab4Case 4, run as
ab_cells.sh base 32597andab_cells.sh head 28642, thenab_analyze.pyon each cells folderBefore (dc378de)
Case 1: Sonnet 4.5 CloudWatch output tokens per abort
OutputTokenCountper call window reads 1569, 1564 and 1462 tokensCase 2: Sonnet 5.5 upstream socket after the client disconnects
Case 3: Sonnet 4.5 full stream
max_tokens300 each return HTTP 200, onemessage_stop,end_turnand 126 output tokensCase 4: the other streaming routes on the same transport
/v1/responsesand/anthropicpass-through/v1/messagesalready stops reading at onceRaw output, Before
DeepSeek reran after a model id fix, and the Responses abort ran twice more
After (3dab9ff)
Case 1: Sonnet 4.5 CloudWatch output tokens per abort
OutputTokenCountper call window reads 314, 287 and 277 tokensCase 2: Sonnet 5.5 upstream socket after the client disconnects
Case 3: Sonnet 4.5 full stream
Case 4: the other streaming routes on the same transport
/v1/messagesRaw output, After
CloudWatch re-read after a token renewal
Recorded upstream requests of the
-reccells, Before against After, frompython3 rec_diff.py abbase abheadhttpx control, the After image with
DISABLE_AIOHTTP_TRANSPORT=TrueThings the QA run noticed. The PR causes none of them
/v1/responsesstream cut before its first output delta writes no spend row on either legGauntlet-mini: SURVIVED, 5/5 lenses, 1 confirmed and fixed, run 5a5e210477eb, reviewed dccf503
Type
🐛 Bug Fix
✅ Test
Caveats (if any)
Low
_loopshared_sessionuser'sssl_shutdown_timeoutgrace is skipped mid-bodyaclose_if_supportedfromstreaming_iterator, notstream_closeDecisions
DISABLE_AIOHTTP_TRANSPORT=True, the httpx transport this PR leaves alone already closes an aborted OpenAI stream within 5 s, so After matches itFinal Attestation
Bugbot summary