[Enhancement]Migrate sdk signaling core infra onto stream core swift#1178
Open
ipavlidakis wants to merge 29 commits into
Open
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughStreamVideo integrates StreamCore for package dependencies, errors, logging, coordinator signaling, and SFU signaling. Coordinator and SFU WebSocket flows now use StreamCore-backed wrappers, typed event conversion, published connection states, updated recovery handling, and revised test mocks. ChangesStreamCore Migration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
![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 (6)
StreamVideoTests/WebSocketClient/CoordinatorWebSocket_Tests.swift (1)
14-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRename tests to the required scenario pattern.
These test names do not follow the repository’s
test_<given>_<when>_<then>_()convention.Proposed rename
-func test_map_initializedConnectingAuthenticating() { +func test_streamCoreStates_mapped_preservesEquivalentStates() { -func test_map_connected_carriesConnectionId() { +func test_connectedState_mapped_carriesConnectionId() { -func test_map_disconnectionSources() { +func test_disconnectionSources_mapped_preservesEquivalentSources() {As per coding guidelines, “Name test methods with the pattern
test_<given>_<when>_<then>_().”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@StreamVideoTests/WebSocketClient/CoordinatorWebSocket_Tests.swift` around lines 14 - 49, The test methods in CoordinatorWebSocket_Tests.swift need to be renamed to match the repository’s required test naming convention. Update the existing tests around VideoWebSocketConnectionState mapping so each method follows the test_<given>_<when>_<then>_ pattern, using the current scenarios in test_map_initializedConnectingAuthenticating, test_map_connected_carriesConnectionId, and test_map_disconnectionSources as the basis for the new names.Source: Coding guidelines
Sources/StreamVideo/WebSockets/Client/WebSocketInternalEvents.swift (1)
18-20: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConstant naming diverges from lowerCamelCase convention.
static let NewEventReceiveduses UpperCamelCase; per guidelines, constants should use lowerCamelCase (e.g.newEventReceived).As per coding guidelines,
**/*.swift: "Use lowerCamelCase for method and variable names, and for constants".✏️ Suggested rename (verify no external call-site breakage first)
extension Notification.Name { - static let NewEventReceived = Notification.Name("io.getStream.video.core.new_event_received") + static let newEventReceived = Notification.Name("io.getStream.video.core.new_event_received") }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/StreamVideo/WebSockets/Client/WebSocketInternalEvents.swift` around lines 18 - 20, Rename the Notification.Name constant in the Notification.Name extension from NewEventReceived to lowerCamelCase to match Swift naming conventions, and update any references to this symbol throughout the codebase; keep the raw notification string unchanged so WebSocketInternalEvents and any observers continue to use the same event name.Source: Coding guidelines
Sources/StreamVideo/Utils/Logger/Logger+WebRTC.swift (1)
34-39: 🎯 Functional Correctness | 🔵 TrivialAcknowledge the WebRTC log-severity sync regression.
The TODO correctly flags that
severityis now seeded once at init fromStreamCore.LogConfig.leveland no longer tracks subsequent level changes, since StreamCore'sLogConfighas nodidSethook. This is a real (if narrow) behavior regression versus the pre-migration implementation.Do you want this tracked as a follow-up issue against
stream-core-swiftto expose a level-change hook, so syncing can be restored?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/StreamVideo/Utils/Logger/Logger`+WebRTC.swift around lines 34 - 39, The WebRTC log-severity sync in Logger+WebRTC.swift is now only initialized from StreamCore.LogConfig.level and no longer updates when the log level changes. Keep the TODO in place, but add a follow-up to track restoring this behavior once StreamCore exposes a level-change hook; reference the severity static property and the StreamCore.LogConfig integration so it’s clear this is a regression to be fixed in stream-core-swift.Package.swift (1)
26-30: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider tighter pinning for the pre-1.0
stream-core-swiftdependency.
.package(url:from:)resolves to.upToNextMajor(from:), sofrom: "0.8.0"permits any future0.x.yrelease up to (excluding)1.0.0. For a pre-1.0 SDK, minor bumps can carry breaking changes per semver convention, and other dependencies in this project (e.g.stream-video-swift-webrtc,sentry-cocoa) already useexactVersionfor tighter control.♻️ Suggested tighter pinning
- .package(url: "https://github.com/GetStream/stream-core-swift.git", from: "0.8.0") + .package(url: "https://github.com/GetStream/stream-core-swift.git", exact: "0.8.0")🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Package.swift` around lines 26 - 30, The `Package.swift` dependency declaration for `stream-core-swift` is too loose for a pre-1.0 package because `from:` allows any future `0.x.y` release. Tighten the pinning in the package manifest by changing the `stream-core-swift` entry in the dependency list to a fixed version (matching the project’s stricter dependency style used by `stream-video-swift-webrtc` and `sentry-cocoa`) so updates are intentional and controlled.Sources/StreamVideo/WebRTC/v2/SFU/SFUWebSocket.swift (1)
94-97: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSilent message drop when
engineis nil.
webSocket.engine?.send(message:)silently no-ops ifenginehasn't been established yet. Combined withSFUAdapter.statusCheck()only asserting (not blocking) before callingsend/sendHealthCheck, a health check, join, or leave request issued in this window fails with no log or signal back to the caller.🛡️ Suggested fix: log when the engine is unavailable
func send(_ message: any StreamCore.SendableEvent) { - webSocket.engine?.send(message: message) + guard let engine = webSocket.engine else { + log.warning("Attempted to send \(type(of: message)) with no active SFU engine.", subsystems: .sfu) + return + } + engine.send(message: message) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Sources/StreamVideo/WebRTC/v2/SFU/SFUWebSocket.swift` around lines 94 - 97, The SFUWebSocket.send(_:), and any similar sendHealthCheck/send paths, currently drop messages silently when webSocket.engine is nil. Update the send logic to detect the unavailable engine case, log a clear error or warning with context, and avoid silently no-oping; if needed, propagate a failure back through SFUAdapter.statusCheck() callers so join/leave/health check requests get a signal instead of disappearing.StreamVideoTests/Mock/MockSFUStack.swift (1)
12-22: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winClarify that
setConnectionState/receiveEventalways target the initial socket, notnextWebSocket.
webSocketis fixed atinit()and never updated whennextWebSocket(i.e.,box.socket) is reassigned, sosetConnectionState/receiveEventalways drive the original mock socket. Current tests correctly route around this by calling.simulate/.receivedirectly on the new socket after a refresh, but nothing here signals that constraint to future test authors — a natural mistake (callingsfuStack.setConnectionState(...)post-refresh expecting it to hit the active socket) would silently target a stale mock.Consider a short doc comment on
setConnectionState/receiveEventmirroring the one already onnextWebSocket.📝 Suggested doc comments
+ /// Drives connection-state updates on the *original* socket returned by + /// `webSocket`. After assigning `nextWebSocket` to simulate a refresh, + /// interact with that new mock instance directly instead. func setConnectionState(to state: WebSocketConnectionState) { webSocket.simulate(state: .init(webSocketConnectionState: state)) } + /// Delivers SFU events on the *original* socket returned by `webSocket`. + /// After assigning `nextWebSocket` to simulate a refresh, call `.receive` + /// on that new mock instance directly instead. func receiveEvent(_ event: WrappedEvent) { if case let .sfuEvent(payload) = event { webSocket.receive(payload) } }Also applies to: 40-48
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@StreamVideoTests/Mock/MockSFUStack.swift` around lines 12 - 22, Add a short doc comment on MockSFUStack’s setConnectionState and receiveEvent methods to clarify that they always act on the original webSocket created at init, not on nextWebSocket/box.socket after a refresh. Reference the existing nextWebSocket property and the setConnectionState/receiveEvent helpers so future test authors understand they must call simulate/receive on the refreshed MockSFUWebSocket directly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Sources/StreamVideo/StreamVideo.swift`:
- Around line 565-567: The connection state updates are being handled off the
main thread in StreamVideo.connectionStateCancellable, while
handleConnectionStateChange mutates the `@Published` State.connection. Update the
connectionStatePublisher chain to hop onto DispatchQueue.main before sink so the
state change path stays on the main queue and remains safe for UI observation.
---
Nitpick comments:
In `@Package.swift`:
- Around line 26-30: The `Package.swift` dependency declaration for
`stream-core-swift` is too loose for a pre-1.0 package because `from:` allows
any future `0.x.y` release. Tighten the pinning in the package manifest by
changing the `stream-core-swift` entry in the dependency list to a fixed version
(matching the project’s stricter dependency style used by
`stream-video-swift-webrtc` and `sentry-cocoa`) so updates are intentional and
controlled.
In `@Sources/StreamVideo/Utils/Logger/Logger`+WebRTC.swift:
- Around line 34-39: The WebRTC log-severity sync in Logger+WebRTC.swift is now
only initialized from StreamCore.LogConfig.level and no longer updates when the
log level changes. Keep the TODO in place, but add a follow-up to track
restoring this behavior once StreamCore exposes a level-change hook; reference
the severity static property and the StreamCore.LogConfig integration so it’s
clear this is a regression to be fixed in stream-core-swift.
In `@Sources/StreamVideo/WebRTC/v2/SFU/SFUWebSocket.swift`:
- Around line 94-97: The SFUWebSocket.send(_:), and any similar
sendHealthCheck/send paths, currently drop messages silently when
webSocket.engine is nil. Update the send logic to detect the unavailable engine
case, log a clear error or warning with context, and avoid silently no-oping; if
needed, propagate a failure back through SFUAdapter.statusCheck() callers so
join/leave/health check requests get a signal instead of disappearing.
In `@Sources/StreamVideo/WebSockets/Client/WebSocketInternalEvents.swift`:
- Around line 18-20: Rename the Notification.Name constant in the
Notification.Name extension from NewEventReceived to lowerCamelCase to match
Swift naming conventions, and update any references to this symbol throughout
the codebase; keep the raw notification string unchanged so
WebSocketInternalEvents and any observers continue to use the same event name.
In `@StreamVideoTests/Mock/MockSFUStack.swift`:
- Around line 12-22: Add a short doc comment on MockSFUStack’s
setConnectionState and receiveEvent methods to clarify that they always act on
the original webSocket created at init, not on nextWebSocket/box.socket after a
refresh. Reference the existing nextWebSocket property and the
setConnectionState/receiveEvent helpers so future test authors understand they
must call simulate/receive on the refreshed MockSFUWebSocket directly.
In `@StreamVideoTests/WebSocketClient/CoordinatorWebSocket_Tests.swift`:
- Around line 14-49: The test methods in CoordinatorWebSocket_Tests.swift need
to be renamed to match the repository’s required test naming convention. Update
the existing tests around VideoWebSocketConnectionState mapping so each method
follows the test_<given>_<when>_<then>_ pattern, using the current scenarios in
test_map_initializedConnectingAuthenticating,
test_map_connected_carriesConnectionId, and test_map_disconnectionSources as the
basis for the new names.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 40f3075c-e545-460e-8db3-5c80758d8e57
📒 Files selected for processing (77)
Package.swiftSources/StreamVideo/Errors/Errors.swiftSources/StreamVideo/Errors/StreamAPIError.swiftSources/StreamVideo/OpenApi/URLSessionTransport.swiftSources/StreamVideo/StreamVideo.swiftSources/StreamVideo/StreamVideoEnvironment.swiftSources/StreamVideo/Utils/ClientEventReporting/ClientEventFailure.swiftSources/StreamVideo/Utils/Extensions/Concurrency/Task+DisposableBag.swiftSources/StreamVideo/Utils/Logger/Array+Logger.swiftSources/StreamVideo/Utils/Logger/Data+DebugPrettyPrintedJSON.swiftSources/StreamVideo/Utils/Logger/Destination/BaseLogDestination.swiftSources/StreamVideo/Utils/Logger/Destination/ConsoleLogDestination.swiftSources/StreamVideo/Utils/Logger/Destination/LogDestination.swiftSources/StreamVideo/Utils/Logger/Formatter/LogFormatter.swiftSources/StreamVideo/Utils/Logger/Formatter/PrefixLogFormatter.swiftSources/StreamVideo/Utils/Logger/Logger+WebRTC.swiftSources/StreamVideo/Utils/Logger/Logger.swiftSources/StreamVideo/Utils/Logger/Publisher+Logger.swiftSources/StreamVideo/Utils/Logger/StreamCoreLogging.swiftSources/StreamVideo/Utils/StateMachine/StreamStateMachine.swiftSources/StreamVideo/Utils/Store/StoreLogger.swiftSources/StreamVideo/WebRTC/WebRTCEventDecoder.swiftSources/StreamVideo/WebRTC/v2/SFU/Protocols/WebSocketClientProviding.swiftSources/StreamVideo/WebRTC/v2/SFU/SFUAdapter.swiftSources/StreamVideo/WebRTC/v2/SFU/SFUConnectionState.swiftSources/StreamVideo/WebRTC/v2/SFU/SFUEvent+StreamCore.swiftSources/StreamVideo/WebRTC/v2/SFU/SFUWebSocket.swiftSources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator+Disconnected.swiftSources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator+FastReconnecting.swiftSources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator+Joined.swiftSources/StreamVideo/WebRTC/v2/StateMachine/Stages/WebRTCCoordinator+Stage.swiftSources/StreamVideo/WebRTC/v2/WebRTCAuthenticator.swiftSources/StreamVideo/WebSockets/Client/ConnectionRecoveryHandler.swiftSources/StreamVideo/WebSockets/Client/ConnectionStatus.swiftSources/StreamVideo/WebSockets/Client/Coordinator/CoordinatorEventDecoder.swiftSources/StreamVideo/WebSockets/Client/Coordinator/CoordinatorWebSocket.swiftSources/StreamVideo/WebSockets/Client/Coordinator/CoordinatorWebSocketTypes.swiftSources/StreamVideo/WebSockets/Client/Coordinator/Reconnection/BackgroundStateReconnectionPolicy.swiftSources/StreamVideo/WebSockets/Client/Coordinator/Reconnection/CallKitReconnectionPolicy.swiftSources/StreamVideo/WebSockets/Client/Coordinator/Reconnection/CompositeReconnectionPolicy.swiftSources/StreamVideo/WebSockets/Client/Coordinator/Reconnection/InternetAvailabilityReconnectionPolicy.swiftSources/StreamVideo/WebSockets/Client/Coordinator/Reconnection/WebSocketAutomaticReconnectionPolicy.swiftSources/StreamVideo/WebSockets/Client/HealthCheckInfo.swiftSources/StreamVideo/WebSockets/Client/URLSessionWebSocketEngine.swiftSources/StreamVideo/WebSockets/Client/WebSocketClient.swiftSources/StreamVideo/WebSockets/Client/WebSocketEngine.swiftSources/StreamVideo/WebSockets/Client/WebSocketEngineError.swiftSources/StreamVideo/WebSockets/Client/WebSocketInternalEvents.swiftSources/StreamVideo/WebSockets/Client/WebSocketPingController.swiftStreamVideo.xcodeproj/project.pbxprojStreamVideoTests/Call/Call_JoinRecovery_Tests.swiftStreamVideoTests/IntegrationTests/Call_IntegrationTests.swiftStreamVideoTests/Mock/MockCoordinatorWebSocket.swiftStreamVideoTests/Mock/MockSFUStack.swiftStreamVideoTests/Mock/MockWebSocketClientFactory.swiftStreamVideoTests/Mock/WebSocketClientEnvironment_Mock.swiftStreamVideoTests/Mock/WebSocketEngine_Mock.swiftStreamVideoTests/Mock/WebSocketPingController_Mock.swiftStreamVideoTests/TestUtils/WebSocketPingController_Delegate.swiftStreamVideoTests/Utils/ClientEventReporting/ClientEventDelivery_Tests.swiftStreamVideoTests/Utils/ClientEventReporting/ClientEventFailure_Tests.swiftStreamVideoTests/Utils/ClientEventReporting/ClientEventReporter_Tests.swiftStreamVideoTests/WebRTC/Retries_Tests.swiftStreamVideoTests/WebRTC/SFU/Mocks/MockSFUWebSocket.swiftStreamVideoTests/WebRTC/SFU/Mocks/MockWebSocketClient.swiftStreamVideoTests/WebRTC/SFU/Mocks/MockWebSocketEngine.swiftStreamVideoTests/WebRTC/SFU/SFUAdapter_Tests.swiftStreamVideoTests/WebRTC/SFU/SFUEventAdapter_Tests.swiftStreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_DisconnectedStageTests.swiftStreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_FastReconnectingStageTests.swiftStreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_JoinedStageTests.swiftStreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_JoiningStageTests.swiftStreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_LeavingStageTests.swiftStreamVideoTests/WebRTC/v2/StateMachine/Stages/WebRTCCoordinatorStateMachine_RejoiningStageTests.swiftStreamVideoTests/WebSocketClient/CoordinatorWebSocket_Tests.swiftStreamVideoTests/WebSocketClient/WebSocketClient_Tests.swiftStreamVideoTests/WebSocketClient/WebSocketPingController_Tests.swift
💤 Files with no reviewable changes (23)
- Sources/StreamVideo/WebSockets/Client/WebSocketClient.swift
- StreamVideoTests/Mock/MockWebSocketClientFactory.swift
- StreamVideoTests/WebSocketClient/WebSocketPingController_Tests.swift
- Sources/StreamVideo/Utils/Logger/Formatter/PrefixLogFormatter.swift
- StreamVideoTests/TestUtils/WebSocketPingController_Delegate.swift
- Sources/StreamVideo/Utils/Logger/Publisher+Logger.swift
- Sources/StreamVideo/WebSockets/Client/WebSocketEngine.swift
- StreamVideoTests/Mock/WebSocketPingController_Mock.swift
- Sources/StreamVideo/WebSockets/Client/ConnectionRecoveryHandler.swift
- Sources/StreamVideo/Utils/Logger/Destination/ConsoleLogDestination.swift
- StreamVideoTests/Mock/WebSocketClientEnvironment_Mock.swift
- Sources/StreamVideo/Utils/Logger/Formatter/LogFormatter.swift
- Sources/StreamVideo/Utils/Logger/Destination/LogDestination.swift
- Sources/StreamVideo/WebSockets/Client/URLSessionWebSocketEngine.swift
- StreamVideoTests/WebRTC/SFU/Mocks/MockWebSocketClient.swift
- StreamVideoTests/Mock/WebSocketEngine_Mock.swift
- Sources/StreamVideo/Utils/Logger/Array+Logger.swift
- Sources/StreamVideo/Utils/Logger/Destination/BaseLogDestination.swift
- Sources/StreamVideo/WebRTC/v2/SFU/Protocols/WebSocketClientProviding.swift
- StreamVideoTests/WebRTC/SFU/Mocks/MockWebSocketEngine.swift
- StreamVideoTests/WebSocketClient/WebSocketClient_Tests.swift
- Sources/StreamVideo/WebSockets/Client/WebSocketPingController.swift
- Sources/StreamVideo/Utils/Logger/Logger.swift
ipavlidakis
force-pushed
the
iliaspavlidakis/ios-1816-migrate-sdk-signaling-core-infra-onto-stream-core-swift
branch
from
July 3, 2026 08:31
52a3f7c to
820fd02
Compare
ipavlidakis
changed the title
martinmitrevski
left a comment
martinmitrevski
left a comment
Contributor
There was a problem hiding this comment.
partial review, will proceed at some point again
| /// `StreamAPIError` and the generated `APIError`) goes away once the OpenAPI | ||
| /// generator is changed to emit `APIError` as `StreamCore.APIError`. Then the | ||
| /// whole app uses one API error type and this file can be deleted. | ||
| public typealias StreamAPIError = StreamCore.APIError |
Contributor
There was a problem hiding this comment.
can't we use the APIError directly?
Contributor
Author
There was a problem hiding this comment.
There are currently two nominally different APIError types: the generated StreamVideo.APIError and StreamCore.APIError. Using bare APIError resolves to the generated model, not StreamCore’s type. StreamAPIError therefore disambiguates Core errors without requiring import StreamCore in every consumer, which would also introduce symbol collisions. ClientError.apiError remains the generated type for source compatibility, with Core errors converted at the boundary. The alias can be removed once the generated model migrates to StreamCore (which i'm not even sure if it's possible to happen as it will require backend to align the apiErrors between chat and video and potentially feeds).
| // them under their historical StreamVideo names so existing call sites keep | ||
| // working module-wide without importing StreamCore in every file. | ||
|
|
||
| public typealias LogLevel = StreamCore.LogLevel |
Contributor
There was a problem hiding this comment.
can't we solve this with @_exported import StreamCore?
Contributor
Author
There was a problem hiding this comment.
An exported import would expose all of StreamCore downstream and introduce collisions with Video’s existing types. The focused aliases preserve compatibility without broadening the public module surface, so I’ve kept the current approach.
martinmitrevski
left a comment
martinmitrevski
left a comment
Contributor
There was a problem hiding this comment.
looks good so far. There's still some potential to reduce the SDK size further.
| /// therefore cannot import StreamCore). `SFUWebSocket` maps | ||
| /// `StreamCore.WebSocketConnectionState` into this type at the boundary. | ||
| /// | ||
| /// - TODO: [StreamCore migration] This boundary enum exists so the WebRTC state |
Contributor
There was a problem hiding this comment.
I don't really understand this reasoning, wdym by unifying leaf types?
Contributor
Author
There was a problem hiding this comment.
Those are are the leaf types #1195 (basically types that are primitive but they are being used as they are without extensions)
| /// the WebRTC state machine never import StreamCore. | ||
| /// | ||
| /// - TODO: [StreamCore migration] This isolation wrapper exists because video | ||
| /// still owns duplicated leaf types (`log`, `ClientError`, `DisposableBag`, |
Contributor
There was a problem hiding this comment.
are there any blockers for unifying them or is it just still not done?
Contributor
Author
There was a problem hiding this comment.
There is no fundamental blocker; it is intentionally deferred to keep this migration focused. The wrapper still normalizes StreamCore connection states, errors, and events into the types consumed by SFUAdapter, and provides its test-mocking seam. Removing it requires migrating SFUAdapter, the WebRTC state machine, and their mocks to consume StreamCore types directly. I’ve updated the documentation to make this remaining work explicit.
ipavlidakis
force-pushed
the
iliaspavlidakis/ios-1816-migrate-sdk-signaling-core-infra-onto-stream-core-swift
branch
2 times, most recently
from
July 13, 2026 08:41
83b030c to
366624e
Compare
Public Interface+ extension StreamCore.LogConfig
+
+ public static var webRTCLogsEnabled: Bool
+ extension LogSubsystem
+
+ public static let videoDefault
+ extension Stream_Video_Sfu_Event_SfuEvent.OneOf_EventPayload: StreamCore.Event
+
+ public func healthcheck()-> StreamCore.HealthCheckInfo?
+ public func error()-> Error?
- public class PrefixLogFormatter: LogFormatter
-
- public init(prefixes: [LogLevel: String])
-
-
- public func format(logDetails: LogDetails,message: String)-> String
- public struct LogDetails: Sendable
-
- public let loggerIdentifier: String
- public let subsystem: LogSubsystem
- public let level: LogLevel
- public let date: Date
- public let message: String
- public let threadName: String
- public let functionName: StaticString
- public let fileName: StaticString
- public let lineNumber: UInt
- public let error: Error?
- public protocol LogDestination: Sendable
- public extension LogDestination
- public struct Log: Publisher
-
- public func receive(subscriber: S)
- public final class ConsoleLogDestination: BaseLogDestination, @unchecked Sendable
-
- override public func write(message: String)
- public enum LogConfig
-
- public nonisolated static var identifier
- public nonisolated static var level: LogLevel
- public nonisolated static var dateFormatter: DateFormatter
- public nonisolated static var formatters
- public nonisolated static var showDate
- public nonisolated static var showLevel
- public nonisolated static var showIdentifier
- public nonisolated static var showThreadName
- public nonisolated static var showFileName
- public nonisolated static var showLineNumber
- public nonisolated static var showFunctionName
- public nonisolated static var subsystems: LogSubsystem
- public nonisolated static var destinationTypes: [LogDestination.Type]
- public static var destinations: [LogDestination]
- public static var logger: Logger
- public static var webRTCLogsEnabled: Bool
- public class Logger: @unchecked Sendable
-
- public let identifier: String
- public var destinations: [LogDestination]
-
-
- public init(identifier: String = "",destinations: [LogDestination] = [])
-
-
- public func callAsFunction(_ level: LogLevel,functionName: StaticString = #function,fileName: StaticString = #fileID,lineNumber: UInt = #line,message: @autoclosure () -> Any,subsystems: LogSubsystem = .other,error: Error?)
- public func log(_ level: LogLevel,functionName: StaticString = #function,fileName: StaticString = #fileID,lineNumber: UInt = #line,message: @autoclosure () -> Any,subsystems: LogSubsystem = .other,error: Error?)
- public func info(_ message: @autoclosure () -> Any,subsystems: LogSubsystem = .other,functionName: StaticString = #function,fileName: StaticString = #fileID,lineNumber: UInt = #line)
- public func debug(_ message: @autoclosure () -> Any,subsystems: LogSubsystem = .other,functionName: StaticString = #function,fileName: StaticString = #fileID,lineNumber: UInt = #line)
- public func warning(_ message: @autoclosure () -> Any,subsystems: LogSubsystem = .other,functionName: StaticString = #function,fileName: StaticString = #fileID,lineNumber: UInt = #line)
- public func error(_ message: @autoclosure () -> Any,subsystems: LogSubsystem = .other,error: Error? = nil,functionName: StaticString = #function,fileName: StaticString = #fileID,lineNumber: UInt = #line)
- public func assert(_ condition: @autoclosure () -> Bool,_ message: @autoclosure () -> Any,subsystems: LogSubsystem = .other,functionName: StaticString = #function,fileName: StaticString = #fileID,lineNumber: UInt = #line)
- public func assertionFailure(_ message: @autoclosure () -> Any,subsystems: LogSubsystem = .other,functionName: StaticString = #function,fileName: StaticString = #fileID,lineNumber: UInt = #line)
- extension Array
-
- public func log(_ level: LogLevel,subsystems: LogSubsystem = .other,functionName: StaticString = #function,fileName: StaticString = #fileID,lineNumber: UInt = #line,messageBuilder: ((Self) -> String)? = nil)-> Self
- open class BaseLogDestination: LogDestination, @unchecked Sendable
-
- open var identifier: String
- open var level: LogLevel
- open var subsystems: LogSubsystem
- open var dateFormatter: DateFormatter
- open var formatters: [LogFormatter]
- open var showDate: Bool
- open var showLevel: Bool
- open var showIdentifier: Bool
- open var showThreadName: Bool
- open var showFileName: Bool
- open var showLineNumber: Bool
- open var showFunctionName: Bool
-
-
- public required init(identifier: String,level: LogLevel,subsystems: LogSubsystem,showDate: Bool,dateFormatter: DateFormatter,formatters: [LogFormatter],showLevel: Bool,showIdentifier: Bool,showThreadName: Bool,showFileName: Bool,showLineNumber: Bool,showFunctionName: Bool)
-
-
- open func isEnabled(level: LogLevel)-> Bool
- open func isEnabled(level: LogLevel,subsystems: LogSubsystem)-> Bool
- open func process(logDetails: LogDetails)
- open func applyFormatters(logDetails: LogDetails,message: String)-> String
- open func write(message: String)
- public struct LogSubsystem: OptionSet, CustomStringConvertible, Sendable
-
- public let rawValue: Int
- public static let allCases: [LogSubsystem]
- public static let all: LogSubsystem
- public static let other
- public static let database
- public static let httpRequests
- public static let webSocket
- public static let offlineSupport
- public static let webRTC
- public static let peerConnectionPublisher
- public static let peerConnectionSubscriber
- public static let sfu
- public static let iceAdapter
- public static let mediaAdapter
- public static let thermalState
- public static let audioSession
- public static let videoCapturer
- public static let pictureInPicture
- public static let callKit
- public static let webRTCInternal
- public static let audioRecording
- public var description: String
-
-
- public init(rawValue: Int)
- public enum LogLevel: Int, Sendable
-
- case debug = 0
- case info
- case warning
- case error
- public class WebSocket: ClientError, @unchecked Sendable
- public protocol LogFormatter
extension Task
- @discardableResult public init(disposableBag: DisposableBag,identifier: String = UUIDProviderKey.currentValue.get().uuidString,priority: TaskPriority? = nil,subsystem: LogSubsystem = .other,file: StaticString = #file,function: StaticString = #function,line: UInt = #line,@_inheritActorContext block: @Sendable @escaping () async -> Success)
+ @discardableResult public init(disposableBag: DisposableBag,identifier: String = UUIDProviderKey.currentValue.get().uuidString,priority: TaskPriority? = nil,subsystem: LogSubsystem = .videoDefault,file: StaticString = #file,function: StaticString = #function,line: UInt = #line,@_inheritActorContext block: @Sendable @escaping () async -> Success)
- @discardableResult public init(disposableBag: DisposableBag,identifier: String = UUIDProviderKey.currentValue.get().uuidString,priority: TaskPriority? = nil,subsystem: LogSubsystem = .other,file: StaticString = #file,function: StaticString = #function,line: UInt = #line,@_inheritActorContext block: @Sendable @escaping () async throws -> Success)
+ @discardableResult public init(disposableBag: DisposableBag,identifier: String = UUIDProviderKey.currentValue.get().uuidString,priority: TaskPriority? = nil,subsystem: LogSubsystem = .videoDefault,file: StaticString = #file,function: StaticString = #function,line: UInt = #line,@_inheritActorContext block: @Sendable @escaping () async throws -> Success)
public final class StreamStateMachine
- public init(initialStage: StageType,logSubsystem: LogSubsystem = .other)
+ public init(initialStage: StageType,logSubsystem: LogSubsystem = .videoDefault) |
6 tasks
Add the StreamCore package dependency (>= 0.8.0) to the StreamVideo target via SPM and the Xcode project. No imports yet — this only establishes linkage for the upcoming isolated WebSocket wrappers. Behaviour and public API unchanged. Co-authored-by: Cursor <cursoragent@cursor.com>
… (WebSocket, logger, error model, reconnection) # Conflicts: # Sources/StreamVideo/WebSockets/Client/WebSocketClient.swift # StreamVideoTests/Mock/WebSocketEngine_Mock.swift # StreamVideoTests/WebSocketClient/WebSocketClient_Tests.swift
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
ipavlidakis
force-pushed
the
iliaspavlidakis/ios-1816-migrate-sdk-signaling-core-infra-onto-stream-core-swift
branch
from
July 17, 2026 09:09
ca91f80 to
1a44ddf
Compare
martinmitrevski
left a comment
martinmitrevski
left a comment
Contributor
There was a problem hiding this comment.
Looks good, let's test it a bit before merging. Left few comments as well.
|
|
||
| public extension RawJSON { | ||
| /// Extracts the wrapped value as the specified type, if possible. | ||
| func value<T>() -> T? { |
Contributor
There was a problem hiding this comment.
shouldn't this go into StreamCore?
Contributor
Author
There was a problem hiding this comment.
If chat or feeds not using it, there is no reason to move it down to Core, right?
| log.error("Error refreshing token, will disconnect ws connection", error: error) | ||
| } | ||
| } else { | ||
| connectionRecoveryHandler?.webSocketClient(client, didUpdateConnectionState: state) |
Contributor
There was a problem hiding this comment.
we don't need this any longer?
Contributor
Author
There was a problem hiding this comment.
The connectionRecoveryHandler is now part of the CoordinatorWebsocket, which is now the delegate to the handler https://github.com/GetStream/stream-video-swift/pull/1178/changes#diff-66385429e716ff2693e908df6930e45f6baf756f7a9dc83caac3cc07065b1bd5R148
Contributor
Author
There was a problem hiding this comment.
Just to be sure i added also a test to ensure connection recovery in CoordinatorWebSocket_Tests
| // | ||
|
|
||
| import Combine | ||
| import enum StreamCore.RawJSON |
Contributor
There was a problem hiding this comment.
do we need to be this specific? Why not the whole core?
Contributor
Author
There was a problem hiding this comment.
Because we aren't consuming everything from StreamCore directly yet (mainly now the problem is the APIError) we rely on typealiases. That means that in files that both StreamCore and StreamVideo types collide, they create ambiguation causing compilation errors. The specific import is to avoid this problem. Once the APIError migration is in we can go through those specific imports and check if we can remove them but for now that should be fine.
ipavlidakis
force-pushed
the
iliaspavlidakis/ios-1816-migrate-sdk-signaling-core-infra-onto-stream-core-swift
branch
from
July 23, 2026 22:28
b7c140f to
5ad7433
Compare
ipavlidakis
commented
Jul 24, 2026
| ) { | ||
| if case let .disconnected(source) = state, | ||
| let serverError = source.serverError, | ||
| serverError.isInvalidTokenError || serverError.isTokenExpiredError { |
Contributor
Author
There was a problem hiding this comment.
If the token is expired, it doesn't forward the event to the recoveryHandler so no reconnection is being triggered.
ipavlidakis
commented
Jul 24, 2026
| coordinatorHealthCheck: .init( | ||
| cid: nil, | ||
| connectionId: connectedEvent.connectionId, | ||
| createdAt: connectedEvent.createdAt |
Contributor
Author
There was a problem hiding this comment.
The StreamCore.HealthcheckInfo has no createdAt field
ipavlidakis
commented
Jul 24, 2026
| ) | ||
| if let connectURL = try? URL(string: Self.endpointConfig.wsEndpoint)?.appendingQueryItems(queryParams) { | ||
| webSocketClient = makeWebSocketClient(url: connectURL, apiKey: apiKey) | ||
| webSocketClient = makeWebSocketClient(url: connectURL) |
Contributor
Author
There was a problem hiding this comment.
We don't need to pass in the apiKey any more as it's embedded in the connectURL
ipavlidakis
commented
Jul 24, 2026
| log.error("Error refreshing token, will disconnect ws connection", error: error) | ||
| } | ||
| if let serverError = source.serverError, | ||
| serverError.isInvalidTokenError || serverError.isTokenExpiredError { |
Contributor
Author
There was a problem hiding this comment.
This is the supplementary part for the token refresh and the WebsocketReconnectionPolicy. During a websocket disconnection StreamVideo and the CoordinatorWebsocket are being informed. If the disconnection was due to tokenExpired, then the CoordinatorWebsocket does nothing while StreamVideo kicks in a token refresh.
Collaborator
SDK Size
|
Collaborator
StreamVideo XCSize
Show 482 more objects
|
Collaborator
StreamVideoSwiftUI XCSize
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🔗 Issue Links
Resolves https://linear.app/stream/issue/IOS-1816/migrate-sdk-signaling-and-core-infra-onto-stream-core-swift-websocket
🎯 Goal
StreamVideo duplicates a lot of low-level infrastructure that already lives in
stream-core-swift(WebSocket client, logger,APIError, connection recovery). This PR routes both signaling WebSockets (SFU + coordinator), the logger, the API error model, and connection recovery through StreamCore instead of our own copies — removing duplication, keeping behavior consistent across Stream SDKs, and letting us track upstream fixes. It's the first, foundational step of the broader "converge StreamVideo on StreamCore" effort.📝 Summary
stream-core-swift(from0.8.0) as a dependency ofStreamVideo,StreamVideoSwiftUI, andStreamVideoUIKit.StreamCore.WebSocketClientvia a newSFUWebSocketisolation wrapper; state exposed as a video-sideSFUConnectionState.StreamCore.WebSocketClientvia a newCoordinatorWebSocketwrapper (auth handshake, event bridging,connectionId, state mapping).StreamAPIError(StreamCore.APIError) across hand-written code; generated OpenAPI layer keeps its ownAPIError.StreamCore.DefaultConnectionRecoveryHandler, with the video-specific CallKit policy preserved.🛠 Implementation
Isolation-wrapper strategy. Importing StreamCore module-wide would collide with video's still-duplicated leaf types (
ClientError,DisposableBag,Event,EventNotificationCenter, and — before this PR — the logger). To keep the blast radius small, each socket is wrapped in a single StreamCore-importing type (SFUWebSocket,CoordinatorWebSocket) that exposes a StreamCore-free, video-typed surface. Consumers (SFUAdapter, the WebRTC state machine,StreamVideo) stay StreamCore-free.SFUWebSocketownsStreamCore.WebSocketClient, mapsStreamCore.WebSocketConnectionState → SFUConnectionState, bridges SFU payloads (SFUEvent+StreamCore,WebRTCEventDecoder → StreamCore.AnyEventDecoder).SFUAdapteris no longer aConnectionStateDelegate; it mirrors state via the wrapper's publisher. ASFUWebSocketProtocol+MockSFUWebSocketkeep it testable.CoordinatorWebSocketperforms theWSAuthMessageRequestauth handshake on connect, decodes via video'sJsonEventDecoderboxed as aStreamCore.Event(CoordinatorEvent) and forwards into video's existingEventNotificationCenter/middlewares (so event handling is unchanged), maps state back to video'sWebSocketConnectionState, and exposesconnectionId.StreamVideo/StreamVideoEnvironmentwere rewired onto it; token-refresh-on-invalid-token stays inStreamVideo.Publisher+Logger/Logger/LogSubsystem/LogLevel/LogConfig/destinations/formatters/Array+Logger; re-exposed StreamCore's via module-wide typealiases (StreamCoreLogging.swift). Re-addedLogSubsystem.webRTCInternal+LogConfig.webRTCLogsEnabled.LogSubsystem.videoDefaultworks around Swift's default-argument-value rule (member must be same-module).StreamAPIError = StreamCore.APIError. The transport decodes/throws it;ClientError.apiErrorisStreamAPIError?; token/client-error gating reads it. Only stored properties are accessed through the alias, so no StreamCore import is needed at call sites.CoordinatorWebSocketbuildsStreamCore.DefaultConnectionRecoveryHandler. StreamCore's own policies areinternal, so the policy set is re-implemented against StreamCore's public APIs (extracted toCoordinator/Reconnection/) to preserveinternet AND wsAuto AND (background OR CallKit). CallKit's active-call check is injected as a closure (DI is ambiguous inside a StreamCore-importing file).Follow-ups are tracked in code via a greppable
[StreamCore migration]marker (reconnection policies once StreamCore makes thempublic; wrapper/alias/boxing removal once the event system + leaf types are unified;StreamAPIErrorremoval once codegen emitsStreamCore.APIError; restoring WebRTC log-severity sync). No public API is removed source-incompatibly; logger/error/state types are preserved by name via typealiases.🧪 Manual Testing Notes
connectionIdis populated.☑️ Contributor Checklist
Summary by CodeRabbit
New Features
Breaking Changes