From d91d0d75c2846f56c73fcec66923ea5f26974991 Mon Sep 17 00:00:00 2001 From: C1-BA-B1-F3 Date: Fri, 26 Jun 2026 03:15:19 +0800 Subject: [PATCH 1/4] fix: handle null response.output in parse_response When a backend sends 'response.output: null' in a response.completed event (e.g., chatgpt.com Codex backend), parse_response would crash with TypeError: 'NoneType' object is not iterable. This one-line fix coerces None to an empty list, allowing the stream to complete gracefully. Consumers that track output_item.done events can still backfill from their collected items. Fixes #3325 --- src/openai/lib/_parsing/_responses.py | 2 +- tests/lib/test_parsing_responses.py | 87 +++++++++++++++++++++++++++ 2 files changed, 88 insertions(+), 1 deletion(-) create mode 100644 tests/lib/test_parsing_responses.py diff --git a/src/openai/lib/_parsing/_responses.py b/src/openai/lib/_parsing/_responses.py index 232718cef6..ad2297e249 100644 --- a/src/openai/lib/_parsing/_responses.py +++ b/src/openai/lib/_parsing/_responses.py @@ -58,7 +58,7 @@ def parse_response( ) -> ParsedResponse[TextFormatT]: output_list: List[ParsedResponseOutputItem[TextFormatT]] = [] - for output in response.output: + for output in (response.output or []): if output.type == "message": content_list: List[ParsedContent[TextFormatT]] = [] for item in output.content: diff --git a/tests/lib/test_parsing_responses.py b/tests/lib/test_parsing_responses.py new file mode 100644 index 0000000000..aab8d66567 --- /dev/null +++ b/tests/lib/test_parsing_responses.py @@ -0,0 +1,87 @@ +"""Tests for parse_response handling of null/None output fields.""" + +from __future__ import annotations + +from openai._models import construct_type_unchecked +from openai._types import Omit +from openai.lib._parsing._responses import parse_response +from openai.types.responses import Response, ParsedResponse + + +def _make_response(output=None, **kwargs): + """Helper to construct a Response with a given output field.""" + base = { + "id": "resp_test123", + "created_at": 1234567890.0, + "model": "gpt-4o", + "object": "response", + "status": "completed", + "output": output, + "parallel_tool_calls": True, + "tool_choice": "auto", + "tools": [], + "temperature": 1.0, + "top_p": 1.0, + } + base.update(kwargs) + return construct_type_unchecked(type_=Response, value=base) + + +def test_parse_response_with_none_output(): + """Test that parse_response handles null output without crashing.""" + response = _make_response(output=None) + assert response.output is None + + result = parse_response( + text_format=None, + input_tools=None, + response=response, + ) + + assert isinstance(result, ParsedResponse) + assert result.output == [] + + +def test_parse_response_with_empty_list_output(): + """Test that parse_response handles empty list output correctly.""" + response = _make_response(output=[]) + assert response.output == [] + + result = parse_response( + text_format=None, + input_tools=None, + response=response, + ) + + assert isinstance(result, ParsedResponse) + assert result.output == [] + + +def test_parse_response_with_message_output(): + """Test that parse_response still works correctly with actual output items.""" + output_data = [ + { + "id": "msg_test123", + "type": "message", + "status": "completed", + "role": "assistant", + "content": [ + { + "type": "output_text", + "text": "Hello, world!", + "annotations": [], + } + ], + } + ] + response = _make_response(output=output_data) + + result = parse_response( + text_format=Omit(), + input_tools=None, + response=response, + ) + + assert isinstance(result, ParsedResponse) + assert len(result.output) == 1 + assert result.output[0].type == "message" \ No newline at end of file From 7dc030f8de39bf1b874ebaeb22f85d4ca59294e8 Mon Sep 17 00:00:00 2001 From: C1-BA-B1-F3 Date: Fri, 26 Jun 2026 03:29:38 +0800 Subject: [PATCH 2/4] fix: drain remaining SSE events after [DONE] to prevent TCP FIN After receiving the [DONE] SSE event, Stream.__stream__ and AsyncStream.__stream__ now drain remaining events from the iterator before calling response.close(). This ensures the HTTP/1.1 chunked transfer-encoding terminator (0\r\n\r\n) is consumed, allowing h11's their_state to advance to DONE so httpcore returns the connection to the pool gracefully instead of destroying it with TCP FIN. Regression from 6132922c which removed the drain. Fixes #3440 --- src/openai/_streaming.py | 14 ++++++++++++-- tests/test_streaming.py | 27 +++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/openai/_streaming.py b/src/openai/_streaming.py index 45c13cc11d..2251792ae7 100644 --- a/src/openai/_streaming.py +++ b/src/openai/_streaming.py @@ -106,7 +106,14 @@ def __stream__(self) -> Iterator[_T]: response=response, ) finally: - # Ensure the response is closed even if the consumer doesn't read all data + # Drain remaining events from the iterator so that the underlying + # response.iter_bytes() is fully consumed, including the HTTP/1.1 + # chunked transfer-encoding terminator (0\r\n\r\n). Without this, + # h11's their_state won't advance to DONE, causing httpcore to + # destroy the connection (TCP FIN) instead of returning it to the pool. + # See: https://github.com/openai/openai-python/issues/3440 + for _ in iterator: + pass response.close() def __enter__(self) -> Self: @@ -216,7 +223,10 @@ async def __stream__(self) -> AsyncIterator[_T]: response=response, ) finally: - # Ensure the response is closed even if the consumer doesn't read all data + # Drain remaining events so the chunked terminator is consumed before close. + # See: https://github.com/openai/openai-python/issues/3440 + async for _ in iterator: + pass await response.aclose() async def __aenter__(self) -> Self: diff --git a/tests/test_streaming.py b/tests/test_streaming.py index 04f8e51abd..c4f3399627 100644 --- a/tests/test_streaming.py +++ b/tests/test_streaming.py @@ -246,3 +246,30 @@ def make_event_iterator( return AsyncStream( cast_to=object, client=async_client, response=httpx.Response(200, content=to_aiter(content)) )._iter_events() + + +@pytest.mark.asyncio +@pytest.mark.parametrize("sync", [True, False], ids=["sync", "async"]) +async def test_stream_drains_before_close(sync: bool, client: OpenAI, async_client: AsyncOpenAI) -> None: + """Regression test for https://github.com/openai/openai-python/issues/3440 + + After [DONE], the response stream should be fully drained (including the + chunked transfer-encoding terminator) before close() is called. + """ + chunks = [ + b'data: {"choices":[{"delta":{"content":"hi"}}]}\n\n', + b'data: [DONE]\n\n', + ] + + if sync: + response = httpx.Response(200, content=iter(chunks)) + stream = Stream(cast_to=object, client=client, response=response) + for _ in stream: + pass + assert response.is_stream_consumed + else: + response = httpx.Response(200, content=to_aiter(iter(chunks))) + stream = AsyncStream(cast_to=object, client=async_client, response=response) + async for _ in stream: + pass + assert response.is_stream_consumed From 4e0b2a1df117fc765a2646cc7f1e0785c0235393 Mon Sep 17 00:00:00 2001 From: C1-BA-B1-F3 Date: Fri, 26 Jun 2026 04:57:05 +0800 Subject: [PATCH 3/4] fix: use Annotation type for ResponseOutputTextAnnotationAddedEvent.annotation The annotation field was typed as 'object' instead of the proper 'Annotation' union type (AnnotationFileCitation | AnnotationURLCitation | AnnotationContainerFileCitation | AnnotationFilePath). This forced consumers to use typing.cast to access typed fields. Fixes #3419 Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: Sisyphus --- ...onse_output_text_annotation_added_event.py | 3 +- tests/test_annotation_event.py | 143 ++++++++++++++++++ 2 files changed, 145 insertions(+), 1 deletion(-) create mode 100644 tests/test_annotation_event.py diff --git a/src/openai/types/responses/response_output_text_annotation_added_event.py b/src/openai/types/responses/response_output_text_annotation_added_event.py index b9dc262150..a2f256cb97 100644 --- a/src/openai/types/responses/response_output_text_annotation_added_event.py +++ b/src/openai/types/responses/response_output_text_annotation_added_event.py @@ -3,6 +3,7 @@ from typing_extensions import Literal from ..._models import BaseModel +from .response_output_text import Annotation __all__ = ["ResponseOutputTextAnnotationAddedEvent"] @@ -10,7 +11,7 @@ class ResponseOutputTextAnnotationAddedEvent(BaseModel): """Emitted when an annotation is added to output text content.""" - annotation: object + annotation: Annotation """The annotation object being added. (See annotation schema for details.)""" annotation_index: int diff --git a/tests/test_annotation_event.py b/tests/test_annotation_event.py new file mode 100644 index 0000000000..a2084f9d93 --- /dev/null +++ b/tests/test_annotation_event.py @@ -0,0 +1,143 @@ +# Tests for https://github.com/openai/openai-python/issues/3419 +# ResponseOutputTextAnnotationAddedEvent.annotation should be typed as Annotation, not object + +from openai._models import construct_type +from openai.types.responses.response_output_text_annotation_added_event import ResponseOutputTextAnnotationAddedEvent +from openai.types.responses.response_output_text import ( + Annotation, + AnnotationFileCitation, + AnnotationURLCitation, + AnnotationContainerFileCitation, + AnnotationFilePath, +) + + +def test_annotation_url_citation() -> None: + """Test that url_citation annotation is properly typed and parsed.""" + event = ResponseOutputTextAnnotationAddedEvent( + annotation={ + "type": "url_citation", + "title": "Example", + "url": "https://example.com", + "start_index": 0, + "end_index": 10, + }, + annotation_index=0, + content_index=0, + item_id="item_123", + output_index=0, + sequence_number=1, + type="response.output_text.annotation.added", + ) + + # After the fix, annotation should be an AnnotationURLCitation, not plain object + assert isinstance(event.annotation, AnnotationURLCitation) + assert event.annotation.type == "url_citation" + assert event.annotation.title == "Example" + assert event.annotation.url == "https://example.com" + assert event.annotation.start_index == 0 + assert event.annotation.end_index == 10 + + +def test_annotation_file_citation() -> None: + """Test that file_citation annotation is properly typed and parsed.""" + event = ResponseOutputTextAnnotationAddedEvent( + annotation={ + "type": "file_citation", + "file_id": "file_abc123", + "filename": "document.pdf", + "index": 0, + }, + annotation_index=0, + content_index=0, + item_id="item_123", + output_index=0, + sequence_number=1, + type="response.output_text.annotation.added", + ) + + assert isinstance(event.annotation, AnnotationFileCitation) + assert event.annotation.type == "file_citation" + assert event.annotation.file_id == "file_abc123" + assert event.annotation.filename == "document.pdf" + assert event.annotation.index == 0 + + +def test_annotation_container_file_citation() -> None: + """Test that container_file_citation annotation is properly typed and parsed.""" + event = ResponseOutputTextAnnotationAddedEvent( + annotation={ + "type": "container_file_citation", + "container_id": "container_abc", + "file_id": "file_abc123", + "filename": "document.pdf", + "start_index": 0, + "end_index": 10, + }, + annotation_index=0, + content_index=0, + item_id="item_123", + output_index=0, + sequence_number=1, + type="response.output_text.annotation.added", + ) + + assert isinstance(event.annotation, AnnotationContainerFileCitation) + assert event.annotation.type == "container_file_citation" + assert event.annotation.container_id == "container_abc" + assert event.annotation.file_id == "file_abc123" + assert event.annotation.filename == "document.pdf" + assert event.annotation.start_index == 0 + assert event.annotation.end_index == 10 + + +def test_annotation_file_path() -> None: + """Test that file_path annotation is properly typed and parsed.""" + event = ResponseOutputTextAnnotationAddedEvent( + annotation={ + "type": "file_path", + "file_id": "file_abc123", + "index": 0, + }, + annotation_index=0, + content_index=0, + item_id="item_123", + output_index=0, + sequence_number=1, + type="response.output_text.annotation.added", + ) + + assert isinstance(event.annotation, AnnotationFilePath) + assert event.annotation.type == "file_path" + assert event.annotation.file_id == "file_abc123" + assert event.annotation.index == 0 + + +def test_annotation_type_is_union() -> None: + """Test that the annotation field accepts all annotation types.""" + # Verify the field is typed as Annotation (Union of all annotation types), not object + # We check by inspecting the model's field info directly + field_info = ResponseOutputTextAnnotationAddedEvent.model_fields["annotation"] + annotation_type = field_info.annotation + + # The Annotation type is an Annotated[Union[...], ...] + # We can verify it's not just 'object' + assert annotation_type is not object + + +def test_construct_type_with_annotation() -> None: + """Test that construct_type works with the Annotation type for streaming events.""" + # This simulates how streaming events are parsed + annotation_data = { + "type": "url_citation", + "title": "Test", + "url": "https://test.com", + "start_index": 5, + "end_index": 15, + } + + # construct_type should be able to parse this as an Annotation + annotation = construct_type(value=annotation_data, type_=Annotation) + assert isinstance(annotation, AnnotationURLCitation) + assert annotation.title == "Test" + assert annotation.url == "https://test.com" From 9073185699b8d38c34d3fb86a0bf7eaf3b603c39 Mon Sep 17 00:00:00 2001 From: C1-BA-B1-F3 Date: Fri, 26 Jun 2026 05:09:07 +0800 Subject: [PATCH 4/4] fix: ensure response.close() runs even if drain loop raises Wrap the drain loop in a nested try/finally in both Stream.__stream__ and AsyncStream.__stream__ so that response.close() / aclose() is always called, even if iter_bytes() raises during drain (e.g. read timeout or protocol error). Without this, a drain failure could leak the response/connection and replace the original streaming exception. Addresses Codex review P2 on #3443. --- src/openai/_streaming.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/openai/_streaming.py b/src/openai/_streaming.py index 2251792ae7..84ee284c29 100644 --- a/src/openai/_streaming.py +++ b/src/openai/_streaming.py @@ -112,9 +112,11 @@ def __stream__(self) -> Iterator[_T]: # h11's their_state won't advance to DONE, causing httpcore to # destroy the connection (TCP FIN) instead of returning it to the pool. # See: https://github.com/openai/openai-python/issues/3440 - for _ in iterator: - pass - response.close() + try: + for _ in iterator: + pass + finally: + response.close() def __enter__(self) -> Self: return self @@ -225,9 +227,11 @@ async def __stream__(self) -> AsyncIterator[_T]: finally: # Drain remaining events so the chunked terminator is consumed before close. # See: https://github.com/openai/openai-python/issues/3440 - async for _ in iterator: - pass - await response.aclose() + try: + async for _ in iterator: + pass + finally: + await response.aclose() async def __aenter__(self) -> Self: return self