Skip to content

Commit 30cfd30

Browse files
lawrence-forooghianlawrence-forooghian
committed
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] ably/specification#239 (comment)
1 parent 9c87a98 commit 30cfd30

13 files changed

+43
-49
lines changed

‎Example/AblyChatExample/Mocks/MockClients.swift

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,7 @@ actor MockMessages: Messages {
142142
return message
143143
}
144144

145-
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
145+
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
146146
fatalError("Not yet implemented")
147147
}
148148
}
@@ -193,7 +193,7 @@ actor MockRoomReactions: RoomReactions {
193193
.init(mockAsyncSequence: createSubscription())
194194
}
195195

196-
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
196+
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
197197
fatalError("Not yet implemented")
198198
}
199199
}
@@ -242,7 +242,7 @@ actor MockTyping: Typing {
242242
}
243243
}
244244

245-
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
245+
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
246246
fatalError("Not yet implemented")
247247
}
248248
}
@@ -346,7 +346,7 @@ actor MockPresence: Presence {
346346
.init(mockAsyncSequence: createSubscription())
347347
}
348348

349-
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
349+
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
350350
fatalError("Not yet implemented")
351351
}
352352
}
@@ -381,7 +381,7 @@ actor MockOccupancy: Occupancy {
381381
OccupancyEvent(connections: 10, presenceMembers: 5)
382382
}
383383

384-
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
384+
func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
385385
fatalError("Not yet implemented")
386386
}
387387
}

‎Sources/AblyChat/DefaultMessages.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@ internal final class DefaultMessages: Messages, EmitsDiscontinuities {
107107
}
108108

109109
// (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.
110-
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
110+
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
111111
await featureChannel.subscribeToDiscontinuities()
112112
}
113113

‎Sources/AblyChat/DefaultOccupancy.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ internal final class DefaultOccupancy: Occupancy, EmitsDiscontinuities {
5050
}
5151

5252
// (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.
53-
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
53+
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
5454
await featureChannel.subscribeToDiscontinuities()
5555
}
5656
}

‎Sources/AblyChat/DefaultPresence.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ internal final class DefaultPresence: Presence, EmitsDiscontinuities {
194194
}
195195

196196
// (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.
197-
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
197+
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
198198
await featureChannel.subscribeToDiscontinuities()
199199
}
200200

