Skip to content

Commit d1555e1

Browse files
lawrence-forooghianlawrence-forooghian
committed
Make Room's status API consistent with Connection
This undoes the public API change introduced in d51887c, in which we try to make it clear from the type system whether a given room status is guaranteed to have an error, guaranteed to have no error, or maybe has an error. I still think that the API from d51887c is the "correct" one, but I suggested making this change in JS (see our #12) and nobody seemed particularly keen to do so (particularly because the core SDKs, whose errors we are largely just reflecting, don't expose such an API and don't make these guarantees, leading us to have to guess what the correct contract is). So, change the API to match JS and our Connection type; that is, remove the RoomStatus associated types and introduce `error` properties on Room and RoomStatusChange. We can revisit this one day once we have a core SDK API that makes these guarantees (e.g. in upcoming ably-swift). Resolves #406.
1 parent d1b0655 commit d1555e1

File tree

13 files changed

+132
-283
lines changed

13 files changed

+132
-283
lines changed

‎Example/AblyChatExample/ContentView.swift‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -361,11 +361,11 @@ struct ContentView: View {
361361
func subscribeToRoomStatus(room: any Room) {
362362
room.onStatusChange { status in
363363
withAnimation {
364-
if status.current.isAttaching {
364+
if status.current == .attaching {
365365
statusInfo = "\(status.current)...".capitalized
366366
} else {
367367
statusInfo = "\(status.current)".capitalized
368-
if status.current.isAttached {
368+
if status.current == .attached {
369369
after(1) {
370370
withAnimation {
371371
statusInfo = ""

‎Example/AblyChatExample/Mocks/MockClients.swift‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -72,11 +72,14 @@ class MockRoom: Room {
7272
}
7373

7474
var status: RoomStatus = .initialized
75+
var error: ARTErrorInfo?
7576

7677
private func randomStatusInterval() -> Double { 8.0 }
7778

7879
private let randomStatusChange = { @Sendable in
79-
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))
80+
let newStatus: RoomStatus = [.attached, .attached, .attached, .attached, .attaching, .attaching, .suspended].randomElement()!
81+
let error: ARTErrorInfo? = (newStatus == .suspended) ? ARTErrorInfo.createUnknownError() : nil
82+
return RoomStatusChange(current: newStatus, previous: .attaching, error: error)
8083
}
8184

8285
func attach() async throws(ARTErrorInfo) {
@@ -95,7 +98,10 @@ class MockRoom: Room {
9598
return false
9699
}
97100
if needNext {
98-
callback(randomStatusChange())
101+
let statusChange = randomStatusChange()
102+
status = statusChange.current
103+
error = statusChange.error
104+
callback(statusChange)
99105
}
100106
return needNext
101107
}

‎Sources/AblyChat/Connection.swift‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ public protocol Connection: AnyObject, Sendable {
1313
*/
1414
var status: ConnectionStatus { get }
1515

16-
// TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap
1716
/**
1817
* The current error, if any, that caused the connection to enter the current status.
1918
*/
@@ -143,7 +142,6 @@ public struct ConnectionStatusChange: Sendable {
143142
*/
144143
public var previous: ConnectionStatus
145144

146-
// TODO: (https://github.com/ably-labs/ably-chat-swift/issues/12): consider how to avoid the need for an unwrap
147145
/**
148146
* An error that provides a reason why the connection has
149147
* entered the new status, if applicable.

‎Sources/AblyChat/Room.swift‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,11 @@ public protocol Room<Channel>: AnyObject, Sendable {
7979
*/
8080
var status: RoomStatus { get }
8181

82+
/**
83+
* The current error, if any, that caused the room to enter the current status.
84+
*/
85+
var error: ARTErrorInfo? { get }
86+
8287
/**
8388
* Subscribes a given listener to the room status changes.
8489
*
@@ -216,12 +221,19 @@ public struct RoomStatusChange: Sendable {
216221
*/
217222
public var previous: RoomStatus
218223

224+
/**
225+
* An error that provides a reason why the room has
226+
* entered the new status, if applicable.
227+
*/
228+
public var error: ARTErrorInfo?
229+
219230
/// Memberwise initializer to create a `RoomStatusChange`.
220231
///
221232
/// - 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.
222-
public init(current: RoomStatus, previous: RoomStatus) {
233+
public init(current: RoomStatus, previous: RoomStatus, error: ARTErrorInfo? = nil) {
223234
self.current = current
224235
self.previous = previous
236+
self.error = error
225237
}
226238
}
227239

@@ -397,6 +409,10 @@ internal class DefaultRoom<Realtime: InternalRealtimeClientProtocol, LifecycleMa
397409
lifecycleManager.roomStatus
398410
}
399411

412+
internal var error: ARTErrorInfo? {
413+
lifecycleManager.error
414+
}
415+
400416
// MARK: - Discontinuities
401417

402418
@discardableResult

‎Sources/AblyChat/RoomLifecycleManager.swift‎

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ internal protocol RoomLifecycleManager: Sendable {
88
func performDetachOperation() async throws(InternalError)
99
func performReleaseOperation() async
1010
var roomStatus: RoomStatus { get }
11+
var error: ARTErrorInfo? { get }
1112
@discardableResult
1213
func onRoomStatusChange(_ callback: @escaping @MainActor (RoomStatusChange) -> Void) -> StatusSubscription
1314

@@ -53,28 +54,22 @@ internal final class DefaultRoomLifecycleManagerFactory: RoomLifecycleManagerFac
5354
}
5455

5556
private extension RoomStatus {
56-
init(channelState: ARTRealtimeChannelState, error: ARTErrorInfo?) {
57+
init(channelState: ARTRealtimeChannelState) {
5758
switch channelState {
5859
case .initialized:
5960
self = .initialized
6061
case .attaching:
61-
self = .attaching(error: error)
62+
self = .attaching
6263
case .attached:
63-
self = .attached(error: error)
64+
self = .attached
6465
case .detaching:
65-
self = .detaching(error: error)
66+
self = .detaching
6667
case .detached:
67-
self = .detached(error: error)
68+
self = .detached
6869
case .suspended:
69-
guard let error else {
70-
fatalError("Expected an error with SUSPENDED channel state")
71-
}
72-
self = .suspended(error: error)
70+
self = .suspended
7371
case .failed:
74-
guard let error else {
75-
fatalError("Expected an error with FAILED channel state")
76-
}
77-
self = .failed(error: error)
72+
self = .failed
7873
@unknown default:
7974
fatalError("Unknown channel state \(channelState)")
8075
}
@@ -91,6 +86,7 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
9186
// MARK: - Variable properties
9287

9388
internal private(set) var roomStatus: RoomStatus
89+
internal private(set) var error: ARTErrorInfo?
9490
private var currentOperationID: UUID?
9591

9692
// CHA-RL13
@@ -185,12 +181,13 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
185181
}
186182

187183
/// Updates ``roomStatus`` and emits a status change event.
188-
private func changeStatus(to new: RoomStatus) {
184+
private func changeStatus(to new: RoomStatus, error: ARTErrorInfo? = nil) {
189185
logger.log(message: "Transitioning from \(roomStatus) to \(new)", level: .info)
190186
let previous = roomStatus
191187
roomStatus = new
188+
self.error = error
192189

193-
let statusChange = RoomStatusChange(current: roomStatus, previous: previous)
190+
let statusChange = RoomStatusChange(current: roomStatus, previous: previous, error: error)
194191
roomStatusChangeSubscriptions.emit(statusChange)
195192
}
196193

@@ -203,7 +200,7 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
203200
// CHA-RL11b
204201
if event.event != .update, !hasOperationInProgress {
205202
// CHA-RL11c
206-
changeStatus(to: .init(channelState: event.current, error: event.reason))
203+
changeStatus(to: .init(channelState: event.current), error: event.reason)
207204
}
208205

209206
switch event.event {
@@ -412,7 +409,7 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
412409
defer { currentOperationID = nil }
413410

414411
// CHA-RL1e
415-
changeStatus(to: .attaching(error: nil))
412+
changeStatus(to: .attaching, error: nil)
416413

417414
// CHA-RL1k
418415
do {
@@ -421,14 +418,14 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
421418
// CHA-RL1k2, CHA-RL1k3
422419
let channelState = channel.state
423420
logger.log(message: "Failed to attach channel, error \(error), channel now in \(channelState)", level: .info)
424-
changeStatus(to: .init(channelState: channelState, error: error.toARTErrorInfo()))
421+
changeStatus(to: .init(channelState: channelState), error: error.toARTErrorInfo())
425422
throw error
426423
}
427424

428425
// CHA-RL1k1
429426
isExplicitlyDetached = false
430427
hasAttachedOnce = true
431-
changeStatus(to: .attached(error: nil))
428+
changeStatus(to: .attached, error: nil)
432429
}
433430

434431
// MARK: - DETACH operation
@@ -478,7 +475,7 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
478475
defer { currentOperationID = nil }
479476

480477
// CHA-RL2j
481-
changeStatus(to: .detaching(error: nil))
478+
changeStatus(to: .detaching, error: nil)
482479

483480
// CHA-RL2k
484481
do {
@@ -487,13 +484,13 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
487484
// CHA-RL2k2, CHA-RL2k3
488485
let channelState = channel.state
489486
logger.log(message: "Failed to detach channel, error \(error), channel now in \(channelState)", level: .info)
490-
changeStatus(to: .init(channelState: channelState, error: error.toARTErrorInfo()))
487+
changeStatus(to: .init(channelState: channelState), error: error.toARTErrorInfo())
491488
throw error
492489
}
493490

494491
// CHA-RL2k1
495492
isExplicitlyDetached = true
496-
changeStatus(to: .detached(error: nil))
493+
changeStatus(to: .detached, error: nil)
497494
}
498495

499496
// MARK: - RELEASE operation
@@ -609,9 +606,9 @@ internal class DefaultRoomLifecycleManager: RoomLifecycleManager {
609606
}
610607
nextRoomStatusSubscription.off()
611608
// CHA-RL9b
612-
guard case let .attached(error) = nextRoomStatusChange.current, error == nil else {
609+
guard case .attached = nextRoomStatusChange.current, nextRoomStatusChange.error == nil else {
613610
// CHA-RL9c
614-
throw ARTErrorInfo(chatError: .roomTransitionedToInvalidStateForPresenceOperation(cause: nextRoomStatusChange.current.error)).toInternalError()
611+
throw ARTErrorInfo(chatError: .roomTransitionedToInvalidStateForPresenceOperation(cause: nextRoomStatusChange.error)).toInternalError()
615612
}
616613
case .attached:
617614
// CHA-PR3e, CHA-PR10e, CHA-PR6d

‎Sources/AblyChat/RoomStatus.swift‎

Lines changed: 6 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -12,32 +12,32 @@ public enum RoomStatus: Sendable {
1212
/**
1313
* The library is currently attempting to attach the room.
1414
*/
15-
case attaching(error: ARTErrorInfo?)
15+
case attaching
1616

1717
/**
1818
* The room is currently attached and receiving events.
1919
*/
20-
case attached(error: ARTErrorInfo?)
20+
case attached
2121

2222
/**
2323
* The room is currently detaching and will not receive events.
2424
*/
25-
case detaching(error: ARTErrorInfo?)
25+
case detaching
2626

2727
/**
2828
* The room is currently detached and will not receive events.
2929
*/
30-
case detached(error: ARTErrorInfo?)
30+
case detached
3131

3232
/**
3333
* The room is in an extended state of detachment, but will attempt to re-attach when able.
3434
*/
35-
case suspended(error: ARTErrorInfo)
35+
case suspended
3636

3737
/**
3838
* The room is currently detached and will not attempt to re-attach. User intervention is required.
3939
*/
40-
case failed(error: ARTErrorInfo)
40+
case failed
4141

4242
/**
4343
* 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 {
4848
* The room has been released and is no longer usable.
4949
*/
5050
case released
51-
52-
internal var error: ARTErrorInfo? {
53-
switch self {
54-
case let .attaching(error):
55-
error
56-
case let .attached(error):
57-
error
58-
case let .detaching(error):
59-
error
60-
case let .detached(error):
61-
error
62-
case let .suspended(error):
63-
error
64-
case let .failed(error):
65-
error
66-
case .initialized,
67-
.releasing,
68-
.released:
69-
nil
70-
}
71-
}
72-
73-
// 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:
74-
//
75-
// 1. testing (e.g. `#expect(status.isFailed)`)
76-
// 2. testing that a status does _not_ have a particular case (e.g. if !status.isFailed), which a `case` statement cannot succinctly express
77-
78-
// swiftlint:disable:next missing_docs
79-
public var isAttaching: Bool {
80-
if case .attaching = self {
81-
true
82-
} else {
83-
false
84-
}
85-
}
86-
87-
// swiftlint:disable:next missing_docs
88-
public var isAttached: Bool {
89-
if case .attached = self {
90-
true
91-
} else {
92-
false
93-
}
94-
}
95-
96-
// swiftlint:disable:next missing_docs
97-
public var isDetaching: Bool {
98-
if case .detaching = self {
99-
true
100-
} else {
101-
false
102-
}
103-
}
104-
105-
// swiftlint:disable:next missing_docs
106-
public var isDetached: Bool {
107-
if case .detached = self {
108-
true
109-
} else {
110-
false
111-
}
112-
}
113-
114-
// swiftlint:disable:next missing_docs
115-
public var isSuspended: Bool {
116-
if case .suspended = self {
117-
true
118-
} else {
119-
false
120-
}
121-
}
122-
123-
// swiftlint:disable:next missing_docs
124-
public var isFailed: Bool {
125-
if case .failed = self {
126-
true
127-
} else {
128-
false
129-
}
130-
}
131-
132-
// swiftlint:disable:next missing_docs
133-
public var isReleasing: Bool {
134-
if case .releasing = self {
135-
true
136-
} else {
137-
false
138-
}
139-
}
140-
141-
// swiftlint:disable:next missing_docs
142-
public var isReleased: Bool {
143-
if case .released = self {
144-
true
145-
} else {
146-
false
147-
}
148-
}
14951
}

0 commit comments

Comments
 (0)