From 76bd1aaaf6892030f3b159455954421b87695820 Mon Sep 17 00:00:00 2001 From: alex temnorod Date: Sun, 23 Aug 2026 12:15:50 -0400 Subject: [PATCH 1/2] exception creation, new testing, raising exception in retrieve --- src/openai/lib/__init__.py | 5 + src/openai/lib/_responses.py | 126 +++++++++++ src/openai/resources/responses/responses.py | 156 +++++++++---- tests/lib/test_responses_background_errors.py | 206 ++++++++++++++++++ 4 files changed, 455 insertions(+), 38 deletions(-) create mode 100644 src/openai/lib/_responses.py create mode 100644 tests/lib/test_responses_background_errors.py diff --git a/src/openai/lib/__init__.py b/src/openai/lib/__init__.py index 5c6cb782c0..23666c3890 100644 --- a/src/openai/lib/__init__.py +++ b/src/openai/lib/__init__.py @@ -1,2 +1,7 @@ from ._tools import pydantic_function_tool as pydantic_function_tool from ._parsing import ResponseFormatT as ResponseFormatT +from ._responses import ( + RESPONSE_ERROR_CODE_TO_EXCEPTION as RESPONSE_ERROR_CODE_TO_EXCEPTION, + INCOMPLETE_DETAILS_REASON_TO_EXCEPTION as INCOMPLETE_DETAILS_REASON_TO_EXCEPTION, + exception_for_background_failure as exception_for_background_failure, +) diff --git a/src/openai/lib/_responses.py b/src/openai/lib/_responses.py new file mode 100644 index 0000000000..a7fa1671a7 --- /dev/null +++ b/src/openai/lib/_responses.py @@ -0,0 +1,126 @@ +from __future__ import annotations + +from typing import Dict, Type, Optional, NamedTuple + +from .._exceptions import ( + NotFoundError, + APIStatusError, + RateLimitError, + BadRequestError, + InternalServerError, + PermissionDeniedError, +) +from .._legacy_response import LegacyAPIResponse +from ..types.responses.response import Response + +__all__ = [ + "RESPONSE_ERROR_CODE_TO_EXCEPTION", + "INCOMPLETE_DETAILS_REASON_TO_EXCEPTION", + "exception_for_background_failure", +] + + +class _ErrorMapping(NamedTuple): + """One entry in a background-failure mapping table. + + Bundles the SDK exception class (``None`` means "no exception should be + raised for this code/reason") together with whether retrying the same + request is expected to help, so the two facts can't drift apart. + """ + + exception_class: Optional[Type[APIStatusError]] + retryable: bool + + +# `ResponseError.code` -> SDK exception. This is the entire signal a failed +# background run gives us (see `ResponseError` in +# `openai/types/responses/response_error.py`) -- there's no HTTP status to +# dispatch on, since the poll that surfaces this returns 200 OK. +# +# Unmapped codes (a value OpenAI ships before this table is updated) fall back +# to InternalServerError(retryable=True) in `exception_for_background_failure` +# -- an explicit "failed" status must never look like success. +RESPONSE_ERROR_CODE_TO_EXCEPTION: Dict[str, _ErrorMapping] = { + "server_error": _ErrorMapping(InternalServerError, True), + "rate_limit_exceeded": _ErrorMapping(RateLimitError, True), + "vector_store_timeout": _ErrorMapping(InternalServerError, True), + "data_residency_mismatch": _ErrorMapping(PermissionDeniedError, False), + "invalid_prompt": _ErrorMapping(BadRequestError, False), + "bio_policy": _ErrorMapping(BadRequestError, False), + "invalid_image": _ErrorMapping(BadRequestError, False), + "invalid_image_format": _ErrorMapping(BadRequestError, False), + "invalid_base64_image": _ErrorMapping(BadRequestError, False), + "invalid_image_url": _ErrorMapping(BadRequestError, False), + "image_too_large": _ErrorMapping(BadRequestError, False), + "image_too_small": _ErrorMapping(BadRequestError, False), + "image_parse_error": _ErrorMapping(BadRequestError, False), + "image_content_policy_violation": _ErrorMapping(BadRequestError, False), + "invalid_image_mode": _ErrorMapping(BadRequestError, False), + "image_file_too_large": _ErrorMapping(BadRequestError, False), + "unsupported_image_media_type": _ErrorMapping(BadRequestError, False), + "empty_image_file": _ErrorMapping(BadRequestError, False), + "failed_to_download_image": _ErrorMapping(BadRequestError, False), + "image_file_not_found": _ErrorMapping(NotFoundError, False), +} + +# `IncompleteDetails.reason` -> SDK exception. Separate table from the one +# above: `status="incomplete"` is not automatically a failure (a run that hit +# `max_output_tokens` produced a structurally valid, complete-so-far result), +# so unlike `RESPONSE_ERROR_CODE_TO_EXCEPTION` there is no "unmapped -> +# fall back to an exception" behavior here -- an unrecognized or absent +# reason simply means we return None (don't raise). +INCOMPLETE_DETAILS_REASON_TO_EXCEPTION: Dict[str, _ErrorMapping] = { + "max_output_tokens": _ErrorMapping(None, False), + "content_filter": _ErrorMapping(BadRequestError, False), +} + +_UNMAPPED_FAILURE = _ErrorMapping(InternalServerError, True) + + +def exception_for_background_failure( + raw_response: LegacyAPIResponse[Response], + response: Response, +) -> Optional[APIStatusError]: + """Return the exception a failed/incomplete background Response should raise, or None. + + Background runs finish with a normal HTTP 200 even when the run itself + failed, so the SDK's usual status-code-based error handling never + triggers. `Responses.retrieve()` calls this once a run is no longer + queued/in-progress, to translate `error.code` (or, for status= + "incomplete", `incomplete_details.reason`) into the same exception class + a synchronous request with an equivalent failure would have raised. + + The raised exception also carries a best-effort `.retryable` attribute + (not declared on the base exception classes -- read it with + `getattr(exc, "retryable", False)`). `RESPONSE_ERROR_CODE_TO_EXCEPTION` + and `INCOMPLETE_DETAILS_REASON_TO_EXCEPTION` are the documented source of + truth for this flag. + + Returns None if the response didn't fail, or "failed"/"incomplete" but + without enough information to classify (e.g. no `error`/ + `incomplete_details` populated). + """ + mapping: Optional[_ErrorMapping] = None + message = "Background response run did not complete successfully." + + if response.status == "failed": + if response.error is None: + return None + message = response.error.message + mapping = RESPONSE_ERROR_CODE_TO_EXCEPTION.get(response.error.code, _UNMAPPED_FAILURE) + elif response.status == "incomplete": + reason = response.incomplete_details.reason if response.incomplete_details else None + if reason is None: + return None + mapping = INCOMPLETE_DETAILS_REASON_TO_EXCEPTION.get(reason) + message = f"Background response run was incomplete: {reason}" + else: + return None + + if mapping is None or mapping.exception_class is None: + return None + + body = {"error": {"code": getattr(response.error, "code", None), "message": message}} + exc = mapping.exception_class(message, response=raw_response.http_response, body=body) + exc.retryable = mapping.retryable # type: ignore[attr-defined] + return exc diff --git a/src/openai/resources/responses/responses.py b/src/openai/resources/responses/responses.py index 2232b1a05d..089a598d91 100644 --- a/src/openai/resources/responses/responses.py +++ b/src/openai/resources/responses/responses.py @@ -44,6 +44,7 @@ InputItemsWithStreamingResponse, AsyncInputItemsWithStreamingResponse, ) +from ..._constants import RAW_RESPONSE_HEADER from ..._streaming import Stream, AsyncStream from ...lib._tools import PydanticFunctionTool, ResponsesPydanticFunctionTool from .input_tokens import ( @@ -58,6 +59,7 @@ from ..._send_queue import SendQueue from ..._base_client import _merge_mappings, make_request_options from ..._event_handler import EventHandlerRegistry +from ...lib._responses import exception_for_background_failure from ...types.responses import ( response_create_params, response_compact_params, @@ -1624,28 +1626,67 @@ def retrieve( ) -> Response | Stream[ResponseStreamEvent]: if not response_id: raise ValueError(f"Expected a non-empty value for `response_id` but received {response_id!r}") - return self._get( - path_template("/responses/{response_id}", response_id=response_id), - options=make_request_options( - extra_headers=extra_headers, - extra_query=extra_query, - extra_body=extra_body, - timeout=timeout, - query=maybe_transform( - { - "include": include, - "include_obfuscation": include_obfuscation, - "starting_after": starting_after, - "stream": stream, - }, - response_retrieve_params.ResponseRetrieveParams, + + path = path_template("/responses/{response_id}", response_id=response_id) + query = maybe_transform( + { + "include": include, + "include_obfuscation": include_obfuscation, + "starting_after": starting_after, + "stream": stream, + }, + response_retrieve_params.ResponseRetrieveParams, + ) + + if stream or (extra_headers is not None and RAW_RESPONSE_HEADER in extra_headers): + # Preserve the original behavior untouched for: (a) real SSE streaming + # requests, and (b) calls already going through one of the SDK's own + # raw/streamed-response wrappers (with_raw_response, + # with_streaming_response) -- those inject RAW_RESPONSE_HEADER + # themselves and need `_get` to hand back their own wrapper type, not + # our parsed-and-possibly-raising Response below. + return self._get( + path, + options=make_request_options( + extra_headers=extra_headers, + extra_query=extra_query, + extra_body=extra_body, + timeout=timeout, + query=query, + security={"bearer_auth": True}, ), - security={"bearer_auth": True}, + cast_to=Response, + stream=stream or False, + stream_cls=Stream[ResponseStreamEvent], + ) + + # Ask for the raw HTTP response (same request, no extra round trip) so a + # failed/incomplete background run can be turned into a typed exception + # below -- such a run still comes back as a plain HTTP 200. + raw_response = cast( + "_legacy_response.LegacyAPIResponse[Response]", + self._get( + path, + options=make_request_options( + extra_headers={RAW_RESPONSE_HEADER: "true", **(extra_headers or {})}, + extra_query=extra_query, + extra_body=extra_body, + timeout=timeout, + query=query, + security={"bearer_auth": True}, + ), + cast_to=Response, + stream=False, ), - cast_to=Response, - stream=stream or False, - stream_cls=Stream[ResponseStreamEvent], ) + response = raw_response.parse() + + if response.status not in ("queued", "in_progress", "completed"): + exc = exception_for_background_failure(raw_response, response) + if exc is not None: + raise exc + + return response def delete( self, @@ -3491,28 +3532,67 @@ async def retrieve( ) -> Response | AsyncStream[ResponseStreamEvent]: if not response_id: raise ValueError(f"Expected a non-empty value for `response_id` but received {response_id!r}") - return await self._get( - path_template("/responses/{response_id}", response_id=response_id), - options=make_request_options( - extra_headers=extra_headers, - extra_query=extra_query, - extra_body=extra_body, - timeout=timeout, - query=await async_maybe_transform( - { - "include": include, - "include_obfuscation": include_obfuscation, - "starting_after": starting_after, - "stream": stream, - }, - response_retrieve_params.ResponseRetrieveParams, + + path = path_template("/responses/{response_id}", response_id=response_id) + query = await async_maybe_transform( + { + "include": include, + "include_obfuscation": include_obfuscation, + "starting_after": starting_after, + "stream": stream, + }, + response_retrieve_params.ResponseRetrieveParams, + ) + + if stream or (extra_headers is not None and RAW_RESPONSE_HEADER in extra_headers): + # Preserve the original behavior untouched for: (a) real SSE streaming + # requests, and (b) calls already going through one of the SDK's own + # raw/streamed-response wrappers (with_raw_response, + # with_streaming_response) -- those inject RAW_RESPONSE_HEADER + # themselves and need `_get` to hand back their own wrapper type, not + # our parsed-and-possibly-raising Response below. + return await self._get( + path, + options=make_request_options( + extra_headers=extra_headers, + extra_query=extra_query, + extra_body=extra_body, + timeout=timeout, + query=query, + security={"bearer_auth": True}, ), - security={"bearer_auth": True}, + cast_to=Response, + stream=stream or False, + stream_cls=AsyncStream[ResponseStreamEvent], + ) + + # Ask for the raw HTTP response (same request, no extra round trip) so a + # failed/incomplete background run can be turned into a typed exception + # below -- such a run still comes back as a plain HTTP 200. + raw_response = cast( + "_legacy_response.LegacyAPIResponse[Response]", + await self._get( + path, + options=make_request_options( + extra_headers={RAW_RESPONSE_HEADER: "true", **(extra_headers or {})}, + extra_query=extra_query, + extra_body=extra_body, + timeout=timeout, + query=query, + security={"bearer_auth": True}, + ), + cast_to=Response, + stream=False, ), - cast_to=Response, - stream=stream or False, - stream_cls=AsyncStream[ResponseStreamEvent], ) + response = raw_response.parse() + + if response.status not in ("queued", "in_progress", "completed"): + exc = exception_for_background_failure(raw_response, response) + if exc is not None: + raise exc + + return response async def delete( self, diff --git a/tests/lib/test_responses_background_errors.py b/tests/lib/test_responses_background_errors.py new file mode 100644 index 0000000000..d2fc59eec8 --- /dev/null +++ b/tests/lib/test_responses_background_errors.py @@ -0,0 +1,206 @@ +from __future__ import annotations + +from typing import Any, cast +from unittest import mock +from typing_extensions import TypeAlias + +import httpx2 +import pytest + +from openai import BadRequestError, InternalServerError +from openai._models import construct_type_unchecked +from openai.lib._responses import ( + RESPONSE_ERROR_CODE_TO_EXCEPTION, + INCOMPLETE_DETAILS_REASON_TO_EXCEPTION, + exception_for_background_failure, +) +from openai.types.responses.response import Response +from openai.resources.responses.responses import Responses, AsyncResponses + +ResponsesResource: TypeAlias = Responses | AsyncResponses +RESPONSE_ID = "resp-synthetic" + +ALL_ERROR_CODES = sorted(RESPONSE_ERROR_CODE_TO_EXCEPTION) +ALL_INCOMPLETE_REASONS = sorted(INCOMPLETE_DETAILS_REASON_TO_EXCEPTION) + + +@pytest.fixture(params=[Responses, AsyncResponses]) +def resource(request: pytest.FixtureRequest) -> ResponsesResource: + resource_type = cast("type[ResponsesResource]", request.param) + return resource_type(cast(Any, mock.Mock())) + + +def make_response( + status: str, + *, + error: dict[str, Any] | None = None, + incomplete_details: dict[str, Any] | None = None, +) -> Response: + return construct_type_unchecked( + type_=Response, + value={"id": RESPONSE_ID, "status": status, "error": error, "incomplete_details": incomplete_details}, + ) + + +def raw_response(*args: Any, **kwargs: Any) -> mock.Mock: + http_response = httpx2.Response( + 200, request=httpx2.Request("GET", f"https://api.openai.com/v1/responses/{RESPONSE_ID}") + ) + return mock.Mock(http_response=http_response, parse=mock.Mock(return_value=make_response(*args, **kwargs))) + + +def patch_get(resource: ResponsesResource, result: mock.Mock) -> mock._patch[Any]: + if isinstance(resource, AsyncResponses): + return mock.patch.object(resource, "_get", new=mock.AsyncMock(return_value=result)) + return mock.patch.object(resource, "_get", return_value=result) + + +async def retrieve(resource: ResponsesResource, **kwargs: Any) -> Response: + if isinstance(resource, AsyncResponses): + return cast(Response, await resource.retrieve(RESPONSE_ID, **kwargs)) + return cast(Response, resource.retrieve(RESPONSE_ID, **kwargs)) + + +def test_every_error_code_is_covered() -> None: + assert set(RESPONSE_ERROR_CODE_TO_EXCEPTION) == { + "server_error", + "rate_limit_exceeded", + "invalid_prompt", + "data_residency_mismatch", + "bio_policy", + "vector_store_timeout", + "invalid_image", + "invalid_image_format", + "invalid_base64_image", + "invalid_image_url", + "image_too_large", + "image_too_small", + "image_parse_error", + "image_content_policy_violation", + "invalid_image_mode", + "image_file_too_large", + "unsupported_image_media_type", + "empty_image_file", + "failed_to_download_image", + "image_file_not_found", + } + + +def test_every_incomplete_reason_is_covered() -> None: + assert set(INCOMPLETE_DETAILS_REASON_TO_EXCEPTION) == {"max_output_tokens", "content_filter"} + + +@pytest.mark.parametrize("code", ALL_ERROR_CODES) +def test_every_mapped_error_code_raises_its_class(code: str) -> None: + mapping = RESPONSE_ERROR_CODE_TO_EXCEPTION[code] + response = make_response("failed", error={"code": code, "message": f"synthetic failure for {code}"}) + + exc = exception_for_background_failure(raw_response("failed"), response) + + assert exc is not None + assert type(exc) is mapping.exception_class + assert getattr(exc, "retryable", None) is mapping.retryable + assert exc.message == f"synthetic failure for {code}" + + +@pytest.mark.parametrize("reason", ALL_INCOMPLETE_REASONS) +def test_every_mapped_incomplete_reason(reason: str) -> None: + mapping = INCOMPLETE_DETAILS_REASON_TO_EXCEPTION[reason] + response = make_response("incomplete", incomplete_details={"reason": reason}) + + exc = exception_for_background_failure(raw_response("incomplete"), response) + + if mapping.exception_class is None: + assert exc is None + else: + assert exc is not None + assert type(exc) is mapping.exception_class + assert getattr(exc, "retryable", None) is mapping.retryable + + +def test_unmapped_failure_code_falls_back_to_internal_server_error() -> None: + response = make_response("failed", error={"code": "a_future_code_the_sdk_does_not_know_yet", "message": "mystery"}) + + exc = exception_for_background_failure(raw_response("failed"), response) + + assert isinstance(exc, InternalServerError) + assert getattr(exc, "retryable", None) is True + + +def test_unmapped_incomplete_reason_does_not_raise() -> None: + response = make_response("incomplete", incomplete_details={"reason": "a_future_reason_the_sdk_does_not_know_yet"}) + + assert exception_for_background_failure(raw_response("incomplete"), response) is None + + +def test_failed_without_error_object_does_not_raise() -> None: + response = make_response("failed", error=None) + + assert exception_for_background_failure(raw_response("failed"), response) is None + + +def test_incomplete_without_details_does_not_raise() -> None: + response = make_response("incomplete", incomplete_details=None) + + assert exception_for_background_failure(raw_response("incomplete"), response) is None + + +def test_incomplete_with_empty_reason_does_not_raise() -> None: + response = make_response("incomplete", incomplete_details={"reason": None}) + + assert exception_for_background_failure(raw_response("incomplete"), response) is None + + +@pytest.mark.parametrize("status", ["completed", "cancelled", "queued", "in_progress"]) +def test_non_failure_statuses_do_not_raise(status: str) -> None: + assert exception_for_background_failure(raw_response(status), make_response(status)) is None + + +# --------------------------------------------------------------------------- +# Responses.retrieve() / AsyncResponses.retrieve() wiring +# --------------------------------------------------------------------------- + + +async def test_completed_run_returns_normally(resource: ResponsesResource) -> None: + terminal = raw_response("completed") + with patch_get(resource, terminal): + assert await retrieve(resource) is terminal.parse.return_value + + +async def test_cancelled_run_returns_normally(resource: ResponsesResource) -> None: + terminal = raw_response("cancelled") + with patch_get(resource, terminal): + assert await retrieve(resource) is terminal.parse.return_value + + +async def test_incomplete_max_output_tokens_returns_normally(resource: ResponsesResource) -> None: + terminal = raw_response("incomplete", incomplete_details={"reason": "max_output_tokens"}) + with patch_get(resource, terminal): + assert await retrieve(resource) is terminal.parse.return_value + + +async def test_incomplete_content_filter_raises(resource: ResponsesResource) -> None: + terminal = raw_response("incomplete", incomplete_details={"reason": "content_filter"}) + with patch_get(resource, terminal): + with pytest.raises(BadRequestError): + await retrieve(resource) + + +@pytest.mark.parametrize("code", ALL_ERROR_CODES) +async def test_failed_run_raises_mapped_exception(resource: ResponsesResource, code: str) -> None: + mapping = RESPONSE_ERROR_CODE_TO_EXCEPTION[code] + assert mapping.exception_class is not None # every entry in this table maps to a real exception + terminal = raw_response("failed", error={"code": code, "message": f"synthetic failure for {code}"}) + + with patch_get(resource, terminal): + with pytest.raises(mapping.exception_class): + await retrieve(resource) + + +async def test_failed_run_with_unmapped_code_raises_internal_server_error(resource: ResponsesResource) -> None: + terminal = raw_response("failed", error={"code": "a_future_code_the_sdk_does_not_know_yet", "message": "mystery"}) + + with patch_get(resource, terminal): + with pytest.raises(InternalServerError) as excinfo: + await retrieve(resource) + assert getattr(excinfo.value, "retryable", None) is True From 4db0f14451a61f10da187c48cbf51e3ddd29ca0f Mon Sep 17 00:00:00 2001 From: alex temnorod Date: Sun, 23 Aug 2026 15:53:40 -0400 Subject: [PATCH 2/2] addressed comments --- src/openai/lib/_responses.py | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/src/openai/lib/_responses.py b/src/openai/lib/_responses.py index a7fa1671a7..1f99a51118 100644 --- a/src/openai/lib/_responses.py +++ b/src/openai/lib/_responses.py @@ -24,8 +24,15 @@ class _ErrorMapping(NamedTuple): """One entry in a background-failure mapping table. Bundles the SDK exception class (``None`` means "no exception should be - raised for this code/reason") together with whether retrying the same - request is expected to help, so the two facts can't drift apart. + raised for this code/reason") together with ``retryable``, so the two + facts can't drift apart. + + ``retryable`` describes the *original* background run, i.e. whether + submitting a fresh `create(..., background=True)` call is worth trying. + It does NOT mean calling `retrieve()` again on this same response id will + help -- a response that has reached a terminal "failed"/"incomplete" + status is immutable, so polling it again always returns the identical + failure. """ exception_class: Optional[Type[APIStatusError]] @@ -94,7 +101,10 @@ def exception_for_background_failure( (not declared on the base exception classes -- read it with `getattr(exc, "retryable", False)`). `RESPONSE_ERROR_CODE_TO_EXCEPTION` and `INCOMPLETE_DETAILS_REASON_TO_EXCEPTION` are the documented source of - truth for this flag. + truth for this flag. `retryable` means "submitting a new background run + is worth trying" -- calling `retrieve()` again on this same response id + will never help, since a terminal "failed"/"incomplete" response is + immutable. Returns None if the response didn't fail, or "failed"/"incomplete" but without enough information to classify (e.g. no `error`/ @@ -120,7 +130,8 @@ def exception_for_background_failure( if mapping is None or mapping.exception_class is None: return None - body = {"error": {"code": getattr(response.error, "code", None), "message": message}} + body = {"code": getattr(response.error, "code", None), "message": message} exc = mapping.exception_class(message, response=raw_response.http_response, body=body) + # retryable == "resubmit as a new background run", not "call retrieve() again" exc.retryable = mapping.retryable # type: ignore[attr-defined] return exc