chore(llc): cancel call operations on leave #1022
Conversation
WalkthroughA private lifecycle completer was added to the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Call
participant Coordinator
User->>Call: join()
Call->>Call: check _callLifecycleCompleter
alt Lifecycle active
Call->>Coordinator: perform join
Coordinator-->>Call: join result
else Lifecycle ended
Call-->>User: abort with error
end
User->>Call: leave()
Call->>Call: _callLifecycleCompleter.complete()
Note right of Call: All pending operations abort
sequenceDiagram
participant CallSession
participant SFU_WebSocket
CallSession->>SFU_WebSocket: await SfuJoinResponseEvent or SfuErrorEvent (timeout)
alt SfuErrorEvent received
CallSession->>CallSession: log error
CallSession-->>Caller: return failure result
else SfuJoinResponseEvent received
CallSession->>CallSession: proceed with join response processing
CallSession-->>Caller: return success result
end
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ 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). (8)
🔇 Additional comments (17)
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: 1
🧹 Nitpick comments (1)
packages/stream_video/lib/src/call/call.dart (1)
733-737: Consider using more descriptive error messages for better debugging.The error messages could be more specific to help with debugging when operations are cancelled at different stages.
if (_callLifecycleCompleter.isCompleted) { _logger.w(() => '[join] rejected (call was left)'); - return Result.error('call was left'); + return Result.error('call was left before join could start'); }And for the second check:
if (_callLifecycleCompleter.isCompleted) { _logger.w(() => '[join] rejected (call was left during joining)'); - return Result.error('call was left'); + return Result.error('call was left during join operation'); }Also applies to: 759-763
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/stream_video/lib/src/call/call.dart(14 hunks)
⏰ 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). (8)
- GitHub Check: build
- GitHub Check: analyze
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: stream_video_screen_sharing
- GitHub Check: stream_video
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video_push_notification
- GitHub Check: analyze_legacy_version
🔇 Additional comments (6)
packages/stream_video/lib/src/call/call.dart (6)
289-291: LGTM! Good use of Completer for lifecycle management.The
_callLifecycleCompleterprovides a clean way to signal and check for call termination across async operations.
1415-1440: Excellent implementation of cancellable network wait.The use of
Future.anyto race the network status against the call lifecycle is a clean solution that prevents hanging operations when the call is terminated.
1927-1955: Well-designed helper method for consolidating coordinator operations.The
_performGetOperationmethod effectively eliminates code duplication betweenget()andgetOrCreate()while maintaining flexibility through generics and callbacks.
1970-1994: Clean refactoring of the get method.The method now properly leverages the
_performGetOperationhelper, improving code maintainability while preserving functionality.
2065-2092: Consistent refactoring of the getOrCreate method.The method properly uses the
_performGetOperationhelper while maintaining all original functionality, including the outgoing call state management.
1498-1502: Confirm one-time use of Call instances after leave()I ran a codebase search for any
.join(),.get(), or.getOrCreate()calls followingleave(), as well as tests reusing aCallafterleave(), and found no occurrences. This indicates thatCallinstances are currently designed for one-time use.• No
.join()/.get()calls appear afterleave()
• No tests reuse aCallinstance post-leavePlease verify this matches the intended usage pattern. If reusability is required, consider exposing a way to reset or recreate the
_callLifecycleCompleterbefore rejoining.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1022 +/- ##
========================================
+ Coverage 4.24% 4.29% +0.05%
========================================
Files 574 574
Lines 38526 38577 +51
========================================
+ Hits 1634 1658 +24
- Misses 36892 36919 +27 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
resolves FLU-203
We want to make sure all ongoing operations (joining/reconnecting) are canceled when
call.leave()is called. Also extracted a common internal method forget()andgetOrCreate().Summary by CodeRabbit
Bug Fixes
Refactor