fix(openai): fail generate_reply fast on conversation_already_has_act… - #6818
fix(openai): fail generate_reply fast on conversation_already_has_act…#6818ByteMaster-1 wants to merge 2 commits into
Conversation
…ive_response A client response.create that races an already-active response (usually a server-VAD reply) is rejected with an error frame, not a response.created, so the future generate_reply handed out was never settled and orphaned until the 10s timeout — surfacing as a generic "generate_reply timed out" long after the fact. Correlate the error to the pending future via error.event_id, fail it immediately with a typed llm.RealtimeError carrying the provider code, and still emit the error event. No SDK-side retry/queue; retry stays client-side. Fixes livekit#6817
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
longcw
left a comment
longcw
left a comment
There was a problem hiding this comment.
looks good to me, something nit:
| # handed out. Fail it now with the provider code attached, instead of orphaning it until | ||
| # the 10s timeout turns it into a generic "generate_reply timed out". Fall through so the | ||
| # error still surfaces as an "error" event. | ||
| if (event_id := event.error.event_id) and ( |
There was a problem hiding this comment.
maybe merge this and the above check under a single if (event_id := event.error.event_id) block and make the comments shorter.
There was a problem hiding this comment.
Done — merged both waiter checks under a single if event_id := event.error.event_id:. Since the chat-ctx and response.create event-ids are separate namespaces (chat_ctx_* vs response_create_*) they can't collide, so the second check is now an elif, and the comments are trimmed to one line each. The response.create branch intentionally falls through instead of returning, so the error still hits the existing emit path (recoverable) or the fatal _is_fatal_error raise (terminal) — same reconnect-stopping behavior as before.
| Note: | ||
| ``await handle`` waits for the reply to finish and never raises; inspect | ||
| ``handle.exception()`` for the failure instead. With a realtime model, a reply | ||
| that races an already-active response (server-VAD created) fails fast with an | ||
| ``llm.RealtimeError`` whose ``code`` is ``conversation_already_has_active_response`` | ||
| rather than stalling until a timeout. The retry policy is yours to choose:: | ||
|
|
||
| handle = session.generate_reply(user_input="...") | ||
| await handle | ||
| err = handle.exception() | ||
| if isinstance(err, llm.RealtimeError) and ( | ||
| err.code == "conversation_already_has_active_response" | ||
| ): | ||
| # let the in-flight response play out, then retry | ||
| if session.current_speech is not None: | ||
| await session.current_speech.wait_for_playout() | ||
| handle = session.generate_reply(user_input="...") | ||
| await handle |
There was a problem hiding this comment.
simplify the docstring? handle.exception() is worth to mention but maybe not the example here.
There was a problem hiding this comment.
Trimmed to a short Note: — kept the handle.exception() guidance and the conversation_already_has_active_response code so callers know what to branch on, and dropped the full retry example.
3f96ba2 to
21ec4fe
Compare
|
@longcw can we please check this if possible , |
2 participants
Fixes #6817
Problem
OpenAI Realtime allows one active response per conversation. When a client
response.createraces an already-active response (typically a server-VAD-createdone), the server rejects it with
conversation_already_has_active_response. Therejection arrives as an
errorframe whoseerror.event_idis the client event id —not a
response.created, and noresponse.donefollows.generate_reply()registers_response_created_futures[event_id]and arms a 10stimeout.
_handle_error()logged/emitted the error but never touched that future, soit orphaned until the timeout fired — and then resolved as a generic
"generate_reply timed out", indistinguishable from a real timeout. Downstream theSpeechHandlestayed unset for ~10s andSpeechHandle.exception()raisedInvalidStateErroruntil then.Fix (before → after)
"generate_reply timed out"._handle_errorcorrelateserror.event_idto the pendingfuture and fails it immediately with a typed
llm.RealtimeErrorwhose.codeisconversation_already_has_active_response._realtime_reply_taskalreadyexcept llm.RealtimeErrorand routes it onto theSpeechHandlevia_mark_done.Both surfaces fire: the exception (via
handle.exception()) and the existingrecoverable
"error"event.Scope (maintainer-aligned)
Detect → correlate → fail fast → surface typed + event. No auto-retry and no queue —
retry is client-side. Docstring on
AgentSession.generate_replyshows the recipe(
await handlenever raises; checkhandle.exception(); on the code, awaitcurrent_speech.wait_for_playout()then retry).Future work (not in this PR): an opt-in SDK-side serialization of
response.createto avoid the collision entirely.
Tests
errorwhoseevent_idmatches a pending future fails itimmediately with the typed error/code, and the error event still emits.
event_idtouches no future.pytest tests/test_realtime/(45 passed),ruff, and mypy all pass.