fix: session migration triggers a full reconnect instead of a resume - #1197
Conversation
…ting
For a node migration the server sends `LeaveRequest{Action: RESUME,
Reason: MIGRATION}`, which asks the client to reconnect with `reconnect=1`
and keep its session. The engine's leave handler did exactly that, but
`attemptReconnect` then unconditionally escalated any `leaveReconnect`
into a full reconnect:
if (... || [ClientDisconnectReason.leaveReconnect, ...].contains(reason)) {
fullReconnectOnNext = true;
}
That list predates protocol v13 (#439), when a leave with `can_reconnect`
could only mean a full reconnect. The v13 RESUME branch ported in #574
never updated it, so the resume branch has been dead code since: every
RESUME leave ran `restartConnection()`, emitting `RoomReconnectingEvent`,
dropping every `RemoteParticipant` and re-joining.
Drop `leaveReconnect` from the escalation list — the callers that do need
a full reconnect (the RECONNECT leave branch, the connection check) set
`fullReconnectOnNext` themselves. Also stop forcing the flag to false in
the RESUME branch: client-sdk-js and rust-sdks both treat an escalation as
sticky, so a resume that already failed at the media level is not
downgraded back into a resume loop.
Adds `test/core/leave_action_test.dart` covering both leave actions, and
implements `setConfiguration` on the mock peer connection (the resume path
applies the `ReconnectResponse` ICE servers).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`reconnect=1` is the query parameter the server actually keys off to distinguish a resume from a re-join, and it was the only part of the resume contract the test wasn't checking. Also documents why the socket close that follows the Leave is not simulated: a bare socket drop reconnects with reason `signal`, which resumes on its own, so delivering the close before the leave-driven attempt runs makes the test pass even when the leave action is ignored. In production the close arrives a round-trip later and never wins that race, which is why the reported bug reproduced. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
handleReconnect replaces the pending retry timer together with its reason,
and the reason based escalation ran only when that timer fired. A
Leave{RESUME} arriving right after a peer connection failure therefore
swapped the reason to leaveReconnect and the failed connection was resumed
instead of restarted. Now that leaveReconnect no longer escalates on its
own, decide the escalation in handleReconnect so a later request cannot
drop it. The server side resumeConnection switch stays in attemptReconnect
so the latest ClientConfiguration wins.
The RECONNECT test now answers the re-join and waits for
RoomReconnectedEvent instead of tearing down mid restart, and asserts the
signal URL carries no reconnect flag. Add cases for a stale
fullReconnectOnNext, for resumeConnection DISABLED from the server, and for
a Leave{RESUME} racing a pending peer failure retry. The RESUME test also
checks the ReconnectResponse configuration reached both transports.
E2EContainer gains answerJoin() and a clientConfiguration option so tests
can drive a full reconnect and shape the join response.
| if ([ | ||
| ClientDisconnectReason.negotiationFailed, | ||
| ClientDisconnectReason.peerConnectionFailed, | ||
| ].contains(reason)) { | ||
| fullReconnectOnNext = true; |
There was a problem hiding this comment.
🟡 Successful resume leaves stale escalation
During an active resume, a peer failure makes handleReconnect set fullReconnectOnNext for a retry that can be canceled. attemptReconnect clears the retry after success, but not the flag. A later disconnect is suppressed, or the next resume becomes a full reconnect.
Learn more
A peer connection can report failure while resumeConnection is still restoring ICE. This block records the required full reconnect immediately and schedules a retry. If the active resume then reaches connected state, attemptReconnect cancels that retry but leaves fullReconnectOnNext true. The room subsequently drops an EngineDisconnectedEvent because the disconnect handler treats the flag as an active restart.
Example: A signal reconnect starts, then the primary peer connection briefly reports failed before its ICE restart reaches connected. The resume succeeds and its queued full reconnect is canceled. The next ordinary signal loss emits no RoomDisconnectedEvent; alternatively, a later migration performs an unnecessary full reconnect.
Recommended fix: Track a full-reconnect request separately from the flag consumed by the active attempt. After an attempt succeeds, either dispatch any escalation recorded during that attempt or clear it explicitly; do not cancel its retry while retaining only fullReconnectOnNext. Add a regression test where peerConnectionFailed arrives after _attemptingReconnect becomes true and the active resume subsequently succeeds.
Was this helpful? React with 👍 or 👎 to provide feedback.
Fixes CLT-3324. **Stacked on #1197** (base is that branch, not `main`). Independent of #1198 — they touch different code and can merge in either order. ## Problem Three ways a reconnect request could be lost or altered in `Engine`: 1. **Reason override** — `handleReconnect()` clears the pending timer and reschedules with its own reason, so a later caller (the socket close that follows a server `Leave`) overrides an earlier one, and the escalation implied by the first reason is silently dropped. 2. **Dropped request** — `attemptReconnect()` early-returns on `_attemptingReconnect`, so a full-reconnect request arriving mid-attempt is never acted on. 3. **Stale flag** — a successful attempt calls `_clearPendingReconnect()`, cancelling the queued escalation and leaving `fullReconnectOnNext` stuck true, which also suppresses the next legitimate `RoomDisconnectedEvent`. This is the mechanism behind #1197: pre-fix, whether a migration resumed or full reconnected depended on whether the socket-close handler beat the leave-driven `Timer(0)`. In production the close arrives a round-trip later and loses, so the bug reproduced. ## Fix **(1)** The reason → escalation mapping moves from `attemptReconnect` to `handleReconnect`, where the request originates, so it is captured as state instead of being re-derived later from a reason that may have been replaced. The `resumeConnection == DISABLED` check stays in `attemptReconnect` — that's config, not a request. **(2)/(3)** `fullReconnectOnNext` is consumed at the start of an attempt into a local. From there a `true` value unambiguously means a *new* request arrived while the attempt was running, which the `finally` block dispatches. This is client-sdk-js's pattern; rust-sdks does the equivalent with a sticky `full_reconnect |=`. **API note.** Consuming the flag up front means it no longer describes the running attempt, which `Room` relied on to skip fast-connect republishing during a full reconnect's re-join (and to suppress the mid-reconnect disconnect event). Added `Engine.isFullReconnectInProgress` for that question and pointed `Room` at it. `fullReconnectOnNext` keeps its meaning as the *pending request*, so `sendSimulateScenario(fullReconnect: true)` and the connection check are unaffected. Also aligns the failure path with js/rust: a failed full reconnect stays a full reconnect. ## Tests `test/core/reconnect_request_dispatch_test.dart`. The first test ports rust-sdks' `test_resume_escalation_sticks_across_cycles` (`livekit/tests/peer_connection_signaling_test.rs`), which needs a live SFU, two participants and a published sine track and observes the escalation via `LocalTrackRepublished`. The mock transport lets us inject the concurrent request directly and observe it as `RoomReconnectingEvent`, which only the full path emits. - full-reconnect request injected mid-resume → cycle 1 still resumes, cycle 2 re-joins - `peerConnectionFailed` followed by a `signal` request → still re-joins, does not resume - successful resume → neither flag left set Verified both behavioral tests fail against the pre-fix engine (test 1: cycle 2 never happens; test 2: `reconnect=1`, i.e. it resumed). Full suite (412 tests), `flutter analyze`, format and import_sorter all clean. --- ## Update 2026-09-14 (hiroshi) Additive changes on top of the original PR, after taking it over: **Fix (1) already landed.** The reason to escalation move into `handleReconnect` shipped on `main` with #1197, so this branch now only carries the consume and dispatch change plus the two fixes below. Synced with `main` by merge, no history rewrite. **Two more ways a request could vanish** (`d905f517`): - `restartConnection` cleared `fullReconnectOnNext` after joining. The flag was already consumed when the attempt started, so a true value there was a new request, typically a `RECONNECT` leave from the node just joined, and the reset erased it before the `finally` dispatch. It is no longer cleared there. - `resumeConnection` emitted Resumed without checking the signal socket. If the socket dropped while the peer connections were being restored, the attempt reported success on a dead connection and its success path cancelled the retry the drop had scheduled. It now re-checks the socket before emitting Resumed and throws a recoverable `ConnectException`, so the retry path runs another resume. Same check as client-sdk-js and rust-sdks. **Logging** (`467ec5c4`): the `catch` in `attemptReconnect` now logs why an attempt failed. **Tests added** to `test/core/reconnect_request_dispatch_test.dart`: - a peer failure reported through `handleReconnect` mid-resume is dispatched as a full reconnect afterwards - a `RECONNECT` leave arriving mid-restart is not lost (fails without the fix) - a signal drop right behind the `ReconnectResponse` is retried instead of reported as success (fails without the fix) **Verified on Cloud** with the `disconnectSignalOnResume` scenario from #1202, where the server answers the resume and then cuts the socket. This branch, one second to a working connection, one `RoomReconnectedEvent`: ``` 23:44:36 Handle ReconnectResponse 23:44:36 Signal disconnected DisconnectReason.disconnected 23:44:36 resumeConnection: primary is connected: true 23:44:36 attemptReconnect: resume failed: [ConnectException] resumeConnection: signal connection severed during resume 23:44:36 WebSocket reconnecting in 300 ms, retry times 1 23:44:36 Handle ReconnectResponse 23:44:36 emit (public) RoomReconnectedEvent() ``` `main` at `12ec8ef3`, six seconds, two `RoomReconnectedEvent`s, the first on a dead socket: ``` 23:48:15 Handle ReconnectResponse 23:48:15 Signal disconnected DisconnectReason.disconnected 23:48:15 resumeConnection: primary is connected: true 23:48:15 emit (public) RoomReconnectedEvent() 23:48:15 Could not send message, socket not connected (x17, the ICE restart candidates) 23:48:20 onDisconnected reason:peerConnectionClosed 23:48:21 resumeConnection: primary is connected: false 23:48:21 emit (public) RoomReconnectedEvent() ``` The false success on `main` also silently dropped every trickle candidate for the ICE restart, so recovery came from a media failure rather than from the signal layer. Full suite 418, analyzer, format and import order clean. **Retry limit** (`5baf75e9`): the `SignalConnectedEvent` handler reset `_reconnectAttempts` to zero. A resume opens its socket before the peer connections are restored, so any attempt failing after that point, a media timeout or the severed socket check above, started counting from zero again and the retry limit was unreachable. Removed; `_clearPendingReconnect` resets on a completed attempt and `cleanUp` on disconnect, matching client-sdk-js. New test severs three consecutive resumes and checks the scheduled attempts climb 2, 3, 4. Without the fix they read 2, 2, 2. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Hiroshi Horie <548776+hiroshihorie@users.noreply.github.com>
Fixes CLT-3322.
Problem
When the server initiates a node migration, the SDK performs a full reconnect instead of a resume:
RoomReconnectingEventis emitted, everyRemoteParticipantis torn down (oneParticipantDisconnectedEventeach), the client re-joins, andRoomConnectedEventfires again. It should emitRoomResumingEventand keep the session intact.Reported by a customer testing
Room.sendSimulateScenario(migration: true)on 2.12.0:Migrations are routine on Cloud, so every Flutter client sees its remote participants disappear and re-join, subscriptions rebuilt, and per-participant UI state lost.
Root cause
For a migration the server sends
LeaveRequest{Action: RESUME, Reason: MIGRATION}(livekit/pkg/rtc/participant.go,MaybeStartMigration) and then closes the signal socket.RESUMEmeans: reconnect withreconnect=1, keep the session.Engine's leave handler did the right thing — clearedfullReconnectOnNextand calledhandleReconnect(ClientDisconnectReason.leaveReconnect). ButattemptReconnectimmediately re-set the flag:That list predates protocol v13 (added in #439, when a leave with
can_reconnectcould only mean a full reconnect). The v13RESUMEbranch was ported from client-sdk-js in #574 but the escalation was never updated — so the resume branch has been dead code ever since and everyRESUMEleave ended up inrestartConnection().Neither reference SDK behaves this way:
RTCEngine.attemptReconnectescalates only forresumeConnection === DISABLEDor a never-connected PeerConnection.on_session_eventroutesAction::Resumestraight into a resume cycle.Changes
lib/src/core/engine.dart:leaveReconnectfrom the escalation list inattemptReconnect. The callers that genuinely need a full reconnect — theRECONNECTleave branch andconnection_check/checks/checker.dart— already setfullReconnectOnNext = truethemselves.fullReconnectOnNext = falsein theRESUMEbranch. JS and Rust both treat an escalation as sticky, so a resume that already failed at the media level isn't downgraded back into a resume loop.test/mock/peerconnection_mock.dart: implementsetConfiguration(it threwUnimplementedError; the resume path applies theReconnectResponseICE servers to both transports).Tests
New
test/core/leave_action_test.dart:RESUME(migration) →RoomResumingEvent, noRoomReconnectingEvent, noParticipantDisconnectedEvent, remote participants retained,fullReconnectOnNextback to false.RECONNECT→RoomReconnectingEventand participants dropped, as before.Confirmed the
RESUMEtest fails against the pre-fix code (times out waiting forRoomReconnectedEvent, because the engine re-joins instead of resuming). Full suite (409 tests),flutter analyze,dart formatandimport_sorterall clean.Note for app developers
RoomResumingEventis the Flutter analog of JS'sSignalReconnecting;RoomReconnectingEventmeans a full reconnect. Both paths end inRoomReconnectedEvent.Follow-ups (not in this PR)
attemptReconnectearly-returns on_attemptingReconnect, and the successful attempt's_clearPendingReconnect()cancels the queued retry, leavingfullReconnectOnNextstale-true (which also suppresses the next legitimateRoomDisconnectedEvent). JS consumes the flag at attempt start and re-dispatches infinally; that can't be copied verbatim here becauseRoomreadsengine.fullReconnectOnNextduring the restart's join to skip fast-connect republishing.RoomConnectedEventagain on a full reconnect (driven offEngineJoinResponseEvent); JS only emitsReconnected. Changing that is a public-behavior change.🤖 Generated with Claude Code