(phonic): add logic for maintaining a single ws conn and remove say()#1220
(phonic): add logic for maintaining a single ws conn and remove say()#1220tinalenguyen merged 7 commits intobrian/reuse-realtimefrom
Conversation
🦋 Changeset detectedLatest commit: ded311a The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| // this means the content is the same as the previous session | ||
| const capabilities = this.llm.capabilities; | ||
| if (!rtReused || capabilities.midSessionInstructionsUpdate) { | ||
| if (!rtReused && this.realtimeSession?.realtimeModel.provider == 'phonic') { |
There was a problem hiding this comment.
Should this be if (this.realtimeSession?.realtimeModel.provider == 'phonic'?
In the Python implementation, it looks like update_session is called whether or not rt_reused is true.
There was a problem hiding this comment.
ah, i think it should be if rtReused is true then we call _updateSession, but it's also dependent on whether or not your _updateSession can support both the initial and reset config. in that case we can call _updateSession unconditionally
| return this.startNewAssistantTurn({ userInitiated: true }); | ||
| } | ||
| this.pendingGenerateReplyFut = new Future<llm.GenerationCreatedEvent>(); | ||
| this.sendGenerateReply(instructions); |
There was a problem hiding this comment.
🔴 Unhandled promise rejection from fire-and-forget sendGenerateReply can crash Node.js
In generateReply(), this.sendGenerateReply(instructions) at line 477 is called without being awaited or having .catch() attached. Inside sendGenerateReply, if this.socket.sendGenerateReply(...) at line 489 throws (e.g., the underlying WebSocket is in a closing/errored state that passes the !this.socket guard at line 484), the resulting promise rejection is completely unhandled. In modern Node.js, unhandled promise rejections terminate the process by default. Additionally, since pendingGenerateReplyFut is never rejected through this path, the caller of generateReply() may hang until the socket close handler fires rejectPendingGenerateReply().
| this.sendGenerateReply(instructions); | |
| this.sendGenerateReply(instructions).catch((err) => { | |
| if (this.pendingGenerateReplyFut && !this.pendingGenerateReplyFut.done) { | |
| this.pendingGenerateReplyFut.reject(err instanceof Error ? err : new Error(String(err))); | |
| this.pendingGenerateReplyFut = undefined; | |
| } | |
| }); |
Was this helpful? React with 👍 or 👎 to provide feedback.
| case 'assistant_chose_not_to_respond': | ||
| case 'input_cancelled': |
There was a problem hiding this comment.
🔴 pendingGenerateReplyFut hangs forever when Phonic server sends assistant_chose_not_to_respond or input_cancelled
The new generateReply implementation creates a pendingGenerateReplyFut Future (line 476) and returns its .await promise (line 479). This future is only resolved inside startNewAssistantTurn (line 763-766) or rejected in rejectPendingGenerateReply (only called on socket/session close). However, the server events assistant_chose_not_to_respond and input_cancelled (lines 624-625) fall through to the default break without resolving or rejecting the pending future. When the Phonic server decides not to respond after a generate_reply request, the caller (realtimeReplyTask at agents/src/voice/agent_activity.ts:2925) blocks forever on await this.realtimeSession.generateReply(instructions). This also blocks the speech pipeline drain, which can cause handoff deadlocks since _pauseSchedulingTask waits for _mainTask.result.
(Refers to lines 624-629)
Was this helpful? React with 👍 or 👎 to provide feedback.
3 participants
No description provided.