Retry transient connection drops when opening a remote inference session#470
Retry transient connection drops when opening a remote inference session#470vertix wants to merge 1 commit into
Conversation
A cold backend can reset TLS or close the handshake instead of timing out, surfacing as `ssl.SSLError`/`ConnectionClosed`. The reconnect loop caught only `TimeoutError`, so a mid-session cold start crashed the whole run. Retry these within the existing deadline.
![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: f554269936
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # A cold backend fails before a session exists in any of three ways: the connect times out, the edge | ||
| # resets TLS (``SSLError``), or it closes the handshake (``ConnectionClosed``). All three mean "not | ||
| # ready yet", so retry them within the deadline instead of letting one kill the run. | ||
| except (TimeoutError, ssl.SSLError, ConnectionClosed) as e: |
There was a problem hiding this comment.
In secure deployments, ssl.SSLCertVerificationError is a subclass of ssl.SSLError, so an expired/invalid certificate, wrong TLS endpoint, or similar permanent TLS misconfiguration now gets treated as a cold-start transient and the caller sleeps/retries until the 900s default connect_deadline before receiving a misleading TimeoutError. These should still fail immediately (or be filtered out of the retried SSL errors) so bad TLS configuration is surfaced promptly.
Useful? React with 👍 / 👎.
1 participant
Summary
InferenceClient.new_sessionretried onlyTimeoutErroron connect, but a cold backend (e.g. a Modal container that scaled down between rollouts) can reset TLS or close the WebSocket handshake instead of timing out — surfacing asssl.SSLError/websockets.ConnectionClosed. Those slipped past the retry loop and propagated up through the harness, killing an otherwise-healthy multi-rollout run on the first cold reconnect.This widens the retried set to those two transient failure modes, so they back off and retry within the existing
connect_deadline(like a cold-start timeout) instead of crashing the run. Genuine connection failures (otherOSErrors) still surface immediately as before.Test plan
new_sessionhit a cold Modal backend and died withssl.SSLError: [SSL] internal error→ConnectionClosedError; starting over reconnected fine, confirming the server was healthy and only cold.ruff check/ruff format --checkclean; module compiles.