Merge pull request #150 from ably-labs/allow-nil-discontinuity

Allow discontinuities without an error

Example/AblyChatExample/Mocks/MockClients.swift

@@ -142,7 +142,7 @@ actor MockMessages: Messages {
         return message
     }
 
-    func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
+    func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
         fatalError("Not yet implemented")
     }
 }
@@ -193,7 +193,7 @@ actor MockRoomReactions: RoomReactions {
         .init(mockAsyncSequence: createSubscription())
     }
 
-    func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
+    func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
         fatalError("Not yet implemented")
     }
 }
@@ -242,7 +242,7 @@ actor MockTyping: Typing {
         }
     }
 
-    func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
+    func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
         fatalError("Not yet implemented")
     }
 }
@@ -346,7 +346,7 @@ actor MockPresence: Presence {
         .init(mockAsyncSequence: createSubscription())
     }
 
-    func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
+    func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
         fatalError("Not yet implemented")
     }
 }
@@ -381,7 +381,7 @@ actor MockOccupancy: Occupancy {
         OccupancyEvent(connections: 10, presenceMembers: 5)
     }
 
-    func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
+    func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
         fatalError("Not yet implemented")
     }
 }

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<ARTErrorInfo> {
+    internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
         await featureChannel.subscribeToDiscontinuities()
     }
 

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<ARTErrorInfo> {
+    internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
         await featureChannel.subscribeToDiscontinuities()
     }
 }

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<ARTErrorInfo> {
+    internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
         await featureChannel.subscribeToDiscontinuities()
     }
 

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<ARTErrorInfo>] = []
+    private var discontinuitySubscriptions: [Subscription<ARTErrorInfo?>] = []
 
     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<ARTErrorInfo> {
-        let subscription = Subscription<ARTErrorInfo>(bufferingPolicy: .unbounded)
+    internal func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
+        let subscription = Subscription<ARTErrorInfo?>(bufferingPolicy: .unbounded)
         // TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36)
         discontinuitySubscriptions.append(subscription)
         return subscription

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<ARTErrorInfo> {
+    internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
         await featureChannel.subscribeToDiscontinuities()
     }
 

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<ARTErrorInfo> {
+    internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
         await featureChannel.subscribeToDiscontinuities()
     }
 

Sources/AblyChat/EmitsDiscontinuities.swift

@@ -1,5 +1,5 @@
 import Ably
 
 public protocol EmitsDiscontinuities {
-    func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo>
+    func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?>
 }

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<ARTErrorInfo> {
+    internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
         await contributor.subscribeToDiscontinuities()
     }
 

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<Contributor: RoomLifecycleContributor
     #if DEBUG
         internal init(
             testsOnly_status status: Status? = nil,
-            testsOnly_pendingDiscontinuityEvents pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]]? = nil,
+            testsOnly_pendingDiscontinuityEvents pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo?]]? = nil,
             testsOnly_idsOfContributorsWithTransientDisconnectTimeout idsOfContributorsWithTransientDisconnectTimeout: Set<Contributor.ID>? = nil,
             contributors: [Contributor],
             logger: InternalLogger,
@@ -130,7 +130,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
 
     private init(
         status: Status?,
-        pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]]?,
+        pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo?]]?,
         idsOfContributorsWithTransientDisconnectTimeout: Set<Contributor.ID>?,
         contributors: [Contributor],
         logger: InternalLogger,
@@ -263,7 +263,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
         }
 
         // TODO: Not clear whether there can be multiple or just one (asked in https://github.com/ably/specification/pull/200/files#r1781927850)
-        var pendingDiscontinuityEvents: [ARTErrorInfo] = []
+        var pendingDiscontinuityEvents: [ARTErrorInfo?] = []
         var transientDisconnectTimeout: TransientDisconnectTimeout?
         /// Whether a state change to `ATTACHED` has already been observed for this contributor.
         var hasBeenAttached: Bool
