Fix Bedrock streaming dropping all chunks when Faraday env is nil#813
Open
chen-anders wants to merge 2 commits into
Open
Fix Bedrock streaming dropping all chunks when Faraday env is nil#813chen-anders wants to merge 2 commits into
chen-anders wants to merge 2 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #813 +/- ##
=======================================
Coverage 82.88% 82.88%
=======================================
Files 142 142
Lines 6574 6574
Branches 1148 1148
=======================================
Hits 5449 5449
- Misses 663 664 +1
+ Partials 462 461 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
The Faraday 2 on_data callback gates chunk parsing on `env&.status == 200`. With the net_http adapter, env is nil during streaming (the status is not yet known), so this is false for every chunk. Each valid AWS event-stream frame is then routed to the failed-response handler, which tries to JSON-parse binary eventstream bytes, fails, logs a "failed stream error chunk" debug line, and drops it. The result: ConverseStream responses come back completely empty (zero chunks, blank content, nil token counts) even though the HTTP request itself succeeds with status 200. Invert the guard so a nil env (status unknown) falls through to normal chunk parsing, and only a present env reporting a non-200 status is treated as a failure. Applied in both the Converse streaming module and the shared FaradayHandlers.v2_on_data helper.
Cover the regression where a nil Faraday env (Faraday 2 + net_http during streaming) caused every chunk to be routed to the failed-response handler and discarded. Tests exercise both the shared FaradayHandlers.v2_on_data helper and the Converse stream_response on_data proc directly, asserting that a nil env and a 200 env both parse the chunk while a present non-200 env triggers failure handling.
8321563 to
a1093e9
Compare
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.
What this does
Fixes a bug where Bedrock ConverseStream responses come back completely empty (zero chunks, blank content, nil token counts) even though the HTTP request succeeds with status 200.
The Faraday 2
on_datacallback gates chunk parsing onenv&.status == 200. With thenet_httpadapter,envis nil during streaming (the status is not yet known when chunks arrive), so this condition is false for every chunk. Each valid AWS event-stream frame is then routed to the failed-response handler, which tries toJSON.parsebinary eventstream bytes, fails, logs a "failed stream error chunk" debug line, and silently drops it. The result is an empty streamed response.This affects any app on Faraday 2.x + the net_http adapter (Faraday's default) using Bedrock streaming.
Fix: invert the guard so a nil env (status unknown) falls through to normal chunk parsing, and only a present env reporting a non-200 status is treated as a failure. Applied in both spots that build the v2
on_dataproc:RubyLLM::Streaming::FaradayHandlers.v2_on_data(shared SSE handler)RubyLLM::Protocols::Converse::Streaming#stream_response(Bedrock ConverseStream)Reproduction
With Faraday 2.x + net_http, the debug log shows the data arriving fine (
{"delta":{"text":"BANANA"}}, usage tokens, status 200) but every frame hitting "Failed Bedrock stream error chunk". After the fix:chunks.size => 5,resp.content => "BANANA", tokens populated.Type of change
Scope check
Quality check
overcommit --installand all hooks passbundle exec rake vcr:record[provider_name]spec/ruby_llm/streaming_spec.rb,spec/ruby_llm/protocols/converse/streaming_spec.rb)models.json,aliases.json)AI-generated code
API changes