fix(realtime): surface pending-reply drop on reconnect via SpeechHandle.exception() - #6436
fix(realtime): surface pending-reply drop on reconnect via SpeechHandle.exception()#6436ByteMaster-1 wants to merge 3 commits into
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
|
@davidzhao @longcw Gentle bump when you have a moment — this is the reconnect-drop observability follow-up from the #6352 review. CI is green and the Devin flags are resolved; PTAL ,happy to adjust anything. |
|
Hi @longcw @davidzhao , following up on this PR when you have a moment. It serves as the second half of the #6352 rescope, following the merge of its sibling #6445 last week. Please let me know if you would like any adjustments to the current implementation. |
| # re-prompting is sensible once the new session is up, so surface it as a | ||
| # retryable APIConnectionError (retryable=True) rather than a bare RealtimeError | ||
| self._fail_pending_response_futures( | ||
| APIConnectionError( |
There was a problem hiding this comment.
so the only change of this pr is replace the RealtimeError with APIConnectionError here, what is the purpose for that?
There was a problem hiding this comment.
The type swap is the mechanism; the goal is making the reconnect drop actionable for the application.
On a transient socket drop, _reconnect() discards the pending generate_reply future and never re-creates the response — so the turn silently produces nothing. That discard already reaches SpeechHandle.exception() (via the #6304 plumbing), but as a bare RealtimeError it's indistinguishable from a terminal failure like "realtime session closed". An app that wants to re-prompt only on a transient drop would have to string-match the message.
APIConnectionError carries retryable=True, so the app can branch on the same taxonomy the rest of the SDK already uses — no new types, no string matching:
exc = handle.exception()
if isinstance(exc, APIError) and exc.retryable:
# transient reconnect drop — safe to re-prompt
This is exactly the retryable-flag contract #6352 introduced in this same file for fatal errors (retryable=False) — this PR completes the other half for the transient case. The terminal "realtime session closed" path deliberately stays a bare RealtimeError, since there's nothing to retry against.
This is the follow-up we agreed on in the #6352 review ("yes we can make the drop on reconnect inside the plugin observable via SpeechHandle.exception()").
…le.exception()
On a transient socket drop, the OpenAI realtime session reconnects and discards
any pending generate_reply future, then never re-creates the response — so the
turn silently produced nothing. The discard was raised as a bare
llm.RealtimeError, indistinguishable from a terminal failure.
Fail the discarded futures with APIConnectionError instead (retryable=True):
the session is coming back, so re-prompting is sensible. The voice turn loop
already lands generate_reply failures on the SpeechHandle via _mark_done(error=);
widen its catch to APIError so the discard reaches SpeechHandle.exception().
Apps can now distinguish a retryable drop without string-matching:
exc = handle.exception()
if isinstance(exc, APIError) and exc.retryable:
... # transient reconnect discard — safe to re-prompt
The terminal "realtime session closed" path (fatal error / retries exhausted)
stays a bare RealtimeError — nothing to retry against.
Follow-up agreed in the livekit#6352 review.
…eption() The say() await in _realtime_reply_task caught only RealtimeError, while the sibling generate_reply await catches APIError too. Make them symmetric so a transient reconnect discard (retryable APIError) lands on the SpeechHandle for any realtime plugin that implements say() via the response-future mechanism.
21cf08f to
d8ac123
Compare
2 participants
Follow-up agreed in #6352 review ("yes we can make the drop on reconnect inside the plugin observable via SpeechHandle.exception()").
Incident. On a transient WebSocket drop, the OpenAI realtime session reconnects and _reconnect() discards any pending generate_reply future without re-creating the response. The turn silently produced nothing, and the discard was a bare RealtimeError — indistinguishable from a terminal failure, so an application couldn't tell it was safe to re-prompt.
Fix. Discarded futures are now failed with APIConnectionError (retryable=True) — the session is coming back, so re-prompting is sensible. The voice turn loop already routes generate_reply failures onto the SpeechHandle (_mark_done(error=), from #6304); its catch is widened to APIError so the discard deterministically lands on SpeechHandle.exception(). This reuses the same retryable-flag contract #6352 introduced in this file for fatal server errors.
App contract (no new types, no string-matching):
exc = handle.exception()
if isinstance(exc, APIError) and exc.retryable:
# transient reconnect discard — safe to re-prompt
Deliberately out of scope:
generate_reply timeout still raises bare RealtimeError; converting it to APITimeoutError for the same retryable contract is a follow-up PR.
The terminal "realtime session closed" path (fatal error / retries exhausted) stays a bare RealtimeError — nothing to retry against.
A response already streaming when the socket drops is closed via _close_current_generation (partial output, no error) — that's playout truncation, a separate issue.
Timing note: generate_reply has a 10s client-side timeout. If reconnection outlasts it, the timeout wins the race and the handle gets the (bare RealtimeError) timeout instead of the discard — acceptable, and the follow-up timeout PR will fold that into the same retryable contract.
Behavior change for direct callers: anyone consuming a raw RealtimeSession.generate_reply() future and catching only RealtimeError will now see APIConnectionError on reconnect. No in-repo caller is affected (only the voice turn loop consumes that future, and it's updated here).