Fix LivestreamChannelController not reconnecting when connection is dropped
#3782
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughAdds start/stop watching lifecycle APIs to LivestreamChannelController, tracks active livestream controllers in SyncRepository, updates WatchChannelOperation to use LivestreamChannelController and re-enqueue watch operations on reconnect, updates tests and test tooling, and documents the fix in CHANGELOG.md. Changes
Sequence Diagram(s)sequenceDiagram
participant UI as App/UI
participant LCC as LivestreamChannelController
participant SR as SyncRepository
participant CU as ChannelUpdater
UI->>LCC: startWatching(isInRecoveryMode, completion)
alt CID missing
LCC-->>UI: completion(ChannelNotCreatedYet)
else CID present
LCC->>SR: startTrackingLivestreamController(self)
LCC->>CU: startWatching(cid, isInRecoveryMode)
CU-->>LCC: result (success/error)
LCC-->>UI: completion(result) on main
end
sequenceDiagram
participant Net as Network
participant SR as SyncRepository
participant OpQ as Sync Ops Queue
participant LCC as LivestreamChannelController
participant CU as ChannelUpdater
Net-->>SR: Reconnected
SR->>OpQ: enqueue WatchChannelOperation for each tracked LCC
OpQ->>LCC: WatchChannelOperation -> startWatching(isInRecoveryMode=false)
LCC->>CU: startWatching(cid, false)
CU-->>LCC: result
alt success
LCC->>OpQ: mark complete, add cid to watchedAndSynchedChannelIds
else failure
OpQ->>OpQ: retry WatchChannelOperation
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
SDK Size
|
Public Interface public class LivestreamChannelController: DataStoreProvider, EventsControllerDelegate, AppStateObserverDelegate
- public func loadPreviousMessages(before messageId: MessageId? = nil,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func startWatching(isInRecoveryMode: Bool,completion: ((Error?) -> Void)? = nil)
- public func loadNextMessages(after messageId: MessageId? = nil,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func stopWatching(completion: ((Error?) -> Void)? = nil)
- public func loadPageAroundMessageId(_ messageId: MessageId,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func loadPreviousMessages(before messageId: MessageId? = nil,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
- public func loadFirstPage(_ completion: (@MainActor(_ error: Error?) -> Void)? = nil)
+ public func loadNextMessages(after messageId: MessageId? = nil,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
- public func createNewMessage(messageId: MessageId? = nil,text: String,pinning: MessagePinning? = nil,isSilent: Bool = false,attachments: [AnyAttachmentPayload] = [],mentionedUserIds: [UserId] = [],quotedMessageId: MessageId? = nil,skipPush: Bool = false,skipEnrichUrl: Bool = false,restrictedVisibility: [UserId] = [],location: NewLocationInfo? = nil,extraData: [String: RawJSON] = [:],completion: (@MainActor(Result<MessageId, Error>) -> Void)? = nil)
+ public func loadPageAroundMessageId(_ messageId: MessageId,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
- public func deleteMessage(messageId: MessageId,hard: Bool = false,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func loadFirstPage(_ completion: (@MainActor(_ error: Error?) -> Void)? = nil)
- public func loadReactions(for messageId: MessageId,limit: Int = 25,offset: Int = 0,completion: @escaping @MainActor(Result<[ChatMessageReaction], Error>) -> Void)
+ public func createNewMessage(messageId: MessageId? = nil,text: String,pinning: MessagePinning? = nil,isSilent: Bool = false,attachments: [AnyAttachmentPayload] = [],mentionedUserIds: [UserId] = [],quotedMessageId: MessageId? = nil,skipPush: Bool = false,skipEnrichUrl: Bool = false,restrictedVisibility: [UserId] = [],location: NewLocationInfo? = nil,extraData: [String: RawJSON] = [:],completion: (@MainActor(Result<MessageId, Error>) -> Void)? = nil)
- public func flag(messageId: MessageId,reason: String? = nil,extraData: [String: RawJSON]? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func deleteMessage(messageId: MessageId,hard: Bool = false,completion: (@MainActor(Error?) -> Void)? = nil)
- public func unflag(messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func loadReactions(for messageId: MessageId,limit: Int = 25,offset: Int = 0,completion: @escaping @MainActor(Result<[ChatMessageReaction], Error>) -> Void)
- public func addReaction(_ type: MessageReactionType,to messageId: MessageId,score: Int = 1,enforceUnique: Bool = false,skipPush: Bool = false,pushEmojiCode: String? = nil,extraData: [String: RawJSON] = [:],completion: (@MainActor(Error?) -> Void)? = nil)
+ public func flag(messageId: MessageId,reason: String? = nil,extraData: [String: RawJSON]? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
- public func deleteReaction(_ type: MessageReactionType,from messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func unflag(messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
- public func pin(messageId: MessageId,pinning: MessagePinning = .noExpiration,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func addReaction(_ type: MessageReactionType,to messageId: MessageId,score: Int = 1,enforceUnique: Bool = false,skipPush: Bool = false,pushEmojiCode: String? = nil,extraData: [String: RawJSON] = [:],completion: (@MainActor(Error?) -> Void)? = nil)
- public func unpin(messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func deleteReaction(_ type: MessageReactionType,from messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
- public func loadPinnedMessages(pageSize: Int = .messagesPageSize,sorting: [Sorting<PinnedMessagesSortingKey>] = [],pagination: PinnedMessagesPagination? = nil,completion: @escaping @MainActor(Result<[ChatMessage], Error>) -> Void)
+ public func pin(messageId: MessageId,pinning: MessagePinning = .noExpiration,completion: (@MainActor(Error?) -> Void)? = nil)
- public func currentCooldownTime()-> Int
+ public func unpin(messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
- public func enableSlowMode(cooldownDuration: Int,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func loadPinnedMessages(pageSize: Int = .messagesPageSize,sorting: [Sorting<PinnedMessagesSortingKey>] = [],pagination: PinnedMessagesPagination? = nil,completion: @escaping @MainActor(Result<[ChatMessage], Error>) -> Void)
- public func disableSlowMode(completion: (@MainActor(Error?) -> Void)? = nil)
+ public func currentCooldownTime()-> Int
- public func pause()
+ public func enableSlowMode(cooldownDuration: Int,completion: (@MainActor(Error?) -> Void)? = nil)
- public func resume(completion: (@MainActor(Error?) -> Void)? = nil)
+ public func disableSlowMode(completion: (@MainActor(Error?) -> Void)? = nil)
- public func eventsController(_ controller: EventsController,didReceiveEvent event: Event)
+ public func pause()
- public func applicationDidReceiveMemoryWarning()
+ public func resume(completion: (@MainActor(Error?) -> Void)? = nil)
- public func applicationDidMoveToForeground()
+ public func eventsController(_ controller: EventsController,didReceiveEvent event: Event)
+ public func applicationDidReceiveMemoryWarning()
+ public func applicationDidMoveToForeground() |
![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 (9)
Sources/StreamChat/Repositories/SyncOperations.swift (1)
165-186: Add acidpresence guard to avoid noisy retries when channel isn’t created yet.Currently, if
cidis nil,startWatchingwill fail and the op will retry up to the max. Guarding early reduces logs and unnecessary retries.Apply this diff:
init(livestreamController: LivestreamChannelController, context: SyncContext, recovery: Bool) { super.init(maxRetries: syncOperationsMaximumRetries) { [weak livestreamController] _, done in - guard let controller = livestreamController else { + guard let controller = livestreamController else { done(.continue) return } - let cidString = (controller.cid?.rawValue ?? "unknown") + guard let cid = controller.cid else { + done(.continue) + return + } + let cidString = cid.rawValue log.info("Watching active channel \(cidString)", subsystems: .offlineSupport) controller.startWatching(isInRecoveryMode: recovery) { error in - if let cid = controller.cid, error == nil { + if error == nil { log.info("Successfully watched active channel \(cidString)", subsystems: .offlineSupport) context.watchedAndSynchedChannelIds.insert(cid) done(.continue) } else { let errorMessage = error?.localizedDescription ?? "missing cid" log.error("Failed watching active channel \(cidString): \(errorMessage)", subsystems: .offlineSupport) done(.retry) } } } }Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (2)
230-249: Clarify docstring and keep the main-thread completion behavior.API shape looks good. Recommend reflecting the
isInRecoveryModeparameter in the documentation for easier discoverability.Apply this doc tweak:
- /// Start watching a channel - /// - /// - Parameter completion: Called when the API call is finished. Called with `Error` if the remote update fails. + /// Starts watching the channel. + /// + /// - Parameters: + /// - isInRecoveryMode: When true, the request is routed via the recovery client to align with reconnection flows. + /// - completion: Called on the main thread when the API call finishes. Receives an `Error` if the remote update fails.
251-270: Docs: mirror behavior and side effects.The implementation also stops tracking in the sync repository. It’s worth stating this explicitly in the docs to set expectations.
Apply this doc tweak:
- /// Stop watching a channel - /// - /// - Parameter completion: Called when the API call is finished. Called with `Error` if the remote update fails. + /// Stops watching the channel. + /// + /// Also removes this controller from the SyncRepository’s active livestream controllers. + /// + /// - Parameter completion: Called on the main thread when the API call finishes. Receives an `Error` if the remote update fails.Additionally, consider proactively untracking on deinit to keep the collection tidy (it’s weak, so this is optional):
// in deinit client.syncRepository.stopTrackingLivestreamController(self)Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift (4)
2274-2287: Avoid flakiness: ThreadSafeWeakCollection writes are async — assert with eventual consistencyThreadSafeWeakCollection.add/remove use queue.async barriers. Immediate reads can race in tests. Prefer AssertAsync to wait until the mutation is observed.
Apply this diff:
- XCTAssert(controller.client === client) - XCTAssert(client.syncRepository.activeLivestreamControllers.count == 1) - XCTAssert(client.syncRepository.activeLivestreamControllers.allObjects.first === controller) + XCTAssert(controller.client === client) + AssertAsync.willBeEqual(client.syncRepository.activeLivestreamControllers.count, 1) + AssertAsync.willBeTrue(client.syncRepository.activeLivestreamControllers.contains(controller))
2289-2303: Same flakiness concern for startWatching tracking assertionsUse AssertAsync to avoid racing the async barrier write in ThreadSafeWeakCollection.
Apply this diff:
- XCTAssert(controller.client === client) - XCTAssert(client.syncRepository.activeLivestreamControllers.count == 1) - XCTAssert(client.syncRepository.activeLivestreamControllers.allObjects.first === controller) + XCTAssert(controller.client === client) + AssertAsync.willBeEqual(client.syncRepository.activeLivestreamControllers.count, 1) + AssertAsync.willBeTrue(client.syncRepository.activeLivestreamControllers.contains(controller))
2304-2317: Use eventual assertions for both add and remove tracking pathsBoth startTracking and stopTracking are async; assert with eventual consistency for stability. Also, prefer XCTAssertTrue over “== true” for readability.
Apply this diff:
- controller.synchronize() - XCTAssert(client.syncRepository.activeLivestreamControllers.count == 1) + controller.synchronize() + AssertAsync.willBeEqual(client.syncRepository.activeLivestreamControllers.count, 1) - controller.stopWatching() - XCTAssert(client.syncRepository.activeLivestreamControllers.allObjects.isEmpty == true) + controller.stopWatching() + AssertAsync.willBeTrue(client.syncRepository.activeLivestreamControllers.allObjects.isEmpty)Optionally, you can also assert via the collection API:
AssertAsync.willBeTrue(client.syncRepository.activeLivestreamControllers.contains(controller) == false)
2085-2269: Consider adding a negative-path test for missing cidSmall gap: startWatching/stopWatching guard on cid and complete with ClientError.ChannelNotCreatedYet(). Adding a test ensures error propagation remains stable.
Would you like me to add a test like:
- test_startWatching_withoutCid_callsCompletionWithChannelNotCreatedYet
- test_stopWatching_withoutCid_callsCompletionWithChannelNotCreatedYet
Sources/StreamChat/Repositories/SyncRepository.swift (2)
201-210: Potential behavior nuance: tracking on synchronize() means “rewatch” even if never explicitly watchedLivestreamChannelController.synchronize() invokes startTrackingLivestreamController(self). This means any synchronized livestream controller will be re-watched after a reconnect, even if startWatching was never called. If this is the intended parity with ChatChannelController, all good; otherwise, consider tracking only when the controller is actively “watching” (startWatching), and untracking on stopWatching.
40-45: Note on eventual consistency of ThreadSafeWeakCollection in production pathsThreadSafeWeakCollection.add/remove use async barriers. If a syncLocalState run happens immediately after a tracking call on another thread, the newly tracked object might miss that cycle. This matches the existing design for other trackers; just calling it out for awareness. If you want stricter guarantees here, a sync write or an explicit barrier drain would be needed, but that’s a broader pattern change.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (7)
CHANGELOG.md(1 hunks)Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift(1 hunks)Sources/StreamChat/Repositories/SyncOperations.swift(1 hunks)Sources/StreamChat/Repositories/SyncRepository.swift(5 hunks)TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelUpdater_Mock.swift(3 hunks)Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift(1 hunks)Tests/StreamChatTests/Repositories/SyncRepository_Tests.swift(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (6)
Sources/StreamChat/Repositories/SyncRepository.swift (2)
startTrackingLivestreamController(94-97)stopTrackingLivestreamController(99-101)Tests/StreamChatTests/APIClient/Endpoints/ChannelEndpoints_Tests.swift (1)
channelQuery(48-52)TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelUpdater_Mock.swift (2)
startWatching(527-532)stopWatching(534-538)Sources/StreamChat/Controllers/ChannelController/ChannelController.swift (2)
startWatching(1430-1445)stopWatching(1464-1479)Sources/StreamChat/Workers/ChannelUpdater.swift (4)
startWatching(621-631)startWatching(1019-1025)stopWatching(640-644)stopWatching(1027-1033)Sources/StreamChat/StateLayer/Chat.swift (1)
stopWatching(118-121)
Tests/StreamChatTests/Repositories/SyncRepository_Tests.swift (1)
Sources/StreamChat/Repositories/SyncRepository.swift (5)
startTrackingLivestreamController(94-97)stopTrackingLivestreamController(99-101)startTrackingChannelController(85-88)startTrackingChannelListController(112-115)removeAllTracked(121-127)
Sources/StreamChat/Repositories/SyncOperations.swift (2)
Sources/StreamChat/Utils/Logger/Logger.swift (1)
info(323-338)Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (1)
startWatching(233-249)
Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift (3)
TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelUpdater_Mock.swift (3)
startWatching(527-532)cleanUp(161-300)stopWatching(534-538)Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (3)
startWatching(233-249)stopWatching(254-270)synchronize(215-228)Sources/StreamChat/Workers/ChannelUpdater.swift (4)
startWatching(621-631)startWatching(1019-1025)stopWatching(640-644)stopWatching(1027-1033)
Sources/StreamChat/Repositories/SyncRepository.swift (1)
Sources/StreamChat/Utils/ThreadSafeWeakCollection.swift (4)
contains(45-51)add(27-31)remove(33-37)removeAllObjects(39-43)
⏰ 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). (2)
- GitHub Check: Build Test App and Frameworks
- GitHub Check: Metrics
🔇 Additional comments (13)
CHANGELOG.md (1)
6-8: CHANGELOG entry reads well and matches the PR intent.The new “Fixed” entry under StreamChat clearly states the reconnection fix and links to the PR. No issues spotted.
TestTools/StreamChatTestTools/Mocks/StreamChat/Workers/ChannelUpdater_Mock.swift (3)
120-123: Good addition to capture recovery mode flag in tests.Recording
startWatching_isInRecoveryModeunder@Atomicis appropriate and thread-safe for assertions.
261-263: Cleanup covers the new test hook.Resetting
startWatching_isInRecoveryModeincleanUp()prevents test cross-contamination.
526-532: Override correctly records parameters and forwards completion.The mock’s
startWatchingoverride capturescid,isInRecoveryMode, stores the completion, and triggers the canned result. This enables precise test control.Tests/StreamChatTests/Repositories/SyncRepository_Tests.swift (1)
446-503: Solid coverage for livestream controller tracking lifecycle.The tests verify add, idempotency, remove, and inclusion in
removeAllTracked(). This guards the new SyncRepository APIs against regressions.Sources/StreamChat/Repositories/SyncOperations.swift (1)
141-163: Summary discrepancy: legacy ChatChannelController watch path still present.AI summary claims the previous
recoverWatchedChannel(recovery:)path was removed, but this initializer still uses it. If intentional (to keep channel controller recovery alongside livestream), consider updating the summary. If not, remove this initializer to avoid divergent paths.Would you like me to scan SyncRepository for call sites to confirm both initializers are used as intended?
Sources/StreamChat/Controllers/ChannelController/LivestreamChannelController.swift (1)
222-223: Tracking livestream controllers during synchronize is correct.Ensures the repository is aware of active livestream controllers before network work kicks off.
Tests/StreamChatTests/Controllers/ChannelController/LivestreamChannelController_Tests.swift (2)
2087-2161: Start watching tests validate both normal and recovery flows — looks goodThe tests correctly assert the updater receives cid and the isInRecoveryMode flag, and that errors are surfaced. The API surface aligns with ChannelUpdater and the delegate call sites.
2198-2269: Stop watching tests cover success and failure — looks goodThe tests correctly validate that stopWatching calls through to the updater and propagates errors.
Sources/StreamChat/Repositories/SyncRepository.swift (4)
43-45: Tracking livestream controllers — aligns with existing tracking patternAdding activeLivestreamControllers mirrors existing active* trackers and uses a weak, thread-safe collection. This is the right choice to avoid retention and enable re-watch orchestration.
94-101: start/stop tracking APIs are consistent and idempotent — goodGuarding with contains before add matches other trackers and keeps operations stable.
121-127: Include livestream controllers in cleanup — goodremoveAllTracked now clears activeLivestreamControllers too; prevents stale state across sessions.
201-210: Re-watch wiring includes livestream controllers — confirm non-recovery path is intendedAppending WatchChannelOperation(livestreamController:…, recovery: false) mirrors channel controllers’ behavior. Given this is part of the “Background mode operations” (post-recovery), using a regular request is likely correct.
Action: please confirm WatchChannelOperation for livestreams switches to recovery requests only in the “Recovery mode operations” stage (if applicable), and that it does not create duplicate watch attempts when both a Chat and a Livestream controller point to the same cid.
SDK Performance
|
Generated by 🚫 Danger |
SDK Size
|
Public Interface public class LivestreamChannelController: DataStoreProvider, EventsControllerDelegate, AppStateObserverDelegate
- public func loadPreviousMessages(before messageId: MessageId? = nil,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func startWatching(isInRecoveryMode: Bool,completion: ((Error?) -> Void)? = nil)
- public func loadNextMessages(after messageId: MessageId? = nil,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func stopWatching(completion: ((Error?) -> Void)? = nil)
- public func loadPageAroundMessageId(_ messageId: MessageId,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func loadPreviousMessages(before messageId: MessageId? = nil,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
- public func loadFirstPage(_ completion: (@MainActor(_ error: Error?) -> Void)? = nil)
+ public func loadNextMessages(after messageId: MessageId? = nil,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
- public func createNewMessage(messageId: MessageId? = nil,text: String,pinning: MessagePinning? = nil,isSilent: Bool = false,attachments: [AnyAttachmentPayload] = [],mentionedUserIds: [UserId] = [],quotedMessageId: MessageId? = nil,skipPush: Bool = false,skipEnrichUrl: Bool = false,restrictedVisibility: [UserId] = [],location: NewLocationInfo? = nil,extraData: [String: RawJSON] = [:],completion: (@MainActor(Result<MessageId, Error>) -> Void)? = nil)
+ public func loadPageAroundMessageId(_ messageId: MessageId,limit: Int? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
- public func deleteMessage(messageId: MessageId,hard: Bool = false,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func loadFirstPage(_ completion: (@MainActor(_ error: Error?) -> Void)? = nil)
- public func loadReactions(for messageId: MessageId,limit: Int = 25,offset: Int = 0,completion: @escaping @MainActor(Result<[ChatMessageReaction], Error>) -> Void)
+ public func createNewMessage(messageId: MessageId? = nil,text: String,pinning: MessagePinning? = nil,isSilent: Bool = false,attachments: [AnyAttachmentPayload] = [],mentionedUserIds: [UserId] = [],quotedMessageId: MessageId? = nil,skipPush: Bool = false,skipEnrichUrl: Bool = false,restrictedVisibility: [UserId] = [],location: NewLocationInfo? = nil,extraData: [String: RawJSON] = [:],completion: (@MainActor(Result<MessageId, Error>) -> Void)? = nil)
- public func flag(messageId: MessageId,reason: String? = nil,extraData: [String: RawJSON]? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func deleteMessage(messageId: MessageId,hard: Bool = false,completion: (@MainActor(Error?) -> Void)? = nil)
- public func unflag(messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func loadReactions(for messageId: MessageId,limit: Int = 25,offset: Int = 0,completion: @escaping @MainActor(Result<[ChatMessageReaction], Error>) -> Void)
- public func addReaction(_ type: MessageReactionType,to messageId: MessageId,score: Int = 1,enforceUnique: Bool = false,skipPush: Bool = false,pushEmojiCode: String? = nil,extraData: [String: RawJSON] = [:],completion: (@MainActor(Error?) -> Void)? = nil)
+ public func flag(messageId: MessageId,reason: String? = nil,extraData: [String: RawJSON]? = nil,completion: (@MainActor(Error?) -> Void)? = nil)
- public func deleteReaction(_ type: MessageReactionType,from messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func unflag(messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
- public func pin(messageId: MessageId,pinning: MessagePinning = .noExpiration,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func addReaction(_ type: MessageReactionType,to messageId: MessageId,score: Int = 1,enforceUnique: Bool = false,skipPush: Bool = false,pushEmojiCode: String? = nil,extraData: [String: RawJSON] = [:],completion: (@MainActor(Error?) -> Void)? = nil)
- public func unpin(messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func deleteReaction(_ type: MessageReactionType,from messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
- public func loadPinnedMessages(pageSize: Int = .messagesPageSize,sorting: [Sorting<PinnedMessagesSortingKey>] = [],pagination: PinnedMessagesPagination? = nil,completion: @escaping @MainActor(Result<[ChatMessage], Error>) -> Void)
+ public func pin(messageId: MessageId,pinning: MessagePinning = .noExpiration,completion: (@MainActor(Error?) -> Void)? = nil)
- public func currentCooldownTime()-> Int
+ public func unpin(messageId: MessageId,completion: (@MainActor(Error?) -> Void)? = nil)
- public func enableSlowMode(cooldownDuration: Int,completion: (@MainActor(Error?) -> Void)? = nil)
+ public func loadPinnedMessages(pageSize: Int = .messagesPageSize,sorting: [Sorting<PinnedMessagesSortingKey>] = [],pagination: PinnedMessagesPagination? = nil,completion: @escaping @MainActor(Result<[ChatMessage], Error>) -> Void)
- public func disableSlowMode(completion: (@MainActor(Error?) -> Void)? = nil)
+ public func currentCooldownTime()-> Int
- public func pause()
+ public func enableSlowMode(cooldownDuration: Int,completion: (@MainActor(Error?) -> Void)? = nil)
- public func resume(completion: (@MainActor(Error?) -> Void)? = nil)
+ public func disableSlowMode(completion: (@MainActor(Error?) -> Void)? = nil)
- public func eventsController(_ controller: EventsController,didReceiveEvent event: Event)
+ public func pause()
- public func applicationDidReceiveMemoryWarning()
+ public func resume(completion: (@MainActor(Error?) -> Void)? = nil)
- public func applicationDidMoveToForeground()
+ public func eventsController(_ controller: EventsController,didReceiveEvent event: Event)
+ public func applicationDidReceiveMemoryWarning()
+ public func applicationDidMoveToForeground() |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
TestTools/StreamChatTestTools/SpyPattern/Spy/LivestreamChannelController_Spy.swift (2)
9-11: Capture and assert the isInRecoveryMode flag in the spyRight now, the spy doesn’t expose whether startWatching was called with recovery=true/false, so tests can’t assert parameter forwarding. Capture it for stronger test guarantees.
Apply this diff:
@@ - var startWatchingError: Error? - let spyState = SpyState() + var startWatchingError: Error? + let spyState = SpyState() + // Captures the last flag passed to `startWatching` for assertions + var startWatching_isInRecoveryMode: Bool? @@ override func startWatching(isInRecoveryMode: Bool, completion: ((Error?) -> Void)? = nil) { record() + startWatching_isInRecoveryMode = isInRecoveryMode completion?(startWatchingError) }Also applies to: 16-19
12-14: Make CID injectable for targeted test scenarios (backward compatible)Accept an optional cid to simplify deterministic assertions (e.g., when coordinating with pre-seeded DB state). Default keeps current behavior.
- init(client: ChatClient_Mock) { - super.init(channelQuery: .init(cid: .unique), client: client) - } + init(client: ChatClient_Mock, cid: ChannelId = .unique) { + super.init(channelQuery: .init(cid: cid), client: client) + }Tests/StreamChatTests/Repositories/SyncOperations_Tests.swift (2)
164-176: Assert parameter forwarding of isInRecoveryMode in success pathNow that the spy can capture the flag, assert it to ensure WatchChannelOperation forwards the recovery parameter correctly.
XCTAssertEqual(context.watchedAndSynchedChannelIds.count, 1) XCTAssertTrue(context.watchedAndSynchedChannelIds.contains(controller.cid!)) XCTAssertCall("startWatching(isInRecoveryMode:completion:)", on: controller) + XCTAssertEqual(controller.startWatching_isInRecoveryMode, true)
178-189: Also assert the isInRecoveryMode flag in failure/retry path and add a recovery-failure test
- Validate the forwarded flag when recovery=false.
- Consider adding a dedicated test for recovery=true + failure to assert there’s no retry.
Apply this diff to strengthen the existing test:
XCTAssertEqual(context.watchedAndSynchedChannelIds.count, 0) XCTAssertCall("startWatching(isInRecoveryMode:completion:)", on: controller, times: 3) + XCTAssertEqual(controller.startWatching_isInRecoveryMode, false)Optionally add the following new test to cover the recovery=true failure path (no retries expected). I can open a follow-up PR if helpful:
func test_WatchChannelOperation_livestreamController_startWatchingFailure_inRecovery_shouldNotRetry() { let context = SyncContext(lastSyncAt: .init()) let controller = LivestreamChannelController_Spy(client: client) controller.startWatchingError = ClientError("Watch failed") let operation = WatchChannelOperation(livestreamController: controller, context: context, recovery: true) operation.startAndWaitForCompletion() XCTAssertEqual(context.watchedAndSynchedChannelIds.count, 0) XCTAssertCall("startWatching(isInRecoveryMode:completion:)", on: controller, times: 1) XCTAssertEqual(controller.startWatching_isInRecoveryMode, true) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
StreamChat.xcodeproj/project.pbxproj(4 hunks)TestTools/StreamChatTestTools/SpyPattern/Spy/LivestreamChannelController_Spy.swift(1 hunks)Tests/StreamChatTests/Repositories/SyncOperations_Tests.swift(1 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). (1)
- GitHub Check: Metrics
🔇 Additional comments (7)
StreamChat.xcodeproj/project.pbxproj (6)
1442-1442: LivestreamChannelController_Spy.swift build file entry looks correctPBXBuildFile entry is well-formed and references the matching PBXFileReference ID.
4309-4309: PBXFileReference for LivestreamChannelController_Spy.swift is correctReference is consistent (sourcecode.swift, path-only) and will resolve under the Spy group.
8475-8475: Added spy to the Spy group — good organizationThe file is grouped alongside other test doubles, keeping test utilities discoverable.
4310-4312: File references are correct—DTO files still exist
TheThreadDTO.swift,ThreadParticipantDTO.swift, andThreadReadDTO.swiftfiles are present on disk underSources/StreamChat/Database/DTOs/. Keeping their PBXFileReference entries is appropriate; no removals are needed.Likely an incorrect or invalid review comment.
1443-1445: Thread DTO build-file entries are valid; files still existThe PBXBuildFile entries for ThreadDTO.swift, ThreadParticipantDTO.swift, and ThreadReadDTO.swift should remain, as these files are present under
Sources/StreamChat/Database/DTOs. No removals are needed inStreamChat.xcodeproj/project.pbxproj. Instead, please update the AI summary to stop claiming these DTO files were deleted.• No changes required in project.pbxproj
• Update the summary to reflect that Thread DTO files remain in the codebaseLikely an incorrect or invalid review comment.
11490-11490: No targets include LivestreamChannelController_Spy.swiftConfirmed via PBXSourcesBuildPhase mapping that
LivestreamChannelController_Spy.swiftisn’t added to any production target (or any target). No action needed here.Tests/StreamChatTests/Repositories/SyncOperations_Tests.swift (1)
145-162: Good coverage: verifies the operation doesn’t retain the controllerThis effectively guards against accidental strong captures inside WatchChannelOperation. No changes needed.
|
🔗 Issue Links
https://linear.app/stream/issue/IOS-1079/fix-livestreamchannelcontroller-not-reconnecting-when-connection-is
🎯 Goal
Fix
LivestreamChannelControllernot reconnecting when the connection is dropped.🛠 Implementation
Sync operation was not tracking livestream controllers.
🧪 Manual Testing Notes
One way to test this is by creating a breakpoint while debugging, and wait for a couple of minutes.
When doing so, sending messages and receiving messages should still work.
☑️ Contributor Checklist
docs-contentrepoSummary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation