From 9c87a98e44f2cd5b521e5718e5b362eb681c36f3 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 25 Nov 2024 16:49:02 -0300 Subject: [PATCH 1/2] Add reason to discontinuity event logging --- Sources/AblyChat/RoomLifecycleManager.swift | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index 51a92545..fc2b0e9b 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -448,12 +448,12 @@ internal actor DefaultRoomLifecycleManager Date: Mon, 25 Nov 2024 17:02:43 -0300 Subject: [PATCH 2/2] Allow discontinuities without an error Although the JS SDK allows users to be notified of a discontinuity without an associated error, when I copied the public API across in 20e7f5f I decided that this was probably unintentional and that we could do better (i.e. that surely every discontinuity has an error). However, in [1] Andy has pointed out at least a couple of ways in you might have a discontinuity without an error. Namely: 1. RTL12 does not guarantee that an UPDATE with resumed == false has an accompanying error 2. > we raise discontinuities if you explicitly detach a room and then > bring it back again So, allow discontinuities without an associated error. [1] https://github.com/ably/specification/issues/239#issuecomment-2498810395 --- .../AblyChatExample/Mocks/MockClients.swift | 10 +++---- Sources/AblyChat/DefaultMessages.swift | 2 +- Sources/AblyChat/DefaultOccupancy.swift | 2 +- Sources/AblyChat/DefaultPresence.swift | 2 +- .../DefaultRoomLifecycleContributor.swift | 8 ++--- Sources/AblyChat/DefaultRoomReactions.swift | 2 +- Sources/AblyChat/DefaultTyping.swift | 2 +- Sources/AblyChat/EmitsDiscontinuities.swift | 2 +- Sources/AblyChat/RoomFeature.swift | 2 +- Sources/AblyChat/RoomLifecycleManager.swift | 30 ++++++++----------- .../DefaultRoomLifecycleManagerTests.swift | 18 +++++------ .../Mocks/MockFeatureChannel.swift | 8 ++--- .../Mocks/MockRoomLifecycleContributor.swift | 4 +-- 13 files changed, 43 insertions(+), 49 deletions(-) diff --git a/Example/AblyChatExample/Mocks/MockClients.swift b/Example/AblyChatExample/Mocks/MockClients.swift index d73300df..b70ae0b2 100644 --- a/Example/AblyChatExample/Mocks/MockClients.swift +++ b/Example/AblyChatExample/Mocks/MockClients.swift @@ -142,7 +142,7 @@ actor MockMessages: Messages { return message } - func subscribeToDiscontinuities() -> Subscription { + func subscribeToDiscontinuities() -> Subscription { fatalError("Not yet implemented") } } @@ -193,7 +193,7 @@ actor MockRoomReactions: RoomReactions { .init(mockAsyncSequence: createSubscription()) } - func subscribeToDiscontinuities() -> Subscription { + func subscribeToDiscontinuities() -> Subscription { fatalError("Not yet implemented") } } @@ -242,7 +242,7 @@ actor MockTyping: Typing { } } - func subscribeToDiscontinuities() -> Subscription { + func subscribeToDiscontinuities() -> Subscription { fatalError("Not yet implemented") } } @@ -346,7 +346,7 @@ actor MockPresence: Presence { .init(mockAsyncSequence: createSubscription()) } - func subscribeToDiscontinuities() -> Subscription { + func subscribeToDiscontinuities() -> Subscription { fatalError("Not yet implemented") } } @@ -381,7 +381,7 @@ actor MockOccupancy: Occupancy { OccupancyEvent(connections: 10, presenceMembers: 5) } - func subscribeToDiscontinuities() -> Subscription { + func subscribeToDiscontinuities() -> Subscription { fatalError("Not yet implemented") } } diff --git a/Sources/AblyChat/DefaultMessages.swift b/Sources/AblyChat/DefaultMessages.swift index d8546526..fe232c50 100644 --- a/Sources/AblyChat/DefaultMessages.swift +++ b/Sources/AblyChat/DefaultMessages.swift @@ -107,7 +107,7 @@ internal final class DefaultMessages: Messages, EmitsDiscontinuities { } // (CHA-M7) Users may subscribe to discontinuity events to know when there’s been a break in messages that they need to resolve. Their listener will be called when a discontinuity event is triggered from the room lifecycle. - internal func subscribeToDiscontinuities() async -> Subscription { + internal func subscribeToDiscontinuities() async -> Subscription { await featureChannel.subscribeToDiscontinuities() } diff --git a/Sources/AblyChat/DefaultOccupancy.swift b/Sources/AblyChat/DefaultOccupancy.swift index 380b6029..0f6db119 100644 --- a/Sources/AblyChat/DefaultOccupancy.swift +++ b/Sources/AblyChat/DefaultOccupancy.swift @@ -50,7 +50,7 @@ internal final class DefaultOccupancy: Occupancy, EmitsDiscontinuities { } // (CHA-O5) Users may subscribe to discontinuity events to know when there’s been a break in occupancy. Their listener will be called when a discontinuity event is triggered from the room lifecycle. For occupancy, there shouldn’t need to be user action as most channels will send occupancy updates regularly as clients churn. - internal func subscribeToDiscontinuities() async -> Subscription { + internal func subscribeToDiscontinuities() async -> Subscription { await featureChannel.subscribeToDiscontinuities() } } diff --git a/Sources/AblyChat/DefaultPresence.swift b/Sources/AblyChat/DefaultPresence.swift index fbeca98d..038518fc 100644 --- a/Sources/AblyChat/DefaultPresence.swift +++ b/Sources/AblyChat/DefaultPresence.swift @@ -194,7 +194,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities { } // (CHA-PR8) Users may subscribe to discontinuity events to know when there’s been a break in presence. Their listener will be called when a discontinuity event is triggered from the room lifecycle. For presence, there shouldn’t need to be user action as the underlying core SDK will heal the presence set. - internal func subscribeToDiscontinuities() async -> Subscription { + internal func subscribeToDiscontinuities() async -> Subscription { await featureChannel.subscribeToDiscontinuities() } diff --git a/Sources/AblyChat/DefaultRoomLifecycleContributor.swift b/Sources/AblyChat/DefaultRoomLifecycleContributor.swift index fd991ed6..1671e003 100644 --- a/Sources/AblyChat/DefaultRoomLifecycleContributor.swift +++ b/Sources/AblyChat/DefaultRoomLifecycleContributor.swift @@ -3,7 +3,7 @@ import Ably internal actor DefaultRoomLifecycleContributor: RoomLifecycleContributor, EmitsDiscontinuities, CustomDebugStringConvertible { internal nonisolated let channel: DefaultRoomLifecycleContributorChannel internal nonisolated let feature: RoomFeature - private var discontinuitySubscriptions: [Subscription] = [] + private var discontinuitySubscriptions: [Subscription] = [] internal init(channel: DefaultRoomLifecycleContributorChannel, feature: RoomFeature) { self.channel = channel @@ -12,14 +12,14 @@ internal actor DefaultRoomLifecycleContributor: RoomLifecycleContributor, EmitsD // MARK: - Discontinuities - internal func emitDiscontinuity(_ error: ARTErrorInfo) { + internal func emitDiscontinuity(_ error: ARTErrorInfo?) { for subscription in discontinuitySubscriptions { subscription.emit(error) } } - internal func subscribeToDiscontinuities() -> Subscription { - let subscription = Subscription(bufferingPolicy: .unbounded) + internal func subscribeToDiscontinuities() -> Subscription { + let subscription = Subscription(bufferingPolicy: .unbounded) // TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36) discontinuitySubscriptions.append(subscription) return subscription diff --git a/Sources/AblyChat/DefaultRoomReactions.swift b/Sources/AblyChat/DefaultRoomReactions.swift index 6f6cfcfe..b4b91017 100644 --- a/Sources/AblyChat/DefaultRoomReactions.swift +++ b/Sources/AblyChat/DefaultRoomReactions.swift @@ -80,7 +80,7 @@ internal final class DefaultRoomReactions: RoomReactions, EmitsDiscontinuities { } // (CHA-ER5) Users may subscribe to discontinuity events to know when there’s been a break in reactions that they need to resolve. Their listener will be called when a discontinuity event is triggered from the room lifecycle. - internal func subscribeToDiscontinuities() async -> Subscription { + internal func subscribeToDiscontinuities() async -> Subscription { await featureChannel.subscribeToDiscontinuities() } diff --git a/Sources/AblyChat/DefaultTyping.swift b/Sources/AblyChat/DefaultTyping.swift index 79b1fef8..2dbdfc02 100644 --- a/Sources/AblyChat/DefaultTyping.swift +++ b/Sources/AblyChat/DefaultTyping.swift @@ -160,7 +160,7 @@ internal final class DefaultTyping: Typing { } // (CHA-T7) Users may subscribe to discontinuity events to know when there’s been a break in typing indicators. Their listener will be called when a discontinuity event is triggered from the room lifecycle. For typing, there shouldn’t need to be user action as the underlying core SDK will heal the presence set. - internal func subscribeToDiscontinuities() async -> Subscription { + internal func subscribeToDiscontinuities() async -> Subscription { await featureChannel.subscribeToDiscontinuities() } diff --git a/Sources/AblyChat/EmitsDiscontinuities.swift b/Sources/AblyChat/EmitsDiscontinuities.swift index 48bcff00..1fbe2a1a 100644 --- a/Sources/AblyChat/EmitsDiscontinuities.swift +++ b/Sources/AblyChat/EmitsDiscontinuities.swift @@ -1,5 +1,5 @@ import Ably public protocol EmitsDiscontinuities { - func subscribeToDiscontinuities() async -> Subscription + func subscribeToDiscontinuities() async -> Subscription } diff --git a/Sources/AblyChat/RoomFeature.swift b/Sources/AblyChat/RoomFeature.swift index 07ba4548..4ee4ab65 100644 --- a/Sources/AblyChat/RoomFeature.swift +++ b/Sources/AblyChat/RoomFeature.swift @@ -58,7 +58,7 @@ internal struct DefaultFeatureChannel: FeatureChannel { internal var contributor: DefaultRoomLifecycleContributor internal var roomLifecycleManager: RoomLifecycleManager - internal func subscribeToDiscontinuities() async -> Subscription { + internal func subscribeToDiscontinuities() async -> Subscription { await contributor.subscribeToDiscontinuities() } diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index fc2b0e9b..472d1fb9 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -37,7 +37,7 @@ internal protocol RoomLifecycleContributor: Identifiable, Sendable { /// Informs the contributor that there has been a break in channel continuity, which it should inform library users about. /// /// It is marked as `async` purely to make it easier to write mocks for this method (i.e. to use an actor as a mock). - func emitDiscontinuity(_ error: ARTErrorInfo) async + func emitDiscontinuity(_ error: ARTErrorInfo?) async } internal protocol RoomLifecycleManager: Sendable { @@ -111,7 +111,7 @@ internal actor DefaultRoomLifecycleManager? = nil, contributors: [Contributor], logger: InternalLogger, @@ -130,7 +130,7 @@ internal actor DefaultRoomLifecycleManager?, contributors: [Contributor], logger: InternalLogger, @@ -263,7 +263,7 @@ internal actor DefaultRoomLifecycleManager ) { storage = contributors.reduce(into: [:]) { result, contributor in @@ -397,7 +397,7 @@ internal actor DefaultRoomLifecycleManager [ARTErrorInfo] { + internal func testsOnly_pendingDiscontinuityEvents(for contributor: Contributor) -> [ARTErrorInfo?] { contributorAnnotations[contributor].pendingDiscontinuityEvents } @@ -441,19 +441,16 @@ internal actor DefaultRoomLifecycleManager.Status? = nil, - forTestingWhatHappensWhenHasPendingDiscontinuityEvents pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo]]? = nil, + forTestingWhatHappensWhenHasPendingDiscontinuityEvents pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo?]]? = nil, forTestingWhatHappensWhenHasTransientDisconnectTimeoutForTheseContributorIDs idsOfContributorsWithTransientDisconnectTimeout: Set? = nil, contributors: [MockRoomLifecycleContributor] = [], clock: SimpleClock = MockSimpleClock() @@ -262,7 +262,7 @@ struct DefaultRoomLifecycleManagerTests { func attach_uponSuccess_emitsPendingDiscontinuityEvents() async throws { // Given: A DefaultRoomLifecycleManager, all of whose contributors’ calls to `attach` succeed let contributors = (1 ... 3).map { _ in createContributor(attachBehavior: .success) } - let pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo]] = [ + let pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo?]] = [ contributors[1].id: [.init(domain: "SomeDomain", code: 123) /* arbitrary */ ], contributors[2].id: [.init(domain: "SomeDomain", code: 456) /* arbitrary */ ], ] @@ -333,7 +333,7 @@ struct DefaultRoomLifecycleManagerTests { current: .attached, previous: .detached, // arbitrary event: .attached, - reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change + reason: nil // arbitrary ) ) ) @@ -961,7 +961,7 @@ struct DefaultRoomLifecycleManagerTests { current: .attached, previous: .attaching, // arbitrary event: .attached, - reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change + reason: nil // arbitrary ) ) ), @@ -1010,7 +1010,7 @@ struct DefaultRoomLifecycleManagerTests { current: .attached, previous: .attaching, // arbitrary event: .attached, - reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change + reason: nil // arbitrary ) ) ), @@ -1203,7 +1203,7 @@ struct DefaultRoomLifecycleManagerTests { current: .attached, previous: .attaching, // arbitrary event: .attached, - reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change + reason: nil // arbitrary ) return .addSubscriptionAndEmitStateChange(contributorAttachedStateChange) @@ -1248,7 +1248,7 @@ struct DefaultRoomLifecycleManagerTests { current: .attached, previous: .attaching, // arbitrary event: .attached, - reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change + reason: nil // arbitrary ) ) // Not related to this test, just so that the CHA-RL5d wait completes ), @@ -1296,7 +1296,7 @@ struct DefaultRoomLifecycleManagerTests { current: .attached, previous: .attaching, // arbitrary event: .attached, - reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change + reason: nil // arbitrary ) ) // Not related to this test, just so that the CHA-RL5d wait completes ), @@ -1794,7 +1794,7 @@ struct DefaultRoomLifecycleManagerTests { current: .attached, previous: .detached, // arbitrary event: .attached, - reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change + reason: nil // arbitrary ) ) ) diff --git a/Tests/AblyChatTests/Mocks/MockFeatureChannel.swift b/Tests/AblyChatTests/Mocks/MockFeatureChannel.swift index 2b12b92f..dde42044 100644 --- a/Tests/AblyChatTests/Mocks/MockFeatureChannel.swift +++ b/Tests/AblyChatTests/Mocks/MockFeatureChannel.swift @@ -4,7 +4,7 @@ import Ably final actor MockFeatureChannel: FeatureChannel { let channel: RealtimeChannelProtocol // TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36) - private var discontinuitySubscriptions: [Subscription] = [] + private var discontinuitySubscriptions: [Subscription] = [] private let resultOfWaitToBeAbleToPerformPresenceOperations: Result? init( @@ -15,13 +15,13 @@ final actor MockFeatureChannel: FeatureChannel { resultOfWaitToBeAbleToPerformPresenceOperations = resultOfWaitToBeAblePerformPresenceOperations } - func subscribeToDiscontinuities() async -> Subscription { - let subscription = Subscription(bufferingPolicy: .unbounded) + func subscribeToDiscontinuities() async -> Subscription { + let subscription = Subscription(bufferingPolicy: .unbounded) discontinuitySubscriptions.append(subscription) return subscription } - func emitDiscontinuity(_ discontinuity: ARTErrorInfo) { + func emitDiscontinuity(_ discontinuity: ARTErrorInfo?) { for subscription in discontinuitySubscriptions { subscription.emit(discontinuity) } diff --git a/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift index e008fd28..eb6c5f48 100644 --- a/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift +++ b/Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift @@ -5,14 +5,14 @@ actor MockRoomLifecycleContributor: RoomLifecycleContributor { nonisolated let feature: RoomFeature nonisolated let channel: MockRoomLifecycleContributorChannel - private(set) var emitDiscontinuityArguments: [ARTErrorInfo] = [] + private(set) var emitDiscontinuityArguments: [ARTErrorInfo?] = [] init(feature: RoomFeature, channel: MockRoomLifecycleContributorChannel) { self.feature = feature self.channel = channel } - func emitDiscontinuity(_ error: ARTErrorInfo) async { + func emitDiscontinuity(_ error: ARTErrorInfo?) async { emitDiscontinuityArguments.append(error) } }