Move error inside RoomLifecycle

Lifecycle statuses have an error if and only if they have certain cases,
so encode this constraint in the type system to avoid unnecessary
unwraps.

Part of #12.

Example/AblyChatExample/Mocks/MockClients.swift

@@ -412,7 +412,7 @@ actor MockRoomStatus: RoomStatus {
 
     private func createSubscription() -> MockSubscription<RoomStatusChange> {
         let subscription = MockSubscription<RoomStatusChange>(randomElement: {
-            RoomStatusChange(current: [.attached, .attached, .attached, .attached, .attaching, .attaching, .suspended].randomElement()!, previous: .attaching)
+            RoomStatusChange(current: [.attached, .attached, .attached, .attached, .attaching, .attaching, .suspended(error: .createUnknownError())].randomElement()!, previous: .attaching)
         }, interval: 8)
         mockSubscriptions.append(subscription)
         return subscription

Sources/AblyChat/RoomLifecycleManager.swift

@@ -48,7 +48,6 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
     }
 
     internal private(set) var current: RoomLifecycle
-    internal private(set) var error: ARTErrorInfo?
     // TODO: This currently allows the the tests to inject a value in order to test the spec points that are predicated on whether “a channel lifecycle operation is in progress”. In https://github.com/ably-labs/ably-chat-swift/issues/52 we’ll set this property based on whether there actually is a lifecycle operation in progress.
     private let hasOperationInProgress: Bool
     /// Manager state that relates to individual contributors, keyed by contributors’ ``Contributor.id``. Stored separately from ``contributors`` so that the latter can be a `let`, to make it clear that the contributors remain fixed for the lifetime of the manager.
@@ -263,7 +262,7 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
                     preconditionFailure("FAILED state change event should have a reason")
                 }
 
-                changeStatus(to: .failed, error: reason)
+                changeStatus(to: .failed(error: reason))
 
                 // TODO: CHA-RL4b5 is a bit unclear about how to handle failure, and whether they can be detached concurrently (asked in https://github.com/ably/specification/pull/200/files#r1777471810)
                 for contributor in contributors {
@@ -282,7 +281,7 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
                     preconditionFailure("SUSPENDED state change event should have a reason")
                 }
 
-                changeStatus(to: .suspended, error: reason)
+                changeStatus(to: .suspended(error: reason))
             }
         default:
             break
@@ -295,13 +294,12 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
         #endif
     }
 