@@ -279,7 +279,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
 
         init(
             contributors: [Contributor],
-            pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]],
+            pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo?]],
             idsOfContributorsWithTransientDisconnectTimeout: Set<Contributor.ID>
         ) {
             storage = contributors.reduce(into: [:]) { result, contributor in
@@ -397,7 +397,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
             return subscription
         }
 
-        internal func testsOnly_pendingDiscontinuityEvents(for contributor: Contributor) -> [ARTErrorInfo] {
+        internal func testsOnly_pendingDiscontinuityEvents(for contributor: Contributor) -> [ARTErrorInfo?] {
             contributorAnnotations[contributor].pendingDiscontinuityEvents
         }
 
@@ -441,19 +441,16 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
                 break
             }
 
-            guard let reason = stateChange.reason else {
-                // TODO: Decide the right thing to do here (https://github.com/ably-labs/ably-chat-swift/issues/74)
-                preconditionFailure("State change event with resumed == false should have a reason")
-            }
+            let reason = stateChange.reason
 
             if hasOperationInProgress {
                 // CHA-RL4a3
-                logger.log(message: "Recording pending discontinuity event for contributor \(contributor)", level: .info)
+                logger.log(message: "Recording pending discontinuity event \(String(describing: reason)) for contributor \(contributor)", level: .info)
 
                 contributorAnnotations[contributor].pendingDiscontinuityEvents.append(reason)
             } else {
                 // CHA-RL4a4
-                logger.log(message: "Emitting discontinuity event for contributor \(contributor)", level: .info)
+                logger.log(message: "Emitting discontinuity event \(String(describing: reason)) for contributor \(contributor)", level: .info)
 
                 await contributor.emitDiscontinuity(reason)
             }
@@ -464,12 +461,10 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
             if hasOperationInProgress {
                 if !stateChange.resumed, hadAlreadyAttached {
                     // CHA-RL4b1
-                    logger.log(message: "Recording pending discontinuity event for contributor \(contributor)", level: .info)
 
-                    guard let reason = stateChange.reason else {
-                        // TODO: Decide the right thing to do here (https://github.com/ably-labs/ably-chat-swift/issues/74)
-                        preconditionFailure("Non-initial ATTACHED state change with resumed == false should have a reason")
-                    }
+                    let reason = stateChange.reason
+
+                    logger.log(message: "Recording pending discontinuity event \(String(describing: reason)) for contributor \(contributor)", level: .info)
 
                     contributorAnnotations[contributor].pendingDiscontinuityEvents.append(reason)
                 }
@@ -860,7 +855,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
         logger.log(message: "Emitting pending discontinuity events", level: .info)
         for contributor in contributors {
             for pendingDiscontinuityEvent in contributorAnnotations[contributor].pendingDiscontinuityEvents {
-                logger.log(message: "Emitting pending discontinuity event \(pendingDiscontinuityEvent) to contributor \(contributor)", level: .info)
+                logger.log(message: "Emitting pending discontinuity event \(String(describing: pendingDiscontinuityEvent)) to contributor \(contributor)", level: .info)
                 await contributor.emitDiscontinuity(pendingDiscontinuityEvent)
             }
         }

Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift

@@ -54,7 +54,7 @@ struct DefaultRoomLifecycleManagerTests {
 
     private func createManager(
         forTestingWhatHappensWhenCurrentlyIn status: DefaultRoomLifecycleManager<MockRoomLifecycleContributor>.Status? = nil,
-        forTestingWhatHappensWhenHasPendingDiscontinuityEvents pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo]]? = nil,
+        forTestingWhatHappensWhenHasPendingDiscontinuityEvents pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo?]]? = nil,
         forTestingWhatHappensWhenHasTransientDisconnectTimeoutForTheseContributorIDs idsOfContributorsWithTransientDisconnectTimeout: Set<MockRoomLifecycleContributor.ID>? = 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
                 )
             )
         )

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<ARTErrorInfo>] = []
+    private var discontinuitySubscriptions: [Subscription<ARTErrorInfo?>] = []
     private let resultOfWaitToBeAbleToPerformPresenceOperations: Result<Void, ARTErrorInfo>?
 
     init(
@@ -15,13 +15,13 @@ final actor MockFeatureChannel: FeatureChannel {
         resultOfWaitToBeAbleToPerformPresenceOperations = resultOfWaitToBeAblePerformPresenceOperations
     }
 
-    func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
-        let subscription = Subscription<ARTErrorInfo>(bufferingPolicy: .unbounded)
+    func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
+        let subscription = Subscription<ARTErrorInfo?>(bufferingPolicy: .unbounded)
         discontinuitySubscriptions.append(subscription)
         return subscription
     }
 
-    func emitDiscontinuity(_ discontinuity: ARTErrorInfo) {
+    func emitDiscontinuity(_ discontinuity: ARTErrorInfo?) {
         for subscription in discontinuitySubscriptions {
             subscription.emit(discontinuity)
         }

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)
     }
 }