fix(llc): rewatch calls after coordinator reconnects #1029
Conversation
|
""" WalkthroughThis change introduces logic to track and manage "watched calls" in the Changes
Sequence Diagram(s)sequenceDiagram
participant CoordinatorWebSocket
participant StreamVideo
participant ClientState
participant Call
CoordinatorWebSocket->StreamVideo: Emits CoordinatorReconnectedEvent on reconnect
StreamVideo->ClientState: Checks watchedCalls
alt watchedCalls not empty
StreamVideo->StreamVideo: Re-query calls with watch=true for watched call CIDs
StreamVideo->Call: _performGetOperation(watch: true)
Call->ClientState: setWatchedCall(this)
end
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
Poem
""" 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/stream_video/CHANGELOG.md (1)
11-11: Minor wording tweak for consistencyMost bullet points in this section refer to “a call” in lowercase, and use active “when X happens” phrasing. Consider aligning wording:
-* Fixed an issue where coordinator events were not listened to after a fast reconnect in a Call. +* Fixed an issue where coordinator events were not listened to after a fast reconnect in a call.Purely stylistic – feel free to ignore if “Call” is intentionally capitalised.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/stream_video/CHANGELOG.md(1 hunks)packages/stream_video/lib/src/call/call.dart(1 hunks)packages/stream_video/lib/src/coordinator/models/coordinator_events.dart(1 hunks)packages/stream_video/lib/src/coordinator/open_api/coordinator_ws.dart(1 hunks)packages/stream_video/lib/src/core/client_state.dart(6 hunks)packages/stream_video/lib/src/stream_video.dart(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/stream_video/CHANGELOG.md
[style] ~11-~11: Consider using a different verb for a more formal wording.
Context: ...l now be correctly set upon unmuting. * Fixed an issue where coordinator events were ...
(FIX_RESOLVE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video
- GitHub Check: stream_video_push_notification
- GitHub Check: analyze_legacy_version
- GitHub Check: build
- GitHub Check: analyze
🔇 Additional comments (9)
packages/stream_video/lib/src/call/call.dart (1)
1946-1949: LGTM! Correct implementation of watched call tracking.The addition of
_streamVideo.state.setWatchedCall(this)whenwatchis true is well-placed and implements the core functionality needed for re-watching calls after coordinator reconnection. The placement after_observeEvents()follows a logical sequence: first set up event observation, then register the call as watched.packages/stream_video/lib/src/coordinator/models/coordinator_events.dart (1)
55-67: LGTM! Well-structured event class.The
CoordinatorReconnectedEventfollows the established patterns in the codebase with proper equality comparison through thepropsgetter and consistent optional field types. The implementation aligns well with similar events likeCoordinatorDisconnectedEvent.packages/stream_video/lib/src/coordinator/open_api/coordinator_ws.dart (1)
51-62: LGTM! Proper reconnection event handling.The implementation correctly identifies the reconnection scenario by checking the state transition from
reconnectingtoconnectedand emits the appropriateCoordinatorReconnectedEventwith the currentuserIdandconnectionId. This follows the existing pattern for event emission in the WebSocket class.packages/stream_video/lib/src/stream_video.dart (1)
477-500: LGTM! Effective reconnection handling for watched calls.The implementation correctly addresses the core issue by re-querying watched calls after reconnection. The logic is sound:
- Checks if there are watched calls to re-subscribe to
- Uses the
watch: trueflag to maintain subscription state- Filters by call CIDs to target only previously watched calls
- Includes proper error handling with logging
- Uses
unawaitedappropriately for fire-and-forget operationThis ensures call watching state is maintained consistently after coordinator reconnection.
packages/stream_video/lib/src/core/client_state.dart (5)
25-26: LGTM! Clean interface extension.The new
watchedCallsstate emitter andsetWatchedCallmethod are properly added to the interface, following the established patterns in the codebase.Also applies to: 46-47
59-59: LGTM! Proper mutable state implementation.The
MutableClientStatecorrectly implements the newwatchedCallsstate emitter with proper initialization to an empty list.Also applies to: 74-75
124-127: LGTM! Logical integration with active call management.Adding the call to watched calls when setting it as active ensures consistency - active calls should naturally be watched for updates.
141-145: LGTM! Consistent cleanup on call removal.Removing calls from both active and watched lists maintains state consistency when calls are removed.
147-154: LGTM! Proper duplicate prevention.The
setWatchedCallimplementation correctly prevents duplicate entries by checking if a call with the same CID already exists in the watched calls list.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1029 +/- ##
========================================
+ Coverage 4.40% 4.41% +0.01%
========================================
Files 574 574
Lines 38593 38626 +33
========================================
+ Hits 1700 1707 +7
- Misses 36893 36919 +26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/stream_video/CHANGELOG.md (1)
11-11: Refine wording for clarity & formality“were not listened to” reads awkwardly; “were not handled” (or “processed”) is clearer and matches the surrounding bullet style.
-* Fixed an issue where coordinator events were not listened to after a fast reconnect in a Call. +* Fixed an issue where coordinator events were not handled after a fast reconnect in a Call.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/stream_video/CHANGELOG.md(1 hunks)packages/stream_video/lib/src/call/call.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/stream_video/lib/src/call/call.dart
🧰 Additional context used
🪛 LanguageTool
packages/stream_video/CHANGELOG.md
[style] ~11-~11: Consider using a different verb for a more formal wording.
Context: ...l now be correctly set upon unmuting. * Fixed an issue where coordinator events were ...
(FIX_RESOLVE)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_push_notification
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video
- GitHub Check: analyze
- GitHub Check: build
- GitHub Check: analyze_legacy_version
resolves FLU-115
We need to re-watch the previously watched calls after the coordinator connection is restored. That applies to active calls as well as calls watched by calling the
call.get(watch: true)method.Summary by CodeRabbit
Bug Fixes
New Features