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

other stuff done since first version of this commit:

- start implementing the low-hanging fruit of DETACH
  • Loading branch information
lawrence-forooghian committed Sep 12, 2024
1 parent 9014b0c commit b54200a
Show file tree
Hide file tree
Showing 6 changed files with 758 additions and 2 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
57 changes: 55 additions & 2 deletions Sources/AblyChat/Errors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,19 @@ 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 roomInFailedState = 102_101 // CHA-RL2d
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, .roomInFailedState, .roomIsReleasing, .roomIsReleased:
400
}
}
Expand All @@ -29,16 +37,31 @@ 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 roomInFailedState
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 .roomInFailedState:
.roomInFailedState
case .roomIsReleasing:
.roomIsReleasing
case .roomIsReleased:
.roomIsReleased
}
}

Expand All @@ -47,6 +70,31 @@ 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 .roomInFailedState:
"Cannot perform operation because the room is in a failed state."
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,
.roomInFailedState,
.roomIsReleasing,
.roomIsReleased:
nil
}
}
}
Expand All @@ -58,6 +106,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
175 changes: 175 additions & 0 deletions Sources/AblyChat/RoomLifecycleManager.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
import Ably

// TODO: integrate with the rest of the SDK
internal actor RoomLifecycleManager {
/// The interface that the lifecycle manager expects its contributing realtime channels to conform to.
///
/// We use this instead of the ``RealtimeChannel`` interface as its ``attach`` and ``detach`` methods are `async` instead of using callbacks. This makes it easier to write mocks for (since ``RealtimeChannel`` doesn’t express to the type system that the callbacks it receives need to be `Sendable`, it’s hard to, for example, create a mock that creates a `Task` and then calls the callback from inside this task).
///
/// We choose to also mark the channel’s mutable state as `async`. This is a way of highlighting that since `ARTRealtimeChannel` mutates this state on a separate thread, it’s possible for this state to have changed since the last time you checked it, or since the last time you performed an operation that might have mutated it, or since the last time you recieved an event informing you that it changed.
internal protocol Contributor: Sendable {
func attach() async throws
func detach() async throws

var state: ARTRealtimeChannelState { get async }
var errorReason: ARTErrorInfo? { get async }
}

internal private(set) var current: RoomLifecycle = .initialized
internal private(set) var error: ARTErrorInfo?

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

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

internal init(forTestingWhatHappensWhenCurrentlyIn current: RoomLifecycle, contributors: [Contributor] = [], 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.attach()
} catch {
let contributorState = await 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 = await 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 = await 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 await (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.detach()
break
} catch {
logger.log(message: "Failed to detach non-failed contributor \(contributor), error \(error). Retrying.", level: .info)
// Loop repeats
}
}
}
}

/// Implements CHA-RL2’s DETACH operation.
internal func performDetachOperation() async throws {
switch current {
case .detached:
// CHA-RL2a
return
case .releasing:
// CHA-RL2b
throw ARTErrorInfo(chatError: .roomIsReleasing)
case .released:
// CHA-RL2c
throw ARTErrorInfo(chatError: .roomIsReleased)
case .failed:
// CHA-RL2d
throw ARTErrorInfo(chatError: .roomInFailedState)
case .initialized, .suspended, .attaching, .attached, .detaching:
break
}

// CHA-RL2e
changeStatus(to: .detaching)

// CHA-RL2f
for contributor in contributors {
logger.log(message: "Detaching contributor \(contributor)", level: .info)
try await contributor.detach()
}

// CHA-RL2g
changeStatus(to: .detached)
}
}
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)
}
93 changes: 93 additions & 0 deletions Tests/AblyChatTests/Mocks/MockContributor.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
import Ably
@testable import AblyChat

final actor MockContributor: RoomLifecycleManager.Contributor {
private let attachBehavior: AttachOrDetachBehavior?
private let detachBehavior: AttachOrDetachBehavior?

private(set) var attachCallCount = 0
private(set) var detachCallCount = 0

init(
attachBehavior: AttachOrDetachBehavior? = nil,
detachBehavior: AttachOrDetachBehavior? = nil
) {
self.attachBehavior = attachBehavior
self.detachBehavior = detachBehavior
}

var state: ARTRealtimeChannelState = .initialized
var errorReason: ARTErrorInfo?

enum AttachOrDetachResult {
case success
case failure(ARTErrorInfo)

func performCallback(_ callback: ARTCallback?) {
switch self {
case .success:
callback?(nil)
case let .failure(error):
callback?(error)
}
}
}

enum AttachOrDetachBehavior {
/// Receives an argument indicating how many times (including the current call) the method for which this is providing a mock implementation has been called.
case fromFunction(@Sendable (Int) async -> AttachOrDetachResult)
case complete(AttachOrDetachResult)
case completeAndChangeState(AttachOrDetachResult, newState: ARTRealtimeChannelState)

static var success: Self {
.complete(.success)
}

static func failure(_ error: ARTErrorInfo) -> Self {
.complete(.failure(error))
}
}

func attach() async throws {
attachCallCount += 1

guard let attachBehavior else {
fatalError("attachBehavior must be set before attach is called")
}

try await performBehavior(attachBehavior, callCount: attachCallCount)
}

func detach() async throws {
detachCallCount += 1

guard let detachBehavior else {
fatalError("detachBehavior must be set before detach is called")
}

try await performBehavior(detachBehavior, callCount: detachCallCount)
}

private func performBehavior(_ behavior: AttachOrDetachBehavior, callCount: Int) async throws {
let result: AttachOrDetachResult
switch behavior {
case let .fromFunction(function):
result = await function(callCount)
case let .complete(completeResult):
result = completeResult
case let .completeAndChangeState(completeResult, newState):
state = newState
if case let .failure(error) = completeResult {
errorReason = error
}
result = completeResult
}

switch result {
case .success:
return
case let .failure(error):
throw error
}
}
}
Loading

0 comments on commit b54200a

Please sign in to comment.