‎Sources/AblyChat/DefaultRoomLifecycleContributor.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import Ably
33
internal actor DefaultRoomLifecycleContributor: RoomLifecycleContributor, EmitsDiscontinuities, CustomDebugStringConvertible {
44
internal nonisolated let channel: DefaultRoomLifecycleContributorChannel
55
internal nonisolated let feature: RoomFeature
6-
private var discontinuitySubscriptions: [Subscription<ARTErrorInfo>] = []
6+
private var discontinuitySubscriptions: [Subscription<ARTErrorInfo?>] = []
77

88
internal init(channel: DefaultRoomLifecycleContributorChannel, feature: RoomFeature) {
99
self.channel = channel
@@ -12,14 +12,14 @@ internal actor DefaultRoomLifecycleContributor: RoomLifecycleContributor, EmitsD
1212

1313
// MARK: - Discontinuities
1414

15-
internal func emitDiscontinuity(_ error: ARTErrorInfo) {
15+
internal func emitDiscontinuity(_ error: ARTErrorInfo?) {
1616
for subscription in discontinuitySubscriptions {
1717
subscription.emit(error)
1818
}
1919
}
2020

21-
internal func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo> {
22-
let subscription = Subscription<ARTErrorInfo>(bufferingPolicy: .unbounded)
21+
internal func subscribeToDiscontinuities() -> Subscription<ARTErrorInfo?> {
22+
let subscription = Subscription<ARTErrorInfo?>(bufferingPolicy: .unbounded)
2323
// TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36)
2424
discontinuitySubscriptions.append(subscription)
2525
return subscription

‎Sources/AblyChat/DefaultRoomReactions.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ internal final class DefaultRoomReactions: RoomReactions, EmitsDiscontinuities {
8080
}
8181

8282
// (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.
83-
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
83+
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
8484
await featureChannel.subscribeToDiscontinuities()
8585
}
8686

‎Sources/AblyChat/DefaultTyping.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ internal final class DefaultTyping: Typing {
160160
}
161161

162162
// (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.
163-
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
163+
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
164164
await featureChannel.subscribeToDiscontinuities()
165165
}
166166

‎Sources/AblyChat/EmitsDiscontinuities.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import Ably
22

33
public protocol EmitsDiscontinuities {
4-
func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo>
4+
func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?>
55
}

‎Sources/AblyChat/RoomFeature.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ internal struct DefaultFeatureChannel: FeatureChannel {
5858
internal var contributor: DefaultRoomLifecycleContributor
5959
internal var roomLifecycleManager: RoomLifecycleManager
6060

61-
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
61+
internal func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
6262
await contributor.subscribeToDiscontinuities()
6363
}
6464

‎Sources/AblyChat/RoomLifecycleManager.swift

Lines changed: 12 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ internal protocol RoomLifecycleContributor: Identifiable, Sendable {
3737
/// Informs the contributor that there has been a break in channel continuity, which it should inform library users about.
3838
///
3939
/// 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).
40-
func emitDiscontinuity(_ error: ARTErrorInfo) async
40+
func emitDiscontinuity(_ error: ARTErrorInfo?) async
4141
}
4242

4343
internal protocol RoomLifecycleManager: Sendable {
@@ -111,7 +111,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
111111
#if DEBUG
112112
internal init(
113113
testsOnly_status status: Status? = nil,
114-
testsOnly_pendingDiscontinuityEvents pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]]? = nil,
114+
testsOnly_pendingDiscontinuityEvents pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo?]]? = nil,
115115
testsOnly_idsOfContributorsWithTransientDisconnectTimeout idsOfContributorsWithTransientDisconnectTimeout: Set<Contributor.ID>? = nil,
116116
contributors: [Contributor],
117117
logger: InternalLogger,
@@ -130,7 +130,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
130130

131131
private init(
132132
status: Status?,
133-
pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]]?,
133+
pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo?]]?,
134134
idsOfContributorsWithTransientDisconnectTimeout: Set<Contributor.ID>?,
135135
contributors: [Contributor],
136136
logger: InternalLogger,
@@ -263,7 +263,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
263263
}
264264

265265
// TODO: Not clear whether there can be multiple or just one (asked in https://github.com/ably/specification/pull/200/files#r1781927850)
266-
var pendingDiscontinuityEvents: [ARTErrorInfo] = []
266+
var pendingDiscontinuityEvents: [ARTErrorInfo?] = []
267267
var transientDisconnectTimeout: TransientDisconnectTimeout?
268268
/// Whether a state change to `ATTACHED` has already been observed for this contributor.
269269
var hasBeenAttached: Bool
@@ -279,7 +279,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
279279

