fix(client): narrow retry exception handling to transport errors#2956
fix(client): narrow retry exception handling to transport errors#2956mvanhorn wants to merge 2 commits intoopenai:mainfrom
Conversation
The retry loop previously caught `Exception`, which inadvertently intercepted application-level exceptions like Celery's `SoftTimeLimitExceeded`. Narrow the catch to `httpx.TransportError` and `OSError`, which covers all network/transport failures while allowing non-network exceptions to propagate immediately. Fixes openai#2737 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 996810d560
ℹ️ 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".
src/openai/_base_client.py
Outdated
| raise APITimeoutError(request=request) from err | ||
| except Exception as err: | ||
| log.debug("Encountered Exception", exc_info=True) | ||
| except (httpx.TransportError, OSError) as err: |
There was a problem hiding this comment.
Narrowing this handler to httpx.TransportError/OSError lets other httpx.RequestError subclasses (for example TooManyRedirects when redirects loop, or DecodingError on malformed encoded bodies) escape as raw httpx exceptions from request() instead of being normalized to APIConnectionError. That is a behavior regression from the previous code path and can break callers that rely on the documented openai.APIError hierarchy for SDK failures.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch - widened to httpx.RequestError in c90c5cd which is the parent class of TransportError and also catches TooManyRedirects, DecodingError, and any other request-phase subclasses. Still narrower than the original except Exception but no longer misses legitimate httpx errors.
…errors httpx.TransportError missed TooManyRedirects and DecodingError which are also httpx.RequestError subclasses. Using RequestError catches all request-phase failures while still excluding unrelated exceptions. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 participant
Summary
except Exceptionin both sync and async retry loops toexcept (httpx.TransportError, OSError), so only network/transport failures trigger retriesSoftTimeLimitExceeded) were caught and retried instead of propagatingFixes #2737
Details
The retry logic in
SyncAPIClient._retry_requestandAsyncAPIClient._retry_requestpreviously caught bareException. This meant any non-APITimeoutErrorexception - including framework signals like Celery'sSoftTimeLimitExceededorKeyboardInterruptsubclasses - would be caught and retried up tomax_retriestimes before re-raising.httpx.TransportErroris the base class for all httpx transport-level errors (connection failures, read errors, etc.), andOSErrorcovers low-level socket errors that may not be wrapped by httpx. Together they cover the full set of retriable network failures without catching application-level exceptions.Test plan
tests/test_client.pyretry-related tests)httpx.TimeoutExceptionis a subclass ofhttpx.TransportError(already handled separately asAPITimeoutError)SoftTimeLimitExceeded(and similar non-network exceptions) are NOT subclasses ofTransportErrororOSErrorThis contribution was developed with AI assistance (Claude Code).