-    /// Updates ``current`` and ``error`` and emits a status change event.
-    private func changeStatus(to new: RoomLifecycle, error: ARTErrorInfo? = nil) {
-        logger.log(message: "Transitioning from \(current) to \(new), error \(String(describing: error))", level: .info)
+    /// Updates ``current`` and emits a status change event.
+    private func changeStatus(to new: RoomLifecycle) {
+        logger.log(message: "Transitioning from \(current) to \(new)", level: .info)
         let previous = current
         current = new
-        self.error = error
-        let statusChange = RoomStatusChange(current: current, previous: previous, error: error)
+        let statusChange = RoomStatusChange(current: current, previous: previous)
         emitStatusChange(statusChange)
     }
 
@@ -343,14 +341,14 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
                 case .suspended:
                     // CHA-RL1h2
                     let error = ARTErrorInfo(chatError: .attachmentFailed(feature: contributor.feature, underlyingError: contributorAttachError))
-                    changeStatus(to: .suspended, error: error)
+                    changeStatus(to: .suspended(error: error))
 
                     // CHA-RL1h3
                     throw error
                 case .failed:
                     // CHA-RL1h4
                     let error = ARTErrorInfo(chatError: .attachmentFailed(feature: contributor.feature, underlyingError: contributorAttachError))
-                    changeStatus(to: .failed, error: error)
+                    changeStatus(to: .failed(error: error))
 
                     // CHA-RL1h5
                     // TODO: Implement the "asynchronously with respect to CHA-RL1h4" part of CHA-RL1h5 (https://github.com/ably-labs/ably-chat-swift/issues/50)
@@ -450,8 +448,8 @@ internal actor RoomLifecycleManager<Contributor: RoomLifecycleContributor> {
                     }
 
                     // This check is CHA-RL2h2
-                    if current != .failed {
-                        changeStatus(to: .failed, error: error)
+                    if !current.isFailed {
+                        changeStatus(to: .failed(error: error))
                     }
                 default:
                     // CHA-RL2h3: Retry until detach succeeds, with a pause before each attempt

Sources/AblyChat/RoomStatus.swift

@@ -2,33 +2,107 @@ import Ably
 
 public protocol RoomStatus: AnyObject, Sendable {
     var current: RoomLifecycle { get async }
-    // TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap
-    var error: ARTErrorInfo? { get async }
     func onChange(bufferingPolicy: BufferingPolicy) async -> Subscription<RoomStatusChange>
 }
 
-public enum RoomLifecycle: Sendable {
+public enum RoomLifecycle: Sendable, Equatable {
     case initialized
     case attaching
     case attached
     case detaching
     case detached
-    case suspended
-    case failed
+    case suspended(error: ARTErrorInfo)
+    case failed(error: ARTErrorInfo)
     case releasing
     case released
+
+    // Helpers to allow us to test whether a `RoomLifecycle` value has a certain case, without caring about the associated value. These are useful for in contexts where we want to use a `Bool` to communicate a case. For example:
+    //
+    // 1. testing (e.g.  `#expect(status.isFailed)`)
+    // 2. testing that a status does _not_ have a particular case (e.g. if !status.isFailed), which a `case` statement cannot succinctly express
+    //
+    // TODO: These could probably be written as a macro instead to avoid all this boilerplate; by no means pressing, but investigate in https://github.com/ably-labs/ably-chat-swift/issues/71
+
+    public var isInitialized: Bool {
+        if case .initialized = self {
+            true
+        } else {
+            false
+        }
+    }
+
+    public var isAttaching: Bool {
+        if case .attaching = self {
+            true
+        } else {
+            false
+        }
+    }
+
+    public var isAttached: Bool {
+        if case .attached = self {
+            true
+        } else {
+            false
+        }
+    }
+
+    public var isDetaching: Bool {
+        if case .detaching = self {
+            true
+        } else {
+            false
+        }
+    }
+
+    public var isDetached: Bool {
+        if case .detached = self {
+            true
+        } else {
+            false
+        }
+    }
+
+    public var isSuspended: Bool {
+        if case .suspended = self {
+            true
+        } else {
+            false
+        }
+    }
+
+    public var isFailed: Bool {
+        if case .failed = self {
+            true
+        } else {
+            false
+        }
+    }
+
+    public var isReleasing: Bool {
+        if case .releasing = self {
+            true
+        } else {
+            false
+        }
+    }
+
+    public var isReleased: Bool {
+        if case .released = self {
+            true
+        } else {
+            false
+        }
+    }
 }
 
 public struct RoomStatusChange: Sendable {
     public var current: RoomLifecycle
     public var previous: RoomLifecycle
-    // TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap
-    public var error: ARTErrorInfo?
 
-    public init(current: RoomLifecycle, previous: RoomLifecycle, error: ARTErrorInfo? = nil) {
+    public init(current: RoomLifecycle, previous: RoomLifecycle) {
         self.current = current
         self.previous = previous
-        self.error = error
     }
 }
 

Tests/AblyChatTests/Helpers/RoomLifecycle+Error.swift

@@ -0,0 +1,21 @@
+import Ably
+import AblyChat
+
+extension RoomLifecycle {
+    var error: ARTErrorInfo? {
+        switch self {
+        case let .failed(error):
+            error
+        case let .suspended(error):
+            error
+        case .initialized,
+             .attached,
+             .attaching,
+             .detached,
+             .detaching,
+             .releasing,
+             .released:
+            nil
+        }
+    }
+}

Tests/AblyChatTests/Helpers/Subscription+RoomStatusChange.swift

@@ -0,0 +1,34 @@
+import Ably
+import AblyChat
+
+/// Extensions for filtering a subscription by a given case, and then providing access to the values associated with these cases.
+///
+/// This provides better ergonomics than writing e.g. `failedStatusChange = await subscription.first { $0.isSuspended }`, because it means that you don’t have to write another `if case` (or equivalent) to get access to the associated value of `failedStatusChange.current`.
+extension Subscription where Element == RoomStatusChange {
+    struct StatusChangeWithError {
+        /// A status change whose `current` has an associated error; ``error`` provides access to this error.
+        var statusChange: RoomStatusChange
+        /// The error associated with `statusChange.current`.
+        var error: ARTErrorInfo
+    }
+
+    func suspendedElements() async -> AsyncCompactMapSequence<Subscription<RoomStatusChange>, Subscription<RoomStatusChange>.StatusChangeWithError> {
+        compactMap { statusChange in
+            if case let .suspended(error) = statusChange.current {
+                StatusChangeWithError(statusChange: statusChange, error: error)
+            } else {
+                nil
+            }
+        }
+    }
+
+    func failedElements() async -> AsyncCompactMapSequence<Subscription<RoomStatusChange>, Subscription<RoomStatusChange>.StatusChangeWithError> {
+        compactMap { statusChange in
+            if case let .failed(error) = statusChange.current {
+                StatusChangeWithError(statusChange: statusChange, error: error)
+            } else {
+                nil
+            }
+        }
+    }
+}

Tests/AblyChatTests/RoomLifecycleManagerTests.swift

@@ -79,13 +79,6 @@ struct RoomLifecycleManagerTests {
         #expect(await manager.current == .initialized)
     }
 
-    @Test
-    func error_startsAsNil() async {
-        let manager = await createManager()
-
-        #expect(await manager.error == nil)
-    }
-
     // MARK: - ATTACH operation
 
     // @spec CHA-RL1a
@@ -229,7 +222,7 @@ struct RoomLifecycleManagerTests {
         let manager = await createManager(contributors: contributors)
 
         let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded)
-        async let maybeSuspendedStatusChange = statusChangeSubscription.first { $0.current == .suspended }
+        async let maybeSuspendedStatusChange = statusChangeSubscription.suspendedElements().first { _ in true }
 
         // When: `performAttachOperation()` is called on the lifecycle manager
         async let roomAttachResult: Void = manager.performAttachOperation()
@@ -241,7 +234,7 @@ struct RoomLifecycleManagerTests {
         // 3. the room attach operation fails with this same error
         let suspendedStatusChange = try #require(await maybeSuspendedStatusChange)
 
-        #expect(await manager.current == .suspended)
+        #expect(await manager.current.isSuspended)
 
         var roomAttachError: Error?
         do {
@@ -250,7 +243,7 @@ struct RoomLifecycleManagerTests {
             roomAttachError = error
         }
 
-        for error in await [suspendedStatusChange.error, manager.error, roomAttachError] {
+        for error in await [suspendedStatusChange.error, manager.current.error, roomAttachError] {
             #expect(isChatError(error, withCode: .messagesAttachmentFailed, cause: contributorAttachError))
         }
     }
@@ -280,7 +273,7 @@ struct RoomLifecycleManagerTests {
         let manager = await createManager(contributors: contributors)
 
         let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded)
-        async let maybeFailedStatusChange = statusChangeSubscription.first { $0.current == .failed }
+        async let maybeFailedStatusChange = statusChangeSubscription.failedElements().first { _ in true }
 
         // When: `performAttachOperation()` is called on the lifecycle manager
         async let roomAttachResult: Void = manager.performAttachOperation()
@@ -291,7 +284,7 @@ struct RoomLifecycleManagerTests {
         // 3. the room attach operation fails with this same error
         let failedStatusChange = try #require(await maybeFailedStatusChange)
 
-        #expect(await manager.current == .failed)
+        #expect(await manager.current.isFailed)
 
         var roomAttachError: Error?
         do {
@@ -300,7 +293,7 @@ struct RoomLifecycleManagerTests {
             roomAttachError = error
         }
 
-        for error in await [failedStatusChange.error, manager.error, roomAttachError] {
+        for error in await [failedStatusChange.error, manager.current.error, roomAttachError] {
             #expect(isChatError(error, withCode: .messagesAttachmentFailed, cause: contributorAttachError))
         }
     }
@@ -429,7 +422,11 @@ struct RoomLifecycleManagerTests {
     @Test
     func detach_whenFailed() async throws {
         // Given: A RoomLifecycleManager in the FAILED state
-        let manager = await createManager(forTestingWhatHappensWhenCurrentlyIn: .failed)
+        let manager = await createManager(
+            forTestingWhatHappensWhenCurrentlyIn: .failed(
+                error: .createUnknownError() /* arbitrary */
+            )
+        )
 
         // When: `performAttachOperation()` is called on the lifecycle manager
         // Then: It throws a roomInFailedState error
@@ -507,7 +504,7 @@ struct RoomLifecycleManagerTests {
         let manager = await createManager(contributors: contributors)
 
         let statusChangeSubscription = await manager.onChange(bufferingPolicy: .unbounded)
-        async let maybeFailedStatusChange = statusChangeSubscription.first { $0.current == .failed }
+        async let maybeFailedStatusChange = statusChangeSubscription.failedElements().first { _ in true }
 
         // When: `performDetachOperation()` is called on the lifecycle manager
         let maybeRoomDetachError: Error?
@@ -845,7 +842,7 @@ struct RoomLifecycleManagerTests {
         let manager = await createManager(forTestingWhatHappensWhenHasOperationInProgress: false, contributors: contributors)
 
         let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded)
-        async let failedStatusChange = roomStatusSubscription.first { $0.current == .failed }
+        async let failedStatusChange = roomStatusSubscription.failedElements().first { _ in true }
 
         // When: A contributor emits an FAILED event
         let contributorStateChange = ARTChannelStateChange(
@@ -864,7 +861,7 @@ struct RoomLifecycleManagerTests {
         // - the room status transitions to failed, with the error of the status change being the `reason` of the contributor FAILED event
         // - and it calls `detach` on all contributors
         _ = try #require(await failedStatusChange)
-        #expect(await manager.current == .failed)
+        #expect(await manager.current.isFailed)
 
         for contributor in contributors {
             #expect(await contributor.channel.detachCallCount == 1)
@@ -946,24 +943,24 @@ struct RoomLifecycleManagerTests {
         let manager = await createManager(forTestingWhatHappensWhenHasOperationInProgress: false, contributors: [contributor])
 
         let roomStatusSubscription = await manager.onChange(bufferingPolicy: .unbounded)
-        async let maybeSuspendedRoomStatusChange = roomStatusSubscription.first { $0.current == .suspended }
+        async let maybeSuspendedRoomStatusChange = roomStatusSubscription.suspendedElements().first { _ in true }
 
         // When: A contributor emits a state change to SUSPENDED
+        let contributorStateChangeReason = ARTErrorInfo(domain: "SomeDomain", code: 123) // arbitrary
         let contributorStateChange = ARTChannelStateChange(
             current: .suspended,
             previous: .attached, // arbitrary
             event: .suspended,
-            reason: ARTErrorInfo(domain: "SomeDomain", code: 123), // arbitrary
+            reason: contributorStateChangeReason,
             resumed: false // arbitrary
         )
 
         await contributor.channel.emitStateChange(contributorStateChange)
 
         // Then: The room transitions to SUSPENDED, and this state change has error equal to the contributor state change’s `reason`
         let suspendedRoomStatusChange = try #require(await maybeSuspendedRoomStatusChange)
-        #expect(suspendedRoomStatusChange.error === contributorStateChange.reason)
+        #expect(suspendedRoomStatusChange.error === contributorStateChangeReason)
 
-        #expect(await manager.current == .suspended)
-        #expect(await manager.error === contributorStateChange.reason)
+        #expect(await manager.current == .suspended(error: contributorStateChangeReason))
     }
 }