Improve reconnection events, add RoomAttemptReconnectEvent. - #439
Merged
Conversation
cloudwebrtc
marked this pull request as ready for review
December 23, 2023 01:08
cloudwebrtc
force-pushed
the
fix/try-to-fix-issue-438
branch
from
December 23, 2023 16:21
69df9ba to
24c3702
Compare
davidzhao
approved these changes
Dec 27, 2023
Co-authored-by: David Zhao <dz@livekit.io>
hiroshihorie
added a commit
that referenced
this pull request
Sep 14, 2026
…ting (#1197) Fixes CLT-3322. ## Problem When the server initiates a node migration, the SDK performs a **full reconnect** instead of a **resume**: `RoomReconnectingEvent` is emitted, every `RemoteParticipant` is torn down (one `ParticipantDisconnectedEvent` each), the client re-joins, and `RoomConnectedEvent` fires again. It should emit `RoomResumingEvent` and keep the session intact. Reported by a customer testing `Room.sendSimulateScenario(migration: true)` on 2.12.0: ``` Room reconnecting Participant disconnected: scanner_camera Participant disconnected: operator_camera Participant disconnected: additional_camera ... Room connected: 48263580-7cfd-499d-a45c-02741ba74483 ``` 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. `RESUME` means: reconnect with `reconnect=1`, keep the session. `Engine`'s leave handler did the right thing — cleared `fullReconnectOnNext` and called `handleReconnect(ClientDisconnectReason.leaveReconnect)`. But `attemptReconnect` immediately re-set the flag: ```dart if (_clientConfiguration?.resumeConnection == DISABLED || [ClientDisconnectReason.leaveReconnect, // <- always true for a leave-driven reconnect ClientDisconnectReason.negotiationFailed, ClientDisconnectReason.peerConnectionFailed].contains(reason)) { fullReconnectOnNext = true; } ``` That list predates protocol v13 (added in #439, when a leave with `can_reconnect` could only mean a full reconnect). The v13 `RESUME` branch 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 every `RESUME` leave ended up in `restartConnection()`. Neither reference SDK behaves this way: - **client-sdk-js** — `RTCEngine.attemptReconnect` escalates only for `resumeConnection === DISABLED` or a never-connected PeerConnection. - **rust-sdks** — `on_session_event` routes `Action::Resume` straight into a resume cycle. ## Changes `lib/src/core/engine.dart`: 1. Drop `leaveReconnect` from the escalation list in `attemptReconnect`. The callers that genuinely need a full reconnect — the `RECONNECT` leave branch and `connection_check/checks/checker.dart` — already set `fullReconnectOnNext = true` themselves. 2. Stop forcing `fullReconnectOnNext = false` in the `RESUME` branch. 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`: implement `setConfiguration` (it threw `UnimplementedError`; the resume path applies the `ReconnectResponse` ICE servers to both transports). ## Tests New `test/core/leave_action_test.dart`: - `RESUME` (migration) → `RoomResumingEvent`, no `RoomReconnectingEvent`, no `ParticipantDisconnectedEvent`, remote participants retained, `fullReconnectOnNext` back to false. - `RECONNECT` → `RoomReconnectingEvent` and participants dropped, as before. Confirmed the `RESUME` test fails against the pre-fix code (times out waiting for `RoomReconnectedEvent`, because the engine re-joins instead of resuming). Full suite (409 tests), `flutter analyze`, `dart format` and `import_sorter` all clean. ## Note for app developers `RoomResumingEvent` is the Flutter analog of JS's `SignalReconnecting`; `RoomReconnectingEvent` means a full reconnect. Both paths end in `RoomReconnectedEvent`. ## Follow-ups (not in this PR) - A full-reconnect request arriving while a reconnect attempt is in flight is dropped: `attemptReconnect` early-returns on `_attemptingReconnect`, and the successful attempt's `_clearPendingReconnect()` cancels the queued retry, leaving `fullReconnectOnNext` stale-true (which also suppresses the next legitimate `RoomDisconnectedEvent`). JS consumes the flag at attempt start and re-dispatches in `finally`; that can't be copied verbatim here because `Room` reads `engine.fullReconnectOnNext` during the restart's join to skip fast-connect republishing. - Flutter emits `RoomConnectedEvent` again on a full reconnect (driven off `EngineJoinResponseEvent`); JS only emits `Reconnected`. Changing that is a public-behavior change. 🤖 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>
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
2 participants
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.
No description provided.