280280
init(
281281
contributors: [Contributor],
282-
pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo]],
282+
pendingDiscontinuityEvents: [Contributor.ID: [ARTErrorInfo?]],
283283
idsOfContributorsWithTransientDisconnectTimeout: Set<Contributor.ID>
284284
) {
285285
storage = contributors.reduce(into: [:]) { result, contributor in
@@ -397,7 +397,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
397397
return subscription
398398
}
399399

400-
internal func testsOnly_pendingDiscontinuityEvents(for contributor: Contributor) -> [ARTErrorInfo] {
400+
internal func testsOnly_pendingDiscontinuityEvents(for contributor: Contributor) -> [ARTErrorInfo?] {
401401
contributorAnnotations[contributor].pendingDiscontinuityEvents
402402
}
403403

@@ -441,19 +441,16 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
441441
break
442442
}
443443

444-
guard let reason = stateChange.reason else {
445-
// TODO: Decide the right thing to do here (https://github.com/ably-labs/ably-chat-swift/issues/74)
446-
preconditionFailure("State change event with resumed == false should have a reason")
447-
}
444+
let reason = stateChange.reason
448445

449446
if hasOperationInProgress {
450447
// CHA-RL4a3
451-
logger.log(message: "Recording pending discontinuity event \(reason) for contributor \(contributor)", level: .info)
448+
logger.log(message: "Recording pending discontinuity event \(String(describing: reason)) for contributor \(contributor)", level: .info)
452449

453450
contributorAnnotations[contributor].pendingDiscontinuityEvents.append(reason)
454451
} else {
455452
// CHA-RL4a4
456-
logger.log(message: "Emitting discontinuity event \(reason) for contributor \(contributor)", level: .info)
453+
logger.log(message: "Emitting discontinuity event \(String(describing: reason)) for contributor \(contributor)", level: .info)
457454

458455
await contributor.emitDiscontinuity(reason)
459456
}
@@ -465,12 +462,9 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
465462
if !stateChange.resumed, hadAlreadyAttached {
466463
// CHA-RL4b1
467464

468-
guard let reason = stateChange.reason else {
469-
// TODO: Decide the right thing to do here (https://github.com/ably-labs/ably-chat-swift/issues/74)
470-
preconditionFailure("Non-initial ATTACHED state change with resumed == false should have a reason")
471-
}
465+
let reason = stateChange.reason
472466

473-
logger.log(message: "Recording pending discontinuity event \(reason) for contributor \(contributor)", level: .info)
467+
logger.log(message: "Recording pending discontinuity event \(String(describing: reason)) for contributor \(contributor)", level: .info)
474468

475469
contributorAnnotations[contributor].pendingDiscontinuityEvents.append(reason)
476470
}
@@ -861,7 +855,7 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
861855
logger.log(message: "Emitting pending discontinuity events", level: .info)
862856
for contributor in contributors {
863857
for pendingDiscontinuityEvent in contributorAnnotations[contributor].pendingDiscontinuityEvents {
864-
logger.log(message: "Emitting pending discontinuity event \(pendingDiscontinuityEvent) to contributor \(contributor)", level: .info)
858+
logger.log(message: "Emitting pending discontinuity event \(String(describing: pendingDiscontinuityEvent)) to contributor \(contributor)", level: .info)
865859
await contributor.emitDiscontinuity(pendingDiscontinuityEvent)
866860
}
867861
}

‎Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ struct DefaultRoomLifecycleManagerTests {
5454

5555
private func createManager(
5656
forTestingWhatHappensWhenCurrentlyIn status: DefaultRoomLifecycleManager<MockRoomLifecycleContributor>.Status? = nil,
57-
forTestingWhatHappensWhenHasPendingDiscontinuityEvents pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo]]? = nil,
57+
forTestingWhatHappensWhenHasPendingDiscontinuityEvents pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo?]]? = nil,
5858
forTestingWhatHappensWhenHasTransientDisconnectTimeoutForTheseContributorIDs idsOfContributorsWithTransientDisconnectTimeout: Set<MockRoomLifecycleContributor.ID>? = nil,
5959
contributors: [MockRoomLifecycleContributor] = [],
6060
clock: SimpleClock = MockSimpleClock()
@@ -262,7 +262,7 @@ struct DefaultRoomLifecycleManagerTests {
262262
func attach_uponSuccess_emitsPendingDiscontinuityEvents() async throws {
263263
// Given: A DefaultRoomLifecycleManager, all of whose contributors’ calls to `attach` succeed
264264
let contributors = (1 ... 3).map { _ in createContributor(attachBehavior: .success) }
265-
let pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo]] = [
265+
let pendingDiscontinuityEvents: [MockRoomLifecycleContributor.ID: [ARTErrorInfo?]] = [
266266
contributors[1].id: [.init(domain: "SomeDomain", code: 123) /* arbitrary */ ],
267267
contributors[2].id: [.init(domain: "SomeDomain", code: 456) /* arbitrary */ ],
268268
]
@@ -333,7 +333,7 @@ struct DefaultRoomLifecycleManagerTests {
333333
current: .attached,
334334
previous: .detached, // arbitrary
335335
event: .attached,
336-
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
336+
reason: nil // arbitrary
337337
)
338338
)
339339
)
@@ -961,7 +961,7 @@ struct DefaultRoomLifecycleManagerTests {
961961
current: .attached,
962962
previous: .attaching, // arbitrary
963963
event: .attached,
964-
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
964+
reason: nil // arbitrary
965965
)
966966
)
967967
),
@@ -1010,7 +1010,7 @@ struct DefaultRoomLifecycleManagerTests {
10101010
current: .attached,
10111011
previous: .attaching, // arbitrary
10121012
event: .attached,
1013-
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
1013+
reason: nil // arbitrary
10141014
)
10151015
)
10161016
),
@@ -1203,7 +1203,7 @@ struct DefaultRoomLifecycleManagerTests {
12031203
current: .attached,
12041204
previous: .attaching, // arbitrary
12051205
event: .attached,
1206-
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
1206+
reason: nil // arbitrary
12071207
)
12081208

