Conversation
__decode__ and AsyncJSONLDecoder.__decode__ only closed the underlying http_response when the generator ran to completion. a json decode error, or a transport error from the underlying byte stream, propagated straight out of the generator and left the connection open, since close() was never called on any non-normal exit path. wrapped both generator bodies in try/finally calling close()/await close(), matching how the SSE stream decoder already handles this. verified with two new tests that construct a real httpx2 stream (not a bare Response(200), which starts pre-closed and would make the test a no-op) and confirm response.is_closed is True after a decode error - both fail on unfixed main and pass with the fix.
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.
Description
When a batch-result JSONL stream raises a JSON decoding error, its HTTP response stays open.
JSONLDecoder.__decode__/AsyncJSONLDecoder.__decode__only closehttp_responsewhen the generator runs to completion normally — an exception raised mid-decode (fromjson.loads, or a transport error from the underlying byte stream) propagates straight out and skips the close entirely.Fixed by wrapping both generator bodies in try/finally calling
close()/await close(), matching the existing cleanup pattern already used by the SSE stream decoder (Stream.__stream__/AsyncStream.__stream__).Closes #1928
Verification
Added
test_sync_closes_response_on_decode_errorandtest_async_closes_response_on_decode_errortotests/decoders/test_jsonl.py. These construct a realhttpx2.Responsebacked by an actualByteStream(not a bareResponse(200), which startsis_closed=Truewith nothing to close and would make the test a no-op regardless of the fix — caught this while writing the test), feed invalid JSON through the decoder, and assertresponse.is_closedafterward. Both confirmed to fail on unfixed main and pass with the fix. Fulltests/decoders/test_jsonl.pysuite: 8 passed.I initially also wrote tests for a raised transport error (
httpx2.ReadErrorfrom the byte stream itself), but those passed regardless of the fix —httpx2's owniter_bytes()/aiter_bytes()already closes the response internally when the underlying stream raises, so that specific path isn't a real gap. Kept only the two tests that actually distinguish fixed from unfixed behavior.