Merge pull request #408 from ably/406-align-RoomStatus-with-ConnectionStatus

[ECO-5602] Make `Room`'s status API consistent with `Connection` and JS

Author: lawrence-forooghian

Example/AblyChatExample/ContentView.swift

@@ -377,11 +377,11 @@ struct ContentView: View {
     func subscribeToRoomStatus(room: any Room) {
         room.onStatusChange { status in
             withAnimation {
-                if status.current.isAttaching {
+                if status.current == .attaching {
                     statusInfo = "\(status.current)...".capitalized
                 } else {
                     statusInfo = "\(status.current)".capitalized
-                    if status.current.isAttached {
+                    if status.current == .attached {
                         after(1) {
                             withAnimation {
                                 statusInfo = ""

Example/AblyChatExample/Mocks/MockClients.swift

@@ -72,11 +72,14 @@ class MockRoom: Room {
     }
 
     var status: RoomStatus = .initialized
+    var error: ARTErrorInfo?
 
     private func randomStatusInterval() -> Double { 8.0 }
 
     private let randomStatusChange = { @Sendable in
-        RoomStatusChange(current: [.attached(error: nil), .attached(error: nil), .attached(error: nil), .attached(error: nil), .attaching(error: nil), .attaching(error: nil), .suspended(error: .createUnknownError())].randomElement()!, previous: .attaching(error: nil))
+        let newStatus: RoomStatus = [.attached, .attached, .attached, .attached, .attaching, .attaching, .suspended].randomElement()!
+        let error: ARTErrorInfo? = (newStatus == .suspended) ? ARTErrorInfo.createUnknownError() : nil
+        return RoomStatusChange(current: newStatus, previous: .attaching, error: error)
     }
 
     func attach() async throws(ARTErrorInfo) {
@@ -95,7 +98,10 @@ class MockRoom: Room {
                 return false
             }
             if needNext {
-                callback(randomStatusChange())
+                let statusChange = randomStatusChange()
+                status = statusChange.current
+                error = statusChange.error
+                callback(statusChange)
             }
             return needNext
         }

Sources/AblyChat/Connection.swift

@@ -13,7 +13,6 @@ public protocol Connection: AnyObject, Sendable {
      */
     var status: ConnectionStatus { get }
 
-    // TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap
     /**
      * The current error, if any, that caused the connection to enter the current status.
      */
@@ -143,7 +142,6 @@ public struct ConnectionStatusChange: Sendable {
      */
     public var previous: ConnectionStatus
 
-    // TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap
     /**
      * An error that provides a reason why the connection has
      * entered the new status, if applicable.

Sources/AblyChat/Room.swift

@@ -79,6 +79,11 @@ public protocol Room<Channel>: AnyObject, Sendable {
      */
     var status: RoomStatus { get }
 
+    /**
+     * The current error, if any, that caused the room to enter the current status.
+     */
+    var error: ARTErrorInfo? { get }
+
     /**
      * Subscribes a given listener to the room status changes.
      *
@@ -216,12 +221,19 @@ public struct RoomStatusChange: Sendable {
      */
     public var previous: RoomStatus
 
+    /**
+     * An error that provides a reason why the room has
+     * entered the new status, if applicable.
+     */
+    public var error: ARTErrorInfo?
+
     /// Memberwise initializer to create a `RoomStatusChange`.
     ///
     /// - Note: You should not need to use this initializer when using the Chat SDK. It is exposed only to allow users to create mock versions of the SDK's protocols.
-    public init(current: RoomStatus, previous: RoomStatus) {
+    public init(current: RoomStatus, previous: RoomStatus, error: ARTErrorInfo? = nil) {
         self.current = current
         self.previous = previous
+        self.error = error
     }
 }
 
@@ -397,6 +409,10 @@ internal class DefaultRoom<Realtime: InternalRealtimeClientProtocol, LifecycleMa
         lifecycleManager.roomStatus
     }
 
+    internal var error: ARTErrorInfo? {
+        lifecycleManager.error
+    }
+
     // MARK: - Discontinuities
 
     @discardableResult

Sources/AblyChat/RoomLifecycleManager.swift

@@ -8,6 +8,7 @@ internal protocol RoomLifecycleManager: Sendable {
     func performDetachOperation() async throws(InternalError)
     func performReleaseOperation() async
     var roomStatus: RoomStatus { get }
+    var error: ARTErrorInfo? { get }
     @discardableResult
     func onRoomStatusChange(_ callback: @escaping @MainActor (RoomStatusChange) -> Void) -> StatusSubscription
 
@@ -53,28 +54,22 @@ internal final class DefaultRoomLifecycleManagerFactory: RoomLifecycleManagerFac
 }
 
 private extension RoomStatus {
-    init(channelState: ARTRealtimeChannelState, error: ARTErrorInfo?) {
+    init(channelState: ARTRealtimeChannelState) {
         switch channelState {
         case .initialized:
             self = .initialized
         case .attaching:
-            self = .attaching(error: error)
+            self = .attaching
         case .attached:
-            self = .attached(error: error)
+            self = .attached
         case .detaching:
-            self = .detaching(error: error)
+            self = .detaching
         case .detached:
-            self = .detached(error: error)
+            self = .detached
         case .suspended:
-            guard let error else {
-                fatalError("Expected an error with SUSPENDED channel state")
-            }
-            self = .suspended(error: error)
+            self = .suspended
         case .failed:
-            guard let error else {
-                fatalError("Expected an error with FAILED channel state")
-            }
-            self = .failed(error: error)
+            self = .failed
         @unknown default:
             fatalError("Unknown channel state \(channelState)")
         }
@@ -91,6 +86,7 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
     // MARK: - Variable properties
 
     internal private(set) var roomStatus: RoomStatus
+    internal private(set) var error: ARTErrorInfo?
     private var currentOperationID: UUID?
 
     // CHA-RL13
@@ -185,12 +181,13 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
     }
 
     /// Updates ``roomStatus`` and emits a status change event.
-    private func changeStatus(to new: RoomStatus) {
+    private func changeStatus(to new: RoomStatus, error: ARTErrorInfo? = nil) {
         logger.log(message: "Transitioning from \(roomStatus) to \(new)", level: .info)
         let previous = roomStatus
         roomStatus = new
+        self.error = error
 
-        let statusChange = RoomStatusChange(current: roomStatus, previous: previous)
+        let statusChange = RoomStatusChange(current: roomStatus, previous: previous, error: error)
         roomStatusChangeSubscriptions.emit(statusChange)
     }
 
@@ -203,7 +200,7 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
         // CHA-RL11b
         if event.event != .update, !hasOperationInProgress {
             // CHA-RL11c
-            changeStatus(to: .init(channelState: event.current, error: event.reason))
+            changeStatus(to: .init(channelState: event.current), error: event.reason)
         }
 
         switch event.event {
@@ -412,7 +409,7 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
         defer { currentOperationID = nil }
 
         // CHA-RL1e
-        changeStatus(to: .attaching(error: nil))
+        changeStatus(to: .attaching, error: nil)
 
         // CHA-RL1k
         do {
@@ -421,14 +418,14 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
             // CHA-RL1k2, CHA-RL1k3
             let channelState = channel.state
             logger.log(message: "Failed to attach channel, error \(error), channel now in \(channelState)", level: .info)
-            changeStatus(to: .init(channelState: channelState, error: error.toARTErrorInfo()))
+            changeStatus(to: .init(channelState: channelState), error: error.toARTErrorInfo())
             throw error
         }
 
         // CHA-RL1k1
         isExplicitlyDetached = false
         hasAttachedOnce = true
-        changeStatus(to: .attached(error: nil))
+        changeStatus(to: .attached, error: nil)
     }
 
     // MARK: - DETACH operation
@@ -478,7 +475,7 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
         defer { currentOperationID = nil }
 
         // CHA-RL2j
-        changeStatus(to: .detaching(error: nil))
+        changeStatus(to: .detaching, error: nil)
 
         // CHA-RL2k
         do {
@@ -487,13 +484,13 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
             // CHA-RL2k2, CHA-RL2k3
             let channelState = channel.state
             logger.log(message: "Failed to detach channel, error \(error), channel now in \(channelState)", level: .info)
-            changeStatus(to: .init(channelState: channelState, error: error.toARTErrorInfo()))
+            changeStatus(to: .init(channelState: channelState), error: error.toARTErrorInfo())
             throw error
         }
 
         // CHA-RL2k1
         isExplicitlyDetached = true
-        changeStatus(to: .detached(error: nil))
+        changeStatus(to: .detached, error: nil)
     }
 
     // MARK: - RELEASE operation
@@ -609,9 +606,9 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
             }
             nextRoomStatusSubscription.off()
             // CHA-RL9b
-            guard case let .attached(error) = nextRoomStatusChange.current, error == nil else {
+            guard case .attached = nextRoomStatusChange.current, nextRoomStatusChange.error == nil else {
                 // CHA-RL9c
-                throw ARTErrorInfo(chatError: .roomTransitionedToInvalidStateForPresenceOperation(cause: nextRoomStatusChange.current.error)).toInternalError()
+                throw ARTErrorInfo(chatError: .roomTransitionedToInvalidStateForPresenceOperation(cause: nextRoomStatusChange.error)).toInternalError()
             }
         case .attached:
             // CHA-PR3e, CHA-PR10e, CHA-PR6d

Sources/AblyChat/RoomStatus.swift

@@ -12,32 +12,32 @@ public enum RoomStatus: Sendable {
     /**
      * The library is currently attempting to attach the room.
      */
-    case attaching(error: ARTErrorInfo?)
+    case attaching
 
     /**
      * The room is currently attached and receiving events.
      */
-    case attached(error: ARTErrorInfo?)
+    case attached
 
     /**
      * The room is currently detaching and will not receive events.
      */
-    case detaching(error: ARTErrorInfo?)
+    case detaching
 
     /**
      * The room is currently detached and will not receive events.
      */
-    case detached(error: ARTErrorInfo?)
+    case detached
 
     /**
      * The room is in an extended state of detachment, but will attempt to re-attach when able.
      */
-    case suspended(error: ARTErrorInfo)
+    case suspended
 
     /**
      * The room is currently detached and will not attempt to re-attach. User intervention is required.
      */
-    case failed(error: ARTErrorInfo)
+    case failed
 
     /**
      * The room is in the process of releasing. Attempting to use a room in this state may result in undefined behavior.
@@ -48,102 +48,4 @@ public enum RoomStatus: Sendable {
      * The room has been released and is no longer usable.
      */
     case released
-
-    internal var error: ARTErrorInfo? {
-        switch self {
-        case let .attaching(error):
-            error
-        case let .attached(error):
-            error
-        case let .detaching(error):
-            error
-        case let .detached(error):
-            error
-        case let .suspended(error):
-            error
-        case let .failed(error):
-            error
-        case .initialized,
-             .releasing,
-             .released:
-            nil
-        }
-    }
-
-    // Helpers to allow us to test whether a `RoomStatus` 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
-
-    // swiftlint:disable:next missing_docs
-    public var isAttaching: Bool {
-        if case .attaching = self {
-            true
-        } else {
-            false
-        }
-    }
-
-    // swiftlint:disable:next missing_docs
-    public var isAttached: Bool {
-        if case .attached = self {
-            true
-        } else {
-            false
-        }
-    }
-
-    // swiftlint:disable:next missing_docs
-    public var isDetaching: Bool {
-        if case .detaching = self {
-            true
-        } else {
-            false
-        }
-    }
-
-    // swiftlint:disable:next missing_docs
-    public var isDetached: Bool {
-        if case .detached = self {
-            true
-        } else {
-            false
-        }
-    }
-
-    // swiftlint:disable:next missing_docs
-    public var isSuspended: Bool {
-        if case .suspended = self {
-            true
-        } else {
-            false
-        }
-    }
-
-    // swiftlint:disable:next missing_docs
-    public var isFailed: Bool {
-        if case .failed = self {
-            true
-        } else {
-            false
-        }
-    }
-
-    // swiftlint:disable:next missing_docs
-    public var isReleasing: Bool {
-        if case .releasing = self {
-            true
-        } else {
-            false
-        }
-    }
-
-    // swiftlint:disable:next missing_docs
-    public var isReleased: Bool {
-        if case .released = self {
-            true
-        } else {
-            false
-        }
-    }
 }