12091209
return .addSubscriptionAndEmitStateChange(contributorAttachedStateChange)
@@ -1248,7 +1248,7 @@ struct DefaultRoomLifecycleManagerTests {
12481248
current: .attached,
12491249
previous: .attaching, // arbitrary
12501250
event: .attached,
1251-
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
1251+
reason: nil // arbitrary
12521252
)
12531253
) // Not related to this test, just so that the CHA-RL5d wait completes
12541254
),
@@ -1296,7 +1296,7 @@ struct DefaultRoomLifecycleManagerTests {
12961296
current: .attached,
12971297
previous: .attaching, // arbitrary
12981298
event: .attached,
1299-
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
1299+
reason: nil // arbitrary
13001300
)
13011301
) // Not related to this test, just so that the CHA-RL5d wait completes
13021302
),
@@ -1794,7 +1794,7 @@ struct DefaultRoomLifecycleManagerTests {
17941794
current: .attached,
17951795
previous: .detached, // arbitrary
17961796
event: .attached,
1797-
reason: .createUnknownError() // Not related to this test, just to avoid a crash in CHA-RL4b1 handling of this state change
1797+
reason: nil // arbitrary
17981798
)
17991799
)
18001800
)

‎Tests/AblyChatTests/Mocks/MockFeatureChannel.swift

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import Ably
44
final actor MockFeatureChannel: FeatureChannel {
55
let channel: RealtimeChannelProtocol
66
// TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36)
7-
private var discontinuitySubscriptions: [Subscription<ARTErrorInfo>] = []
7+
private var discontinuitySubscriptions: [Subscription<ARTErrorInfo?>] = []
88
private let resultOfWaitToBeAbleToPerformPresenceOperations: Result<Void, ARTErrorInfo>?
99

1010
init(
@@ -15,13 +15,13 @@ final actor MockFeatureChannel: FeatureChannel {
1515
resultOfWaitToBeAbleToPerformPresenceOperations = resultOfWaitToBeAblePerformPresenceOperations
1616
}
1717

18-
func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo> {
19-
let subscription = Subscription<ARTErrorInfo>(bufferingPolicy: .unbounded)
18+
func subscribeToDiscontinuities() async -> Subscription<ARTErrorInfo?> {
19+
let subscription = Subscription<ARTErrorInfo?>(bufferingPolicy: .unbounded)
2020
discontinuitySubscriptions.append(subscription)
2121
return subscription
2222
}
2323

24-
func emitDiscontinuity(_ discontinuity: ARTErrorInfo) {
24+
func emitDiscontinuity(_ discontinuity: ARTErrorInfo?) {
2525
for subscription in discontinuitySubscriptions {
2626
subscription.emit(discontinuity)
2727
}

‎Tests/AblyChatTests/Mocks/MockRoomLifecycleContributor.swift

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@ actor MockRoomLifecycleContributor: RoomLifecycleContributor {
55
nonisolated let feature: RoomFeature
66
nonisolated let channel: MockRoomLifecycleContributorChannel
77

8-
private(set) var emitDiscontinuityArguments: [ARTErrorInfo] = []
8+
private(set) var emitDiscontinuityArguments: [ARTErrorInfo?] = []
99

1010
init(feature: RoomFeature, channel: MockRoomLifecycleContributorChannel) {
1111
self.feature = feature
1212
self.channel = channel
1313
}
1414

15-
func emitDiscontinuity(_ error: ARTErrorInfo) async {
15+
func emitDiscontinuity(_ error: ARTErrorInfo?) async {
1616
emitDiscontinuityArguments.append(error)
1717
}
1818
}

0 commit comments

Comments
 (0)