livekit: do not answer a server-initiated Leave with a client Leave - #1426
Conversation
When the server ends a session (room deleted, duplicate identity, participant removed) it sends Leave{Disconnect} and closes the signalling socket. The engine handled that by running the same close path as a client-initiated disconnect, which sends a Leave back over a socket that is already gone, so every server-side disconnect logged "dropping pass-through signal — no stream available" at WARN. The session already records that the disconnect was server-initiated for the data-channel guard; use the same flag to skip the reply.
Adds an e2e test that deletes the room through the room service and asserts the client goes straight to Disconnected{RoomDeleted} without a Reconnecting attempt.
Changeset ✓This PR includes a changeset covering all affected packages:
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 1 potential issue.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| if !self.disconnecting.load(Ordering::Acquire) { | ||
| self.signal_client | ||
| .send(proto::signal_request::Message::Leave(proto::LeaveRequest { | ||
| action: proto::leave_request::Action::Disconnect.into(), | ||
| reason: reason as i32, | ||
| ..Default::default() | ||
| })) | ||
| .await; |
There was a problem hiding this comment.
🟡 Reconnect-time server leaves still reply
When SignalClient::restart receives Leave{Disconnect}, disconnecting stays false because that response bypasses on_session_disconnected. close then sends a client Leave into the empty stream, preserving the warning this change targets.
Learn more
A server Leave can arrive while SignalClient::restart waits for its reconnect response. In that path, get_reconnect_response consumes the Leave and returns SignalError::LeaveRequest; the normal signal event handler never sets disconnecting. The reconnect loop recognizes the terminal action and calls close, but SignalClient::restart has already left its stream slot empty. The new guard therefore passes and send logs the same no-stream warning.
Example: The signal link drops and a resume starts. The server rejects that resume with Leave{Disconnect, ServerShutdown}. The engine closes correctly, but first attempts a client Leave and logs dropping pass-through signal — no stream available.
Recommended fix: Carry the server-initiated terminal-disconnect state from SignalError::LeaveRequest into session teardown. For example, expose a session method that marks disconnecting, or pass an explicit close origin, before RtcEngineInner::close invokes RtcSession::close on reconnect-time Leave errors.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Problem
Every server-initiated disconnect (room deleted, duplicate identity, participant removed) logs at WARN:
Reproduced with the Python SDK by joining a room and running
lk room delete: the line appears right beforeDisconnected from room ... with reason: RoomDeleted. It also shows up at the end of everylk agent simulatejob, because the simulation service deletes the room when the job finishes.Cause: the server sends
Leave{Disconnect}and closes the websocket. The engine runsRtcSession::close, which sends the client's ownLeaveback; the stream is already gone, so the signal client warns. Answering a server-initiated Leave is pointless: the server has already ended the session.Fix
RtcSession::closeskips the outgoingLeavewhendisconnectingis set. That flag is already set byon_session_disconnectedfor exactly this case (it guards the "publisher data channel closed unexpectedly" log). Client-initiated disconnects still send their Leave.Test
New e2e test
test_room_deleted_disconnects_without_reconnect: connects, deletes the room throughRoomClient::delete_room, and asserts the room reportsDisconnected{RoomDeleted}without aReconnectingevent.livekit-api(services only) becomes a dev-dependency oflivekitfor it.Ran against a local
livekit-server1.13.5 withRUST_LOG=warn --nocapture. Before the fix the run printed the warning; after, three consecutive runs printed none. The fullreconnection_testfile passes (6 tests).Not in this PR
The same local repro also prints
publisher data channel '_reliable' closed unexpectedlyat ERROR on every room deletion: the data channels close before the server's Leave is processed, so thedisconnectingguard is not yet set when the state change fires. Same root cause, separate ordering problem, left for a follow-up.