fix: stop dropping session host responses - #2259
Conversation
🦋 Changeset detectedLatest commit: 480fb6b The changes in this PR will be included in the next version bump. This PR includes changesets to release 39 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}
| override async sendMessage(msg: pb.AgentSessionMessage): Promise<void> { | ||
| const socket = this.socket; | ||
| if (this.closed || socket === null) return; | ||
|
|
||
| const data = msg.toBinary(); | ||
| const header = Buffer.allocUnsafe(TCP_HEADER_SIZE); | ||
| header.writeUInt32BE(data.length, 0); | ||
| const flushed = socket.write(Buffer.concat([header, data])); | ||
| if (this.closed || this.socket === null) { | ||
| throw new Error('tcp session transport is closed'); | ||
| } |
There was a problem hiding this comment.
🟡 Console audio messages sent without error handling can now crash the process during shutdown
Outgoing session messages are now rejected instead of silently dropped once the connection is gone (throw new Error('tcp session transport is closed') at agents/src/voice/remote_session.ts:366-368), but the console audio path still fires these sends without handling a failure, so a shutdown-time send turns into an unhandled rejection.
Impact: A console/agent run can log spurious errors or terminate abruptly while shutting down instead of exiting cleanly.
Fire-and-forget senders in console_io are not adapted to the new throwing contract
Previously both transports returned silently when closed/disconnected, so every caller was safe. Now TcpSessionTransport.sendMessage (agents/src/voice/remote_session.ts:365-375) and RoomSessionTransport.sendMessage (agents/src/voice/remote_session.ts:210-219) reject.
Unhandled callers that were not updated:
agents/src/voice/console_io.ts:124—void this.transport.sendMessage(...)inTcpAudioOutput.flush()agents/src/voice/console_io.ts:149—void this.transport.sendMessage(...)inTcpAudioOutput.clearBuffer()
The console entrypoint runs in the main process and installs no unhandledRejection handler (unlike agents/src/ipc/job_proc_lazy_main.ts:266), so a rejection there follows Node's default abort-on-unhandled-rejection behavior. AgentSession.close() closes the session host — and therefore the shared TCP transport — at agents/src/voice/agent_session.ts:1805, while console.ts closes the same transport again in its finally block; any flush/clear emitted after that point rejects.
Additionally TcpAudioOutput.captureFrame (agents/src/voice/console_io.ts:114) awaits the send, so a closed transport now propagates an error up the audio output pipeline instead of being ignored.
Prompt for agents
The transports in agents/src/voice/remote_session.ts now reject on send failure (RoomSessionTransport.sendMessage and TcpSessionTransport.sendMessage throw 'room/tcp session transport is closed' and wrap stream errors) instead of silently returning as before. Callers that previously relied on the silent no-op were not updated: agents/src/voice/console_io.ts uses `void this.transport.sendMessage(...)` in TcpAudioOutput.flush() and TcpAudioOutput.clearBuffer(), which now produces unhandled promise rejections when the transport is already closed (the console entrypoint in agents/src/console.ts has no process-level unhandledRejection handler). TcpAudioOutput.captureFrame also awaits the send and will now propagate errors up the audio output pipeline during teardown. Audit all call sites of SessionTransport.sendMessage and attach appropriate catch/log handling (or a small helper) so shutdown-time sends degrade gracefully.
Was this helpful? React with 👍 or 👎 to provide feedback.
| return response; | ||
| } | ||
|
|
||
| async waitForReady(timeout = 5000, retryInterval = 500): Promise<void> { |
There was a problem hiding this comment.
🟡 New public readiness method is added without documentation
A new publicly exported method is added (waitForReady at agents/src/voice/remote_session.ts:1495) with no TypeDoc comment, so its behavior and parameters are missing from the generated API docs.
Impact: Users of the published package get an undocumented public API entry.
Repository rule
CONTRIBUTING.md requires: "If writing new methods/interfaces/enums/classes, document them. This project uses TypeDoc for automatic API documentation generation, and every new addition has to be properly documented." RemoteSession is exported from agents/src/voice/index.ts, so waitForReady(timeout = 5000, retryInterval = 500) is part of the public API surface and needs a doc comment describing the millisecond units, retry behavior, and thrown errors.
| async waitForReady(timeout = 5000, retryInterval = 500): Promise<void> { | |
| /** | |
| * Wait until the remote session host answers a ping, retrying transport failures. | |
| * | |
| * @param timeout - Overall deadline in milliseconds | |
| * @param retryInterval - Per-attempt timeout / delay between retries in milliseconds | |
| * @throws The last transport error, or a timeout error if the deadline elapses. | |
| */ | |
| async waitForReady(timeout = 5000, retryInterval = 500): Promise<void> { |
Was this helpful? React with 👍 or 👎 to provide feedback.
Ports livekit/agents#6781.
Summary
RemoteSession.waitForReadyparity and retry transport startup failuresSource diff coverage
livekit-agents/livekit/agents/voice/remote_session.py: adapted toagents/src/voice/remote_session.ts. Python channels, asyncio locks/tasks, and second-based deadlines map to JS queues,Mutex,Task, and a 3000 ms shared deadline. All behavior is retained: explicit send failures, serialized room/TCP writes, ordered single-reader room ingestion, graceful handler draining and request-type logging, queued single-writer events (including pre-start events), and readiness retry/error propagation. The target lackedRemoteSession.waitForReady, so that source dependency was ported as public API rather than omitted.tests/test_remote_session.py: adapted toagents/src/voice/remote_session.test.ts. All 9 added source test cases are ported to Vitest using JS promises, fake RTC byte streams, and logger spies while retaining the source assertions.No source files or behaviors were marked not applicable.
Verification
pnpm test agents/src(115 files, 1597 passed, 5 skipped)pnpm test agents/src/voice/remote_session.test.ts(29 passed)pnpm --filter @livekit/agents typecheckpnpm build(40 packages)pnpm lint(passes with pre-existing warnings)pnpm format:checkcue-clitext-mode runtime drive: initial greeting and subsequent exactport validatedassistant framework event both resolvedThe repository-wide
pnpm testwas also run: 2065 tests passed, with unrelated failures from a missingCEREBRAS_API_KEY, invalid/missing model assets, a Hugging Face network timeout, and existing example/plugin assertions. The complete touched-package suite above passes.pnpm --filter @livekit/agents api:checkis blocked before API comparison by the existing generatedexport * asdeclaration syntax unsupported by the pinned API Extractor.Ported from livekit/agents#6781
Original PR description
Alternative to #6665, addressing livekit/agents#6661. Diagnosis is @samanyugoyal2010's — commits are co-authored to them, happy to close in favour of theirs.
What was happening
A
run_inputresponse that never reached the client looked exactly like a hung agent: the caller blocked for its full 60s timeout while the transcript was complete and the agent log showed a clean shutdown.Three paths dropped one silently:
RoomSessionTransport.send_messageswallowed send failures into a warning and returned.SessionHost.aclosecancelled in-flight handlers, discarding the response of a request that had already done its work.Nothing serialized writes either, and every inbound message spawned a task to read it — which could finish out of order and deliver messages swapped.
The fix
aclosedrains handlers before cancelling, and logs the request types it gave up on.Responses stay direct — a handler awaits the transport and gets the failure — so no per-message future is needed to hand results back. Events are the only thing that needs a queue.
14 tests, each verified to fail without its change.
Caveats