Skip to content

fix(llc): run connect and disconnect one at a time in CoordinatorConnection - #1436

Draft
renefloor wants to merge 6 commits into
feat/flu-853-stream-video-test-seamsfrom
fix/flu-857-coordinator-connection
Draft

renefloor wants to merge 6 commits into
feat/flu-853-stream-video-test-seamsfrom
fix/flu-857-coordinator-connection

Conversation

@renefloor

@renefloor renefloor commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

Fixes FLU-857

Part of FLU-859 · 21/26, stacked on #1435

What changed

CoordinatorConnection (lib/src/core/, @internal) connects the user to the coordinator and disconnects them. It is the one place that writes state.connection. StreamVideo.connect, disconnect and dispose delegate to it.

  • A real single-flight guard (bug 4). asCancelable() wraps the future without an onCancel, so the old cancel never stopped the running _connect or _disconnect. A disconnect during a connect could leave the user connected with the state cleared. A Lock now runs connects and disconnects one at a time, in the order they are called, so the last one asked for wins:
    • a disconnect asked for during a connect waits for it, then disconnects;
    • a connect asked for during a disconnect waits, then connects again.
  • One writer of the connection state. The connect and disconnect steps and the socket's connected and disconnected events all write it through the class. MutableClientState.clear() no longer writes it. The events still come in through StreamVideo's event subscription; see the decisions below.
  • dispose disconnects through the same path, with these differences:
    • It does not unregister the push device.
    • It refuses every later connect.
    • It now also writes the disconnected state and clears the ringing and client state, which before it skipped.
    • The auto-reject timers are still cancelled in StreamVideo.dispose; C1 moves them.
  • The connect options are explicit and documented on connect:
    • includeUserDetails only applies to the connect that opens the socket.
    • registerPushDevice registers the device once per connection, also from a later connect when the first one skipped it. Before, the first caller's options silently applied to everyone, so an auto-connect with registerPushDevice: false (the ringing path) meant a later connect() never registered.
  • A connectUser or token fetch that throws now leaves the state failed. Before, it stayed connecting. A connected user's token that cannot be read is returned as a failure instead of thrown.
  • A connect whose socket is up stays connected. Setting up the event and lifecycle subscriptions and registering the push device run after the connect succeeded, and a throw from either is logged. A failed push registration is tried again by the next connect that asks for it.
  • A disconnect always finishes. It closes the socket also when the connection had dropped and is still reconnecting. Before, that case took an early return that skipped closing it and unregistering the device, and the user was connected again when the socket came back. A failed unregister, a socket that fails to close, or a throw while dropping the connection's state is logged, and the user still ends up disconnected. The result reports a failure to close the socket. Without the early return, a disconnect also unregisters the push device when the user never connected, which sends a deleteDevice request. That includes StreamVideo.reset(disconnect: true), and replacing the singleton with failIfSingletonExists: false until C5 makes that a dispose.

Decisions to check

  • dispose keeps the push device registered. The ticket asks dispose to go through _disconnect, which unregisters it. But the background push handler builds a client, handles the ring and disposes it. Unregistering there would stop the next push. disconnect() still unregisters, as it always did.
  • The state is not derived from the client's own CoordinatorConnectionState. That emitter is private to CoordinatorClientOpenApi, and following it would change what the app sees. When the app pauses, StreamVideo cancels its event subscription before it closes the socket, so state.connection stays connected in the background. A connect on resume relies on that. The class follows the same connected and disconnected events _onEvent handled, so this PR changes who writes the state, not when.
  • The lock does not interrupt a connect. A disconnect during a slow connect, such as a socket that takes the connect timeout to fail, waits for that connect. Interrupting it would need CoordinatorClient to support aborting connectUser.
  • _rewatchCalls stays in StreamVideo. It is a queryCalls on the client, not part of the connection. _onEvent calls it after a connected event.
  • Connects queued behind a failed one each try again. Before, they shared the first connect's result. Now each runs after the one before it, so N callers waiting on an unreachable network can wait up to N connect timeouts.
  • A resume reopening the connection still runs outside the lock, so it can interleave with a disconnect, as on v2. C4 reworks that handling.

Tests

  • test/src/core/coordinator_connection_test.dart (21) runs on the C2 fixture:
    • connect, including overlapping connects, which open one connection with the first connect's user details;
    • a disconnect during a connect, a connect during a disconnect, and a disconnect, connect and disconnect in a row;
    • push device registration and unregistration, and a registration that throws and is tried again;
    • dispose, also during a connect (the socket is not closed until the connect finishes), and a connect after dispose;
    • a failed connect, one that throws, an anonymous user, and a token that cannot be used;
    • a throw while setting up the connection;
    • a disconnect after the socket dropped, a failed unregister, and a socket that fails to close;
    • the socket's own events, and re-watching the watched calls after a connected event.
  • Mutation checks:
    • without the lock, the three interleaving tests fail, and without it on dispose, the dispose-during-connect test;
    • unregistering on dispose fails the dispose test;
    • the old early return on a dropped socket, setting the push flag before registering, and dropping the disconnected write each fail a test.
  • stream_video: 1296 passed.

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Oct 8, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.93939% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 43.80%. Comparing base (18ddfc3) to head (cdbaa4d).

Files with missing lines Patch % Lines
...eam_video/lib/src/core/coordinator_connection.dart 92.59% 6 Missing ⚠️
Additional details and impacted files
@@                           Coverage Diff                            @@
##           feat/flu-853-stream-video-test-seams    #1436      +/-   ##
========================================================================
+ Coverage                                 43.72%   43.80%   +0.08%     
========================================================================
  Files                                       419      420       +1     
  Lines                                     31490    31510      +20     
========================================================================
+ Hits                                      13770    13804      +34     
+ Misses                                    17720    17706      -14     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@renefloor
renefloor added this pull request to stack #1442 October 9, 2026 06:12
@renefloor
renefloor force-pushed the fix/flu-857-coordinator-connection branch from 2db696e to 0184b45 Compare October 9, 2026 12:27
@renefloor
renefloor force-pushed the fix/flu-857-coordinator-connection branch 2 times, most recently from 458781a to c023e63 Compare October 9, 2026 12:58
@renefloor
renefloor force-pushed the fix/flu-857-coordinator-connection branch 2 times, most recently from c8a8267 to 66db7fd Compare October 9, 2026 13:17
renefloor and others added 2 commits October 9, 2026 15:27
…ection

The user's coordinator connection moves out of StreamVideo into
CoordinatorConnection, the one writer of the connection state. A lock
replaces the cancelable operations, whose cancel never stopped the
running body, and dispose disconnects through the same path, keeping
the push device registered.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@renefloor
renefloor force-pushed the fix/flu-857-coordinator-connection branch from 66db7fd to cdbaa4d Compare October 9, 2026 13:37
renefloor and others added 4 commits October 9, 2026 16:10
A connect whose socket is up stays connected when setting up the
subscriptions or registering the push device throws, and a failed
registration is tried again by the next connect. A disconnect always
closes the socket, also after it dropped and is reconnecting, and ends
disconnected when unregistering the push device or dropping the
connection's state fails. The client state no longer writes the
connection state on clear.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e read

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant