Compatibility changes for StreamChat migration #29
Conversation
Generated by 🚫 Danger |
laevandus
force-pushed
the
chat-web-socket-migration
branch
2 times, most recently
from
fe498fe to
af22b77
Compare
laevandus
force-pushed
the
chat-web-socket-migration
branch
from
af22b77 to
a2d531b
Compare
| public var isInvalidTokenError: Bool { | ||
| ClosedRange.tokenInvalidErrorCodes ~= code || code == StreamErrorCode.accessKeyInvalid | ||
| } |
There was a problem hiding this comment.
From Chat's implementation
| /// - Returns: Gzip-compressed `Data` instance. | ||
| /// - Throws: `GzipError` | ||
| public func gzipped() throws -> Data { | ||
| func gzipped() throws -> Data { |
There was a problem hiding this comment.
No need to be public anymore (previous PR did that)
| /// Basically, it's a wrapper over legacy monitor based on `Reachability` (iOS 11 only) | ||
| /// and default monitor based on `Network`.`NWPathMonitor` (iOS 12+). | ||
| public final class InternetConnection: @unchecked Sendable { | ||
| open class InternetConnection: @unchecked Sendable { |
There was a problem hiding this comment.
Test tools need to subclass it on the chat side
| import Foundation | ||
|
|
||
| public protocol StreamTimer { | ||
| public protocol TimerScheduling { |
There was a problem hiding this comment.
Naming conflict: did not know there was StreamTimer in the UIKit module and Timer was conflicting with Foundation.
|
|
||
| @MainActor | ||
| public func beginTask(expirationHandler: (@Sendable () -> Void)?) -> Bool { | ||
| endTask() |
| public protocol ConnectionRecoveryHandler: ConnectionStateDelegate, Sendable { | ||
| func start() | ||
| func stop() | ||
| } |
There was a problem hiding this comment.
Chat needs this to support some flows
| keepConnectionAliveInBackground: keepConnectionAliveInBackground, | ||
| reconnectionPolicies: [ | ||
| WebSocketAutomaticReconnectionPolicy(webSocketClient), | ||
| InternetAvailabilityReconnectionPolicy(internetConnection), |
There was a problem hiding this comment.
This is interesting, I struggled with reconnection when building and running demo app and as soon as I dropped this, things started to work. I spent a day on this, so not planning to spend more time on this. Chat's recovery handler also did not check for internet connection (makes sense since the monitor is not 100% accurate and URLRequest can trigger connection establishment even when the monitor says there is no connection. This is also what Apple suggest (can't remember which WWDC on top of my head))
| Task { @MainActor in | ||
| StreamConcurrency.onMain { |
There was a problem hiding this comment.
Task creates delay whereas onMain does not. Which can be important when scheduling background tasks (really small time window for scheduling one)
| let isWaitingForReconnect = webSocketConnectionState.isAutomaticReconnectionEnabled || source.serverError? | ||
| .isInvalidTokenError == true | ||
|
|
||
| let isWaitingForReconnect = webSocketConnectionState.isAutomaticReconnectionEnabled |
| case userInitiated | ||
|
|
||
| /// The connection timed out while trying to connect. | ||
| case timeout(from: WebSocketConnectionState) |
There was a problem hiding this comment.
Chat needs this state for special reconnection flows
| public weak var connectionStateDelegate: ConnectionStateDelegate? | ||
|
|
||
| public var connectURL: URL | ||
| public var connectRequest: URLRequest? |
There was a problem hiding this comment.
Chat used endpoint type which in the end was turned into URLRequest. Since WS engine needs URLRequest anyway so it makes sense to change this around. Also Chat sets this just before connecting URL, therefore nil is needed.
| public func initialize() { | ||
| connectionState = .initialized | ||
| } |
There was a problem hiding this comment.
Chat needs this for its special reconnection flows
| } | ||
|
|
||
| if let date = self?.iso8601formatter.date(from: dateString) { | ||
| if let date = self?.iso8601formatter.dateWithMicroseconds(from: dateString) { |
There was a problem hiding this comment.
Very important for chat
|
🔗 Issue Links
Related: IOS-1205
🎯 Goal
Required changes for StreamChat migration for adopting web-socket client and remaining types
🛠 Implementation
EventNotificationCenteris now a protocol withDefaultEventNotificationCenter. This is required for chat to inject its own version (one which writes to DB)WebSocketClientchanges for accommodate chat's requirements (e.g. using URLRequest and setting nil when creating the client)☑️ Contributor Checklist
docs-contentrepo