Repository navigation
Conversation
async_request_streamed raises APIError from inside its `async for` over the
response when the server sends an `{"error": ...}` event. Leaving an
`async for` by exception does not close the iterator, so
HttpResponse.async_segments and the _aiter_response_stream it reads stay
suspended. The connection is released only when the garbage collector
finalizes them, which asyncio does later in a task of its own, outside the
Context of the request that failed.
Wrap both loops in contextlib.aclosing, so the stream is closed before the
error reaches the caller. Annotate the two generators as AsyncGenerator so
the type checker accepts them as closeable.
|
Hi @rcleveng, Thanks for reaching out us! |
Author
|
Hey @Venkaiahbabuneelam I updated it yesterday and then again today - PTAL |
This branch has not been deployed
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.
Problem
When the server sends an
{"error": ...}event partway through a streamed response,BaseApiClient.async_request_streamedraisesAPIErrorfrom inside itsasync forover the response. Leaving anasync forby exception does not close the iterator, so two generators stay suspended:HttpResponse.async_segmentsHttpResponse._aiter_response_stream, whichasync_segmentsreadsThe HTTP response is closed only when the garbage collector finalizes them. asyncio runs that finalization later, in a task of its own, outside the Context of the request that failed. In production we saw this after mid-stream
504 DEADLINE_EXCEEDEDerrors from Gemini: the connection was released late, and context-dependent cleanup (OpenTelemetry spans,ContextVarresets) ran in the wrong Context.Fix
Wrap both loops in
contextlib.aclosing, so the stream is closed before the error reaches the caller. Each wrapper is needed: closingasync_segmentsalone leaves_aiter_response_streamsuspended, because aGeneratorExitleaving anasync fordoes not close that iterator either.async_segmentsand_aiter_response_streamare now annotatedAsyncGeneratorrather thanAsyncIterator, so mypy accepts them as closeable.Testing
test_error_event_in_streamed_responses_async_closes_stream. The mock server sends an error event and keeps the connection open, and the test asserts the stream'saclosehas already run whenServerErrorsurfaces. It fails without the change, and it fails with eitheraclosingremoved.google/genai/testsin--mode tap: no new failures. The tests that fail do so identically with and without the change, because the replay files are not in the public repo.mypy google/genai/: no new errors.