diff --git a/src/openai/_streaming.py b/src/openai/_streaming.py index 45c13cc11d..84ee284c29 100644 --- a/src/openai/_streaming.py +++ b/src/openai/_streaming.py @@ -106,8 +106,17 @@ def __stream__(self) -> Iterator[_T]: response=response, ) finally: - # Ensure the response is closed even if the consumer doesn't read all data - response.close() + # 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 + try: + for _ in iterator: + pass + finally: + response.close() def __enter__(self) -> Self: return self @@ -216,8 +225,13 @@ async def __stream__(self) -> AsyncIterator[_T]: response=response, ) finally: - # Ensure the response is closed even if the consumer doesn't read all data - await response.aclose() + # Drain remaining events so the chunked terminator is consumed before close. + # See: https://github.com/openai/openai-python/issues/3440 + try: + async for _ in iterator: + pass + finally: + await response.aclose() async def __aenter__(self) -> Self: return self 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/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/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 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" 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