fix(agents): surface session transport send failures to the caller - #2482
Open
u9g wants to merge 3 commits into
Open
fix(agents): surface session transport send failures to the caller#2482u9g wants to merge 3 commits into
u9g wants to merge 3 commits into
Conversation
RoomSessionTransport.sendMessage returned silently when the transport was closed or the room disconnected, and swallowed write errors with a warning; TcpSessionTransport did the same when closed. A RemoteSession request that could not be sent therefore sat until its timeout, and a SessionHost response that could not be delivered vanished without a trace. Both transports now throw, matching the Python SessionTransport: a dropped message is a request somebody waits out in full, so only the caller can decide what a failure means. The one caller nobody awaits, an event forwarded by SessionHost, logs a single warning instead.
🦋 Changeset detectedLatest commit: 049e817 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 |
sendRequestRaw registered the pending future before sending and only removed it on a response or timeout, so a send that throws left the entry in the map until close(). Clean up in a finally, as the Python RemoteSession does.
Contributor
Author
|
A transport-send-parity.mp4 |
TcpAudioOutput.flush and clearBuffer fire sendMessage without awaiting it. Now that a closed TcpSessionTransport rejects, those calls became unhandled rejections after console teardown. Log at debug instead, matching Python's send_message_threadsafe which drops a send on a closed transport.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
RoomSessionTransport.sendMessagereturned silently when the transport was closed or the room had disconnected, and swallowed write errors behind a warning.TcpSessionTransport.sendMessagereturned silently when closed. Two consequences:RemoteSessionrequest over a dead transport resolved as sent and then waited out its full timeout.SessionHostresponse that could not be delivered disappeared with nothing to correlate to the request.Python's
SessionTransportraises in both cases. Its comment states the contract: a dropped message is a request the caller waits out in full, so failures are raised rather than logged, because only the caller knows whether anyone is waiting on this one. Seesend_messagein https://github.com/livekit/agents/blob/bbfcbcebe9eeac16120ddffdc166d55f59295211/livekit-agents/livekit/agents/voice/remote_session.py#L148-L169 and the TCP variant just below it.Change
RoomSessionTransport.sendMessagethrowsroom session transport is closedwhen closed or the room is disconnected, and rethrows write failures asfailed to send binary stream messagewith the cause attached.TcpSessionTransport.sendMessagethrowstcp session transport is closedwhen closed or the socket is gone. A send already parked on backpressure still resolves when the transport closes, as before.SessionHost.sendEventcatches the rejection and logs onefailed to send session eventwarning, mirroring Python's write loop. Request handling needed nothing:handleRequestSafealready logs and its error-response retry already has a catch.Tests
Three new tests in
remote_session.test.ts: the room transport rejects on a disconnected room, the TCP transport rejects after close, and the host logs rather than leaving an unhandled rejection. All 23 tests in the file pass. The fullsrc/voicedirectory passes exceptamd_session_close.test.ts, which needs a builtdist/and fails identically on an untouched checkout.