Skip to content

fix: only retry transport-level exceptions, not arbitrary ones - #3804

Open
Bhumika-1432006 wants to merge 1 commit into
openai:mainfrom
Bhumika-1432006:fix/retry-only-transport-exceptions
Open

fix: only retry transport-level exceptions, not arbitrary ones#3804
Bhumika-1432006 wants to merge 1 commit into
openai:mainfrom
Bhumika-1432006:fix/retry-only-transport-exceptions

Conversation

@Bhumika-1432006

Copy link
Copy Markdown

Summary

The request retry loop in _base_client.py catches bare Exception and treats it exactly like a connection failure: it retries (if retries remain) and otherwise wraps it in APIConnectionError. That means any exception raised while a request is in flight — even one that has nothing to do with the HTTP request — gets silently swallowed and retried.

Concretely, this breaks graceful shutdown for callers that run the client inside a Celery task with a soft time limit: Celery's SoftTimeLimitExceeded is a plain Exception subclass, so it gets caught here, retried, and the task's cleanup/shutdown logic that depends on that exception propagating never runs.

Fixes #2737.

Root cause

except OpenAIError as err:
    raise err
except Exception as err:              # <-- too broad
    if remaining_retries > 0:
        self._sleep_for_retry(...)
        continue
    raise APIConnectionError(request=request) from err

Both the sync (SyncAPIClient._request) and async (AsyncAPIClient._request) retry loops had this issue.

Fix

  • Added request_exceptions() to src/openai/_httpx2.py, following the same pattern as the existing timeout_exceptions() / status_exceptions() helpers: it returns (httpx2.RequestError,), plus the legacy httpx.RequestError when a legacy httpx.AsyncClient/Client has been injected (per the documented escape hatch).
  • httpx2.RequestError (mirroring httpx's hierarchy) is the base class for all genuine transport-level failures — connection errors, protocol errors, proxy errors, timeouts, etc. — so this is the right level to retry on.
  • Replaced both except Exception as err: blocks with except request_exceptions() as err:.

Unrelated exceptions (e.g. SoftTimeLimitExceeded, or any other exception raised by code running underneath the transport) now propagate immediately, unmodified, and without being retried or wrapped in APIConnectionError.

Tests

  • tests/test_client.py: the existing test_retries_taken[exception-*] parametrization (sync + async) simulated a retryable exception with a bare RuntimeError(...). Since that's no longer retried under this fix, I changed it to httpx2.ConnectError(...) — a genuine transport error — so it still exercises the "retry on exception" path meaningfully.
  • Added new regression tests (sync + async): test_non_transport_exceptions_are_not_retried, which raises a plain RuntimeError from the mocked transport and asserts it (a) is not retried (the mock is only called once) and (b) propagates as the original RuntimeError, not wrapped in APIConnectionError.

All of tests/test_client.py passes locally (198 passed, 2 pre-existing/unrelated failures in test_proxy_environment_variables that reproduce identically on main without this change, 2 skipped). ruff check, ruff format --check, and mypy are clean on the changed files.

Notes for reviewers

  • This only narrows the retry-on-exception branch. The except OpenAIError (re-raise) and except timeout_exceptions() (dedicated APITimeoutError path) branches above it are untouched.
  • I deliberately didn't touch the separate empty-except Exception: blocks used for best-effort cleanup elsewhere in _base_client.py (e.g. around client teardown) — those are a distinct issue (Empty exception handler in _base_client.py #3428) with a different fix shape (logging rather than narrowing).

The request retry loop in _base_client.py caught bare `Exception`,
which meant any error raised while a request was in flight -
including ones with nothing to do with the HTTP request itself -
was treated as a retryable connection error and eventually wrapped
in APIConnectionError.

In particular, running the client inside a Celery task with a soft
time limit causes Celery's SoftTimeLimitExceeded (a plain Exception
subclass) to be swallowed by this handler and retried instead of
propagating, so task cleanup/shutdown logic relying on it never runs
(openai#2737).

Add request_exceptions() alongside the existing timeout_exceptions()/
status_exceptions() helpers in _httpx2.py, and use it to narrow the
retry-on-exception branch to httpx2.RequestError (and the legacy
httpx.RequestError, for users who inject a legacy AsyncClient) -
covering connection failures, protocol errors, and other genuine
transport errors, while letting unrelated exceptions propagate
immediately and unmodified.

Fixes openai#2737
@Bhumika-1432006
Bhumika-1432006 requested a review from a team as a code owner September 5, 2026 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenAI client intercepts Celery SoftTimeLimitExceeded exception if it happens during API call

1 participant