fix(ui): extract callkit manager to make it independent of flutter initialization #1140
Conversation
WalkthroughThis PR refactors iOS push notification handling to implement deferred event processing. The changes introduce late initialization of event handlers with automatic queuing and flushing of events, allowing the system to buffer events before the handler becomes available. Changes
Sequence Diagram(s)sequenceDiagram
participant VoIP as VoIP System
participant Plugin as StreamVideoPushNotificationPlugin
participant Manager as StreamVideoCallkitManager
participant Handler as EventCallbackHandler
rect rgb(230, 245, 250)
Note over VoIP,Handler: New Flow: Event Queuing
end
VoIP->>Manager: sendEvent (before handler attached)
alt No handler attached
Manager->>Manager: Queue event in pendingEvents
Note over Manager: Event buffered
end
Handler->>Manager: setEventHandler(handler)
activate Manager
Manager->>Manager: Attach handler
Manager->>Manager: flushPendingEvents()
loop For each queued event
Manager->>Handler: Deliver queued event
end
deactivate Manager
VoIP->>Manager: sendEvent (after handler attached)
Manager->>Handler: Deliver event immediately
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Brazol
changed the title
![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_push_notification/ios/stream_video_push_notification/Sources/stream_video_push_notification/StreamVideoCallkitManager.swift (1)
45-57: Consider thread synchronization for robustness.CXProviderDelegate methods may be called on CallKit's internal dispatch queues. The unsynchronized access to
eventHandlerandpendingEventscould cause a race condition ifsetEventHandleris called whilesendEventis simultaneously queuing an event. While the window for this race is narrow (only during startup), consider using a serial dispatch queue or lock for these shared resources.🔎 Example using a serial queue
+ private let eventQueue = DispatchQueue(label: "io.getstream.callkit.events") + private func sendEvent(_ event: String, _ body: [String: Any?]?) { if silenceEvents { print(event, " silenced") return } - guard let eventHandler = eventHandler else { - pendingEvents.append((event: event, body: body)) - return - } - - eventHandler.send(event, body ?? [:]) + eventQueue.async { [weak self] in + guard let self = self else { return } + guard let handler = self.eventHandler else { + self.pendingEvents.append((event: event, body: body)) + return + } + handler.send(event, body ?? [:]) + } }Apply similar synchronization to
setEventHandlerandflushPendingEvents.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (4)
packages/stream_video_flutter/CHANGELOG.mdpackages/stream_video_push_notification/CHANGELOG.mdpackages/stream_video_push_notification/ios/stream_video_push_notification/Sources/stream_video_push_notification/StreamVideoCallkitManager.swiftpackages/stream_video_push_notification/ios/stream_video_push_notification/Sources/stream_video_push_notification/StreamVideoPushNotificationPlugin.swift
🧰 Additional context used
🧬 Code graph analysis (1)
packages/stream_video_push_notification/ios/stream_video_push_notification/Sources/stream_video_push_notification/StreamVideoCallkitManager.swift (1)
packages/stream_video_push_notification/ios/stream_video_push_notification/Sources/stream_video_push_notification/StreamVideoPushNotificationPlugin.swift (1)
send(96-105)
⏰ 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: stream_video_push_notification
- GitHub Check: stream_video
- GitHub Check: stream_video_flutter
- GitHub Check: stream_video_noise_cancellation
- GitHub Check: analyze
- GitHub Check: iOS Build (Dogfooding)
- GitHub Check: analyze_legacy_version
- GitHub Check: build
🔇 Additional comments (8)
packages/stream_video_flutter/CHANGELOG.md (1)
1-5: LGTM!The changelog entry accurately documents the fix for the VoIP push crash when received before Flutter initialization, and follows the existing formatting conventions.
packages/stream_video_push_notification/CHANGELOG.md (1)
1-5: LGTM!Changelog entry is consistent with the parallel entry in the stream_video_flutter package and correctly documents the iOS/macOS crash fix.
packages/stream_video_push_notification/ios/stream_video_push_notification/Sources/stream_video_push_notification/StreamVideoCallkitManager.swift (3)
9-9: LGTM!Good use of
static letwith a private initializer to enforce the singleton pattern. This ensures the manager is always available regardless of Flutter's initialization state.
30-43: Well-designed event queuing mechanism.The flush-on-attach pattern correctly ensures events received before Flutter initialization are preserved and delivered in order once the handler is wired up.
59-63: LGTM!The NSDictionary conversion correctly routes Obj-C callers through the unified event handling path with proper nil handling.
packages/stream_video_push_notification/ios/stream_video_push_notification/Sources/stream_video_push_notification/StreamVideoPushNotificationPlugin.swift (3)
5-5: Good extraction of magic string to a constant.This improves maintainability and prevents typos across the getter/setter methods.
18-33: LGTM! Correct initialization order.The registration flow properly wires the event handler to the shared manager before creating the plugin instance, ensuring any queued events from early VoIP pushes are flushed to Flutter once the channel is ready.
65-82: LGTM! Key fix for the VoIP push crash.Static methods now operate directly on
StreamVideoCallkitManager.shared, making them safe to call from PKPushRegistry delegate before Flutter plugin registration completes.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1140 +/- ##
=====================================
Coverage 6.31% 6.31%
=====================================
Files 595 595
Lines 41081 41081
=====================================
Hits 2595 2595
Misses 38486 38486 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
When a VoIP push notification is received, it may be handled before Flutter fully initializes. This causes a null pointer exception when accessing
StreamVideoPushNotificationPlugin.sharedInstance, which is only set during Flutter plugin registration.This PR extracts
StreamVideoCallkitManagerto be a singleton that is always available, even before Flutter initializes so that CallKit calls can be handled regardless of Flutter initialization state. Events that occur before Flutter is ready are queued and delivered once Flutter initializesSummary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.