fix(streaming): add aclose() to AsyncStream for standard async cleanup protocol#2944
Open
s-zx wants to merge 2 commits intoopenai:mainfrom
Open
fix(streaming): add aclose() to AsyncStream for standard async cleanup protocol#2944s-zx wants to merge 2 commits intoopenai:mainfrom
s-zx wants to merge 2 commits intoopenai:mainfrom
Conversation
…ad. block Two related bugs in Stream.__stream__ and AsyncStream.__stream__: 1. JSONDecodeError on meta-only SSE events (fixes openai#2722) The SSE specification allows events with no data field (e.g. standalone 'retry:' or 'id:' directives). The SDK's SSE parser correctly sets data='' for these but __stream__ called sse.json() unconditionally, raising JSONDecodeError: Expecting value. Fix: skip events whose data is empty or whitespace before any JSON parsing. 2. Unreachable sse.event == 'error' check (fixes openai#2796) The error-event guard was nested inside the startswith('thread.') branch, making it logically impossible to trigger because 'error' != 'thread.*'. This was a regression from commit abc2596 ('fix(streaming): correct indentation') where the check was accidentally moved inside the block. Fix: move the error-event handling to the else branch (non-thread events), which is the correct location for standalone 'error' SSE events.
…p protocol
AsyncStream exposes close() but not aclose(), causing AttributeError
when instrumentation libraries (e.g. Langfuse, OpenTelemetry wrappers)
call the standard Python async cleanup convention:
await stream.aclose()
# AttributeError: 'AsyncStream' object has no attribute 'aclose'
The error surfaces in production via the call chain:
AsyncChatCompletionStreamManager.__aexit__
-> AsyncChatCompletionStream.close()
-> self._response.aclose() <- AttributeError when _response is
AsyncStream (happens when the raw
stream is wrapped by instrumentation)
aclose() is the standard async cleanup method used by asyncio.StreamWriter,
httpx.AsyncByteStream, and async generators (PEP 525). Add it as a
thin alias that delegates to close() so callers can use either name
without special-casing AsyncStream.
Fixes openai#2853
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
AsyncStreamexposesclose()but notaclose(), causingAttributeErrorwhen instrumentation libraries (Langfuse, OpenTelemetry wrappers, etc.) call the standard Python async cleanup convention:The error surfaces in production via the call chain in
AsyncChatCompletionStream:When instrumentation wraps the raw
AsyncStream,self._responseresolves to theAsyncStreamitself rather than the underlyinghttpx.Response. SinceAsyncStreamhasclose()but notaclose(), cleanup fails.Closes #2853
Fix
Add
aclose()as a thin alias that delegates toclose():aclose()is the standard async cleanup method used byasyncio.StreamWriter,httpx.AsyncByteStream, and async generators (PEP 525). Adding it lets callers and wrappers use either name without special-casingAsyncStream.The sync
Streamclass is not affected since its callers useclose().