fix: wrap transport failures while consuming a stream - #3814
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 033cd40192
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| except transport_exceptions() as err: | ||
| raise APIConnectionError(request=response.request) from err |
There was a problem hiding this comment.
Wrap HTTP decoding failures as connection errors
When a streaming response has malformed or truncated compressed content (for example, an invalid Content-Encoding: gzip body), response.iter_bytes() raises httpx2.DecodingError. That exception is a RequestError, not a TransportError, so this handler—and the identical async handler—still exposes a raw HTTPX2 exception instead of an APIConnectionError; consequently, except openai.APIError continues to miss this stream-consumption failure even though non-streamed responses wrap it. Catch DecodingError as well, or otherwise cover the relevant request-error family.
Useful? React with 👍 / 👎.
Changes being requested
_base_clientwraps failures from the initial send, butStreamandAsyncStreamiterate the response body with nothing around the loop. A read timeout or a dropped connection part-way through a stream therefore surfaces as a rawhttpx2.ReadTimeoutorhttpx2.RemoteProtocolError, soexcept openai.APIErroraround a streaming call misses the most common streaming failure.Both stream classes now catch transport failures raised while the body is consumed and re-raise them as
APITimeoutErrororAPIConnectionError, keeping the original exception as__cause__.transport_exceptions()sits next to the existing compat helpers in_httpx2.py, so a loaded legacyhttpxis covered the same way astimeout_exceptions().Two knock-on changes come with that. The assistants event handler decides whether to fire
on_timeout()from the same exception set, soAPITimeoutErrorwas added there to keep that callback firing on a stream read timeout. Andtest_assistant_stream_timeout_callbacks_preserve_httpx2_familyasserted the rawhttpx2.ReadTimeout; it now asserts the wrapper and checks__cause__, which still pins the httpx2 family.Raw byte streaming through
with_streaming_responseis untouched and still surfaces httpx exceptions directly. Whether a partly consumed stream can be retried is a separate question and is not addressed here.Additional context & links
Closes #3811
Validation:
tests/api_resources,tests/lib/test_fine_tuning_positional_arguments.pyandtests/test_uv_workflows.py, which need the mock server or networktest_transport_error_mid_streamcovers sync and async against both a timeout and a protocol error; all four cases fail on main