Skip to content

Commit

Permalink
[WIP] Implement room lifecycle spec
Browse files Browse the repository at this point in the history
This is based on [1] at aa7455d. It’s generated some questions, which
I’ve asked on that PR.

I started implementing this as part of #19, before realising that
implementing the spec is not the aim of that task. So, putting this work
on hold until we pick it up again in #28.

So far, only the ATTACH operation is implemented.

[1] ably/specification#200

I have since updated with error names @ bcb7390
  • Loading branch information
lawrence-forooghian committed Sep 11, 2024
1 parent 9014b0c commit eb30743
Show file tree
Hide file tree
Showing 7 changed files with 571 additions and 36 deletions.
1 change: 1 addition & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ To check formatting and code quality, run `swift run BuildTool lint`. Run with `
- We should aim to make it easy for consumers of the SDK to be able to mock out the SDK in the tests for their own code. A couple of things that will aid with this:
- Describe the SDK’s functionality via protocols (when doing so would still be sufficiently idiomatic to Swift).
- When defining a `struct` that is emitted by the public API of the library, make sure to define a public memberwise initializer so that users can create one to be emitted by their mocks. (There is no way to make Swift’s autogenerated memberwise initializer public, so you will need to write one yourself. In Xcode, you can do this by clicking at the start of the type declaration and doing Editor → Refactor → Generate Memberwise Initializer.)
- TODO something about adding spec points in code and tests

## Building for Swift 6

Expand Down
50 changes: 48 additions & 2 deletions Sources/AblyChat/Errors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,18 @@ public enum ErrorCode: Int {
/// TODO this code is a guess, revisit in https://github.com/ably-labs/ably-chat-swift/issues/32
case inconsistentRoomOptions = 1

// TODO: describe, and code is a guess
case channelAttachResultedInSuspended = 2
case channelAttachResultedInFailed = 3

case roomIsReleasing = 102_102 // CHA-RL1b, CHA-RL2b
case roomIsReleased = 102_103 // CHA-RL1c, CHA-RL2c

/// The ``ARTErrorInfo.statusCode`` that should be returned for this error.
internal var statusCode: Int {
// TODO: These are currently a guess, revisit in https://github.com/ably-labs/ably-chat-swift/issues/32
switch self {
case .inconsistentRoomOptions:
case .inconsistentRoomOptions, .channelAttachResultedInSuspended, .channelAttachResultedInFailed, .roomIsReleasing, .roomIsReleased:
400
}
}
Expand All @@ -29,16 +36,28 @@ public enum ErrorCode: Int {
/**
The errors thrown by the Chat SDK.

This type exists in addition to ``ErrorCode`` to allow us to attach metadata which can be incorporated into the error’s `localizedDescription`.
This type exists in addition to ``ErrorCode`` to allow us to attach metadata which can be incorporated into the error’s `localizedDescription` and `cause`.
*/
internal enum ChatError {
case inconsistentRoomOptions(requested: RoomOptions, existing: RoomOptions)
case channelAttachResultedInSuspended(underlyingError: ARTErrorInfo)
case channelAttachResultedInFailed(underlyingError: ARTErrorInfo)
case roomIsReleasing
case roomIsReleased

/// The ``ARTErrorInfo.code`` that should be returned for this error.
internal var code: ErrorCode {
switch self {
case .inconsistentRoomOptions:
.inconsistentRoomOptions
case .channelAttachResultedInSuspended:
.channelAttachResultedInSuspended
case .channelAttachResultedInFailed:
.channelAttachResultedInFailed
case .roomIsReleasing:
.roomIsReleasing
case .roomIsReleased:
.roomIsReleased
}
}

Expand All @@ -47,6 +66,28 @@ internal enum ChatError {
switch self {
case let .inconsistentRoomOptions(requested, existing):
"Rooms.get(roomID:options:) was called with a different set of room options than was used on a previous call. You must first release the existing room instance using Rooms.release(roomID:). Requested options: \(requested), existing options: \(existing)"
case .channelAttachResultedInSuspended:
"TODO"
case .channelAttachResultedInFailed:
"TODO"
case .roomIsReleasing:
"Cannot perform operation because the room is in a releasing state."
case .roomIsReleased:
"Cannot perform operation because the room is in a released state."
}
}

/// The ``ARTErrorInfo.cause`` that should be returned for this error.
internal var cause: ARTErrorInfo? {
switch self {
case let .channelAttachResultedInSuspended(underlyingError):
underlyingError
case let .channelAttachResultedInFailed(underlyingError):
underlyingError
case .inconsistentRoomOptions,
.roomIsReleasing,
.roomIsReleased:
nil
}
}
}
Expand All @@ -58,6 +99,11 @@ internal extension ARTErrorInfo {
userInfo["ARTErrorInfoStatusCode"] = chatError.code.statusCode
userInfo[NSLocalizedDescriptionKey] = chatError.localizedDescription

// TODO: This is kind of an implementation detail (that NSUnderlyingErrorKey is what populates `cause`); consider documenting in ably-cocoa as part of https://github.com/ably-labs/ably-chat-swift/issues/32.
if let cause = chatError.cause {
userInfo[NSUnderlyingErrorKey] = cause
}

self.init(
domain: errorDomain,
code: chatError.code.rawValue,
Expand Down
130 changes: 130 additions & 0 deletions Sources/AblyChat/RoomLifecycleManager.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,130 @@
import Ably

// TODO: integrate with the rest of the SDK
internal actor RoomLifecycleManager {
internal private(set) var current: RoomLifecycle = .initialized
internal private(set) var error: ARTErrorInfo?

private let logger: InternalLogger
private let contributors: [RealtimeChannelProtocol]

internal init(contributors: [RealtimeChannelProtocol] = [], logger: InternalLogger) {
self.contributors = contributors
self.logger = logger
}

internal init(forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle, contributors: [RealtimeChannelProtocol] = [], logger: InternalLogger) {
self.current = current
self.contributors = contributors
self.logger = logger
}

// TODO: clean up old subscriptions (https://github.com/ably-labs/ably-chat-swift/issues/36)
private var subscriptions: [Subscription<RoomStatusChange>] = []

internal func onChange(bufferingPolicy: BufferingPolicy) -> Subscription<RoomStatusChange> {
let subscription: Subscription<RoomStatusChange> = .init(bufferingPolicy: bufferingPolicy)
subscriptions.append(subscription)
return subscription
}

/// 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)
let previous = current
current = new
self.error = error
let statusChange = RoomStatusChange(current: current, previous: previous, error: error)
emitStatusChange(statusChange)
}

private func emitStatusChange(_ change: RoomStatusChange) {
for subscription in subscriptions {
subscription.emit(change)
}
}

/// Implements CHA-RL1’s `ATTACH` operation.
internal func performAttachOperation() async throws {
switch current {
case .attached:
// CHA-RL1a
return
case .releasing:
// CHA-RL1b
throw ARTErrorInfo(chatError: .roomIsReleasing)
case .released:
// CHA-RL1c
throw ARTErrorInfo(chatError: .roomIsReleased)
case .initialized, .suspended, .attaching, .detached, .detaching, .failed:
break
}

// CHA-RL1e
changeStatus(to: .attaching)

// CHA-RL1f
for contributor in contributors {
do {
logger.log(message: "Attaching contributor \(contributor)", level: .info)
try await contributor.attachAsync()
} catch {
let contributorState = contributor.state
logger.log(message: "Failed to attach contributor \(contributor), which is now in state \(contributorState), error \(error)", level: .info)

switch contributorState {
case .suspended:
// CHA-RL1h2
guard let contributorError = contributor.errorReason else {
// TODO: something about this
preconditionFailure("Contributor entered SUSPENDED but its errorReason is not set")
}

let error = ARTErrorInfo(chatError: .channelAttachResultedInSuspended(underlyingError: contributorError))
changeStatus(to: .suspended, error: error)

// CHA-RL1h3
throw contributorError
case .failed:
// CHA-RL1h4
guard let contributorError = contributor.errorReason else {
// TODO: something about this
preconditionFailure("Contributor entered FAILED but its errorReason is not set")
}

let error = ARTErrorInfo(chatError: .channelAttachResultedInFailed(underlyingError: contributorError))
changeStatus(to: .failed, error: error)

// CHA-RL1h5
await detachNonFailedContributors()

// CHA-RL1h1
throw contributorError
default:
// TODO: something about this; quite possible due to thread timing stuff
preconditionFailure("Attach failure left contributor in unexpected state \(contributorState)")
}
}
}

// CHA-RL1g
changeStatus(to: .attached)
}

/// Implements CHA-RL1h5’s "detach all channels that are not in the FAILED state".
private func detachNonFailedContributors() async {
for contributor in contributors where contributor.state != .failed {
// CHA-RL1h6: Retry until detach succeeds
while true {
do {
logger.log(message: "Detaching non-failed contributor \(contributor)", level: .info)
try await contributor.detachAsync()
break
} catch {
logger.log(message: "Failed to detach non-failed contributor \(contributor), error \(error). Retrying.", level: .info)
// Loop repeats
}
}
}
}
}
24 changes: 12 additions & 12 deletions Tests/AblyChatTests/DefaultRoomTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ class DefaultRoomTests: XCTestCase {
// - basketball::$chat::$typingIndicators
// - basketball::$chat::$reactions
let channelsList = [
MockRealtimeChannel(name: "basketball::$chat::$chatMessages", attachResult: .success),
MockRealtimeChannel(name: "basketball::$chat::$typingIndicators", attachResult: .success),
MockRealtimeChannel(name: "basketball::$chat::$reactions", attachResult: .success),
MockRealtimeChannel(name: "basketball::$chat::$chatMessages", attachBehavior: .success),
MockRealtimeChannel(name: "basketball::$chat::$typingIndicators", attachBehavior: .success),
MockRealtimeChannel(name: "basketball::$chat::$reactions", attachBehavior: .success),
]
let channels = MockChannels(channels: channelsList)
let realtime = MockRealtime.create(channels: channels)
Expand Down Expand Up @@ -47,9 +47,9 @@ class DefaultRoomTests: XCTestCase {
// and fails when called on channel basketball::$chat::$reactions
let channelAttachError = ARTErrorInfo.createUnknownError() // arbitrary
let channelsList = [
MockRealtimeChannel(name: "basketball::$chat::$chatMessages", attachResult: .success),
MockRealtimeChannel(name: "basketball::$chat::$typingIndicators", attachResult: .success),
MockRealtimeChannel(name: "basketball::$chat::$reactions", attachResult: .failure(channelAttachError)),
MockRealtimeChannel(name: "basketball::$chat::$chatMessages", attachBehavior: .success),
MockRealtimeChannel(name: "basketball::$chat::$typingIndicators", attachBehavior: .success),
MockRealtimeChannel(name: "basketball::$chat::$reactions", attachBehavior: .failure(channelAttachError)),
]
let channels = MockChannels(channels: channelsList)
let realtime = MockRealtime.create(channels: channels)
Expand All @@ -76,9 +76,9 @@ class DefaultRoomTests: XCTestCase {
// - basketball::$chat::$typingIndicators
// - basketball::$chat::$reactions
let channelsList = [
MockRealtimeChannel(name: "basketball::$chat::$chatMessages", detachResult: .success),
MockRealtimeChannel(name: "basketball::$chat::$typingIndicators", detachResult: .success),
MockRealtimeChannel(name: "basketball::$chat::$reactions", detachResult: .success),
MockRealtimeChannel(name: "basketball::$chat::$chatMessages", detachBehavior: .success),
MockRealtimeChannel(name: "basketball::$chat::$typingIndicators", detachBehavior: .success),
MockRealtimeChannel(name: "basketball::$chat::$reactions", detachBehavior: .success),
]
let channels = MockChannels(channels: channelsList)
let realtime = MockRealtime.create(channels: channels)
Expand Down Expand Up @@ -113,9 +113,9 @@ class DefaultRoomTests: XCTestCase {
// and fails when called on channel basketball::$chat::$reactions
let channelDetachError = ARTErrorInfo.createUnknownError() // arbitrary
let channelsList = [
MockRealtimeChannel(name: "basketball::$chat::$chatMessages", detachResult: .success),
MockRealtimeChannel(name: "basketball::$chat::$typingIndicators", detachResult: .success),
MockRealtimeChannel(name: "basketball::$chat::$reactions", detachResult: .failure(channelDetachError)),
MockRealtimeChannel(name: "basketball::$chat::$chatMessages", detachBehavior: .success),
MockRealtimeChannel(name: "basketball::$chat::$typingIndicators", detachBehavior: .success),
MockRealtimeChannel(name: "basketball::$chat::$reactions", detachBehavior: .failure(channelDetachError)),
]
let channels = MockChannels(channels: channelsList)
let realtime = MockRealtime.create(channels: channels)
Expand Down
18 changes: 18 additions & 0 deletions Tests/AblyChatTests/Helpers/Helpers.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,3 +13,21 @@ func assertIsChatError(_ maybeError: (any Error)?, withCode code: AblyChat.Error
XCTAssertEqual(ablyError.code, code.rawValue, file: file, line: line)
XCTAssertEqual(ablyError.statusCode, code.statusCode, file: file, line: line)
}

/**
Asserts that a given async expression throws an `ARTErrorInfo` in the chat error domain with a given code.

Doesn't take an autoclosure because for whatever reason one of our linting tools removes the `await` on the expression.
*/
func assertThrowsARTErrorInfo(withCode code: AblyChat.ErrorCode, _ expression: () async throws -> some Any, file: StaticString = #filePath, line: UInt = #line) async throws {
let caughtError: Error?

do {
_ = try await expression()
caughtError = nil
} catch {
caughtError = error
}

try assertIsChatError(caughtError, withCode: code, file: file, line: line)
}
Loading

0 comments on commit eb30743

Please sign in to comment.