Repository navigation
Send reliable data after a full reconnect instead of dropping it - #1277
Open
hiroshihorie wants to merge 3 commits into
Open
hiroshihorie wants to merge 3 commits into
hiroshihorie wants to merge 3 commits into
Conversation
sendDataPacket parked a reliable packet sent during any reconnect for the resume replay. A full reconnect never runs that replay, so the packet was reported as sent and never went out. publishData, RPC and data stream writes all hit this. A reliable send now waits the reconnect out and goes out on the connection it leaves, the way rust-sdks waits for reconnection in publish_data. It fails with UnexpectedConnectionState if the engine closes instead. All waits happen before the sequence number is taken, a send that a full reconnect interrupts while it waits for the publisher goes round again instead of failing, and nothing awaits between the last reconnect check and the send. A full reconnect restarts the reliable sequence and clears the resend buffer, as client-sdk-js, client-sdk-swift and rust-sdks do for a new session. Lossy packets are still dropped during a reconnect rather than sent late.
hiroshihorie
marked this pull request as ready for review
November 3, 2026 07:19
hiroshihorie
requested review from
changt,
cloudwebrtc and
xianshijing-lk
as code owners
November 3, 2026 07:19
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
…ends-full-reconnect
cloudwebrtc
approved these changes
Nov 4, 2026
A reliable send made after the reconnect response had replayed the resend buffer, but before the resume finished, was parked in that buffer and never sent, because nothing replayed it again. This branch already fixes it (such sends wait for the reconnect to end), but no unit test covered it. The new test holds the primary connection in connecting so the resume is still running after its response, sends, and checks the packet goes out once the resume ends. It fails on the branch below. The changeset now says that reliable sends wait for the reconnect and fail with UnexpectedConnectionState if the room disconnects instead.
This branch has not been deployed
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.
Top of stack 1276, on #1267.
A reliable data packet sent while the engine does a full reconnect was reported as sent and never went out.
sendDataPacketparked it for the resume replay, and a full reconnect never runs that replay.publishData, RPC and data stream writes all hit this. Found while checking Devin's finding on #1267.A second loss had the same shape on a resume: a packet sent after the reconnect response had replayed the resend buffer, but before the resume finished, was parked and never replayed again. Both are fixed here.
Change
UnexpectedConnectionStateif the engine closes instead, which is how a reconnect that gives up ends.Publisher connection reset. Nothing awaits between the last reconnect check and the send, so a reliable packet is no longer parked.Behavior change for callers
Reliable
publishData, RPC requests and responses, and data stream writes made during a reconnect now wait for the whole reconnect, which can take tens of seconds across retries, and throwUnexpectedConnectionStateif it gives up. Before, they returned at once and the packet was lost. An RPC caller's own timeout can fire while the request is still held, in which case the callee may still run the handler after the reconnect. The changeset says so.Other SDKs
For reliable sends this follows rust-sdks, where
publish_dataholds the reconnect lock's read guard. rust-sdks holds lossy packets too, this PR does not. client-sdk-js has the same loss Flutter had.Tests
test/core/reliable_send_reconnect_test.dart: a send during a full reconnect reaches the new channel only after the reconnect finishes, sequence numbers restart at 1, a waiting send fails when the engine closes, a resume still sends once, and a send made after the reconnect response but before the resume ends goes out. All but the resume-sends-once test fail against Fix waiting for the data channel send buffer to drain #1267'slib.flutter analyze,dart analyze web/, format, import order and version check pass.livekit-server --dev. A sender publishes a numbered reliable packet every 20 ms and the server forces a full reconnect partway. It is marked as an expected failure there until this PR merges, so drop that marker afterwards.Publisher connection resetmainand on Fix waiting for the data channel send buffer to drain #1267, and passes 2 of 2 on this PR, which is the resume window above.reconnect,dataandsignalingpass, apart from the expected failures and one flaky NodeFailure run that times out before the failure is simulated, onmainas well.