HTTP Client 5.1.0: Compression support and streaming error resilience#53
Merged
Merged
Conversation
… streaming crash on non-streaming responses
## Why This Change Was Made
- Streaming HTTP requests (start_stream / stream_yielder) crashed, hung, or
silently timed out when the upstream returned a complete HTTP error response
(401, 403, 429, 500 with a body) instead of starting an SSE stream. Erlang's
httpc sends a complete response tuple that was unhandled in four locations in
dream_httpc_shim.erl. The on_stream_error callback was never invoked, the
caller timed out after 30 seconds (504), and the upstream error details were
lost entirely.
- The HTTP client did not support compressed responses. Servers that support
gzip/deflate compression were always sending uncompressed payloads because
the client never advertised Accept-Encoding.
## What Was Changed
- Added format_complete_response_error/3 helper and a match clause for the
complete response tuple {Ref, {{Version, StatusCode, Reason}, Headers, Body}}
in all four affected streaming locations:
1. decode_stream_message_for_selector/1 — routes to stream_error
2. receive_stream_message/1 — returns stream_error with status and body
3. stream_owner_wait/5 — buffers error for delivery via fetch_next
4. stream_owner_next_message/2 — returns error immediately
- Added transparent gzip/deflate decompression across all three execution modes:
- request_sync/5 (send) — decompresses full response body
- stream_owner_wait/5 (stream_yielder) — zlib inflate context threaded through
process state, decompresses chunks on-the-fly, cleaned up on end/error
- decode_stream_message_for_selector/1 and receive_stream_message/1
(start_stream) — ETS-stored zlib contexts per stream, same lifecycle
- Added maybe_add_accept_encoding/1 that injects "Accept-Encoding: gzip, deflate"
unless the user already set their own
- Added maybe_decompress_response/2 with try_decompress/3 for sync decompression
with graceful error handling on corrupted data
- Added detect_stream_encoding/1, maybe_init_stream_zlib/1, init_zlib_context/1,
decompress_chunk/2, cleanup_zlib/1 for streaming zlib lifecycle
- Added ETS-based zlib management: maybe_store_stream_zlib/2,
maybe_decompress_stream_chunk/2, cleanup_stream_zlib/1
- Added 9 mock server endpoints (6 non-streaming + 3 streaming) for compression
testing, with compression_ffi.erl Erlang FFI for zlib:gzip/compress/streaming
- Added 34 new tests (10 regression + 24 compression permutation matrix)
- Bumped version to 5.1.0, consolidated release notes
## Note to Future Engineer
- The complete response tuple from httpc is {Ref, {{Version, StatusCode, Reason},
Headers, Body}} which looks nothing like the streaming tuples
{Ref, stream_start, Headers} etc. httpc decides which format to use based on
whether the server actually streams — you don't get to choose. Surprise!
- stream_owner_wait is now /5 (was /4) and stream_owner_next_message is now /2
(was /1) because they thread a ZlibCtx parameter. If you add a new message
handler to either function, don't forget to pass ZlibCtx through or you'll
lose the decompression context and get garbage bytes. You're welcome.
- The stream_yielder tests in stream_non_streaming_response_test.gleam use
yielder.take(1) instead of yielder.to_list because the yielder retries
infinitely on start errors (owner never gets set to Some). If you switch to
to_list, your test will run until the heat death of the universe.
- Decompression of corrupted gzip data falls back to raw passthrough with a
warning instead of crashing. This is intentional — we'd rather give you
garbage bytes you can debug than kill your process with no explanation.
- All four streaming locations need the complete response fix. If you only patch
decode_stream_message_for_selector you'll fix start_stream but leave
stream_yielder hanging forever. Ask me how I know.
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
1 participant
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.
Summary
Accept-Encoding: gzip, deflateand decompresses responses across all three execution modes (send(),stream_yielder(),start_stream()), with no API changes requiredContent-Encodingvalues pass through raw bytes with a warning; corrupted compressed data falls back to raw passthrough instead of crashingWhy
Streaming requests would crash or silently time out when the upstream returned a non-streaming error response. Erlang's
httpcsends a completely different message format for complete responses vs streamed chunks, and four locations in the shim were not handling the complete response case. This made it impossible to distinguish between a genuine timeout and an upstream auth failure.Separately, the HTTP client never advertised compression support, so servers were always sending uncompressed payloads even when they supported gzip or deflate. For large response bodies and streaming data, this is a significant bandwidth and latency cost.
What Changed
Streaming crash fix (4 locations in
dream_httpc_shim.erl):decode_stream_message_for_selector/1— routes complete responses tostream_errorreceive_stream_message/1— returnsstream_errorwith status and bodystream_owner_wait/5— buffers error for delivery viafetch_nextstream_owner_next_message/2— returns error immediatelyCompression (all in
dream_httpc_shim.erl+ mock server):Accept-Encoding: gzip, deflateunless user provides their ownHow
format_complete_response_error/3and match clauses for the{Ref, {{Version, StatusCode, Reason}, Headers, Body}}tuple in all four streaming locationsmaybe_add_accept_encoding/1,maybe_decompress_response/2,try_decompress/3for sync decompressiondetect_stream_encoding/1,init_zlib_context/1,decompress_chunk/2,cleanup_zlib/1for streaming zlib lifecyclemaybe_store_stream_zlib/2,maybe_decompress_stream_chunk/2,cleanup_stream_zlib/1) for the message-based pathcompression_ffi.erlandcompression.gleamin the mock server for server-side test compressionTest Coverage
34 new tests (168 total). 10 regression tests for the streaming crash fix, 24 compression tests covering the full permutation matrix: gzip/deflate/identity/unknown/corrupted × sync/stream-callback/stream-pull × header injection scenarios × zlib cleanup verification.
See release notes for full details.