Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ECO-5144] Implement CHA-PR3h etc #163

Merged
merged 3 commits into from
Dec 4, 2024
Merged

[ECO-5144] Implement CHA-PR3h etc #163

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c824a65
Fix typo in spec point IDs
lawrence-forooghian Nov 27, 2024
89677f6
Decouple error codes from status codes
lawrence-forooghian Nov 28, 2024
ba4658d
Implement CHA-PR3h and similar
lawrence-forooghian Nov 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions Sources/AblyChat/DefaultMessages.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,10 +234,11 @@ internal final class DefaultMessages: Messages, EmitsDiscontinuities {
case .failed, .suspended:
// TODO: Revisit as part of https://github.com/ably-labs/ably-chat-swift/issues/32
logger.log(message: "Channel failed to attach", level: .error)
let errorCodeCase = ErrorCode.CaseThatImpliesFixedStatusCode.messagesAttachmentFailed
nillableContinuation?.resume(
throwing: ARTErrorInfo.create(
withCode: ErrorCode.messagesAttachmentFailed.rawValue,
status: ErrorCode.messagesAttachmentFailed.statusCode,
withCode: errorCodeCase.toNumericErrorCode.rawValue,
status: errorCodeCase.statusCode,
lawrence-forooghian marked this conversation as resolved.
Show resolved Hide resolved
message: "Channel failed to attach"
)
)
Expand Down
190 changes: 136 additions & 54 deletions Sources/AblyChat/Errors.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,6 @@ public let errorDomain = "AblyChatErrorDomain"
The error codes for errors in the ``errorDomain`` error domain.
*/
public enum ErrorCode: Int {
case nonspecific = 40000

/// ``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:)``.
///
/// TODO this code is a guess, revisit in https://github.com/ably-labs/ably-chat-swift/issues/32
Expand All @@ -36,30 +34,118 @@ public enum ErrorCode: Int {

case roomInInvalidState = 102_107

/// Has a case for each of the ``ErrorCode`` cases that imply a fixed status code.
internal enum CaseThatImpliesFixedStatusCode {
case inconsistentRoomOptions
case messagesAttachmentFailed
case presenceAttachmentFailed
case reactionsAttachmentFailed
case occupancyAttachmentFailed
case typingAttachmentFailed
case messagesDetachmentFailed
case presenceDetachmentFailed
case reactionsDetachmentFailed
case occupancyDetachmentFailed
case typingDetachmentFailed
case roomInFailedState
case roomIsReleasing
case roomIsReleased

internal var toNumericErrorCode: ErrorCode {
switch self {
case .inconsistentRoomOptions:
.inconsistentRoomOptions
case .messagesAttachmentFailed:
.messagesAttachmentFailed
case .presenceAttachmentFailed:
.presenceAttachmentFailed
case .reactionsAttachmentFailed:
.reactionsAttachmentFailed
case .occupancyAttachmentFailed:
.occupancyAttachmentFailed
case .typingAttachmentFailed:
.typingAttachmentFailed
case .messagesDetachmentFailed:
.messagesDetachmentFailed
case .presenceDetachmentFailed:
.presenceDetachmentFailed
case .reactionsDetachmentFailed:
.reactionsDetachmentFailed
case .occupancyDetachmentFailed:
.occupancyDetachmentFailed
case .typingDetachmentFailed:
.typingDetachmentFailed
case .roomInFailedState:
.roomInFailedState
case .roomIsReleasing:
.roomIsReleasing
case .roomIsReleased:
.roomIsReleased
}
}
lawrence-forooghian marked this conversation as resolved.
Show resolved Hide resolved

/// The ``ARTErrorInfo.statusCode`` that should be returned for this error.
internal var statusCode: Int {
// These status codes are taken from the "Chat-specific Error Codes" section of the spec.
switch self {
case .inconsistentRoomOptions,
.roomInFailedState,
.roomIsReleasing,
.roomIsReleased:
400
case
.messagesAttachmentFailed,
.presenceAttachmentFailed,
.reactionsAttachmentFailed,
.occupancyAttachmentFailed,
.typingAttachmentFailed,
.messagesDetachmentFailed,
.presenceDetachmentFailed,
.reactionsDetachmentFailed,
.occupancyDetachmentFailed,
.typingDetachmentFailed:
500
}
}
}

/// Has a case for each of the ``ErrorCode`` cases that do not imply a fixed status code.
internal enum CaseThatImpliesVariableStatusCode {
case roomInInvalidState

internal var toNumericErrorCode: ErrorCode {
switch self {
case .roomInInvalidState:
.roomInInvalidState
}
}
}
}

/**
* Represents a case of ``ErrorCode`` plus a status code.
*/
internal enum ErrorCodeAndStatusCode {
case fixedStatusCode(ErrorCode.CaseThatImpliesFixedStatusCode)
case variableStatusCode(ErrorCode.CaseThatImpliesVariableStatusCode, statusCode: Int)

/// The ``ARTErrorInfo.code`` that should be returned for this error.
internal var code: ErrorCode {
switch self {
case let .fixedStatusCode(code):
code.toNumericErrorCode
case let .variableStatusCode(code, _):
code.toNumericErrorCode
}
}

/// The ``ARTErrorInfo.statusCode`` that should be returned for this error.
internal var statusCode: Int {
// These status codes are taken from the "Chat-specific Error Codes" section of the spec.
switch self {
case .nonspecific,
.inconsistentRoomOptions,
.roomInFailedState,
.roomIsReleasing,
.roomIsReleased:
400
case
.messagesAttachmentFailed,
.presenceAttachmentFailed,
.reactionsAttachmentFailed,
.occupancyAttachmentFailed,
.typingAttachmentFailed,
.messagesDetachmentFailed,
.presenceDetachmentFailed,
.reactionsDetachmentFailed,
.occupancyDetachmentFailed,
.typingDetachmentFailed,
// CHA-RL9c
.roomInInvalidState:
500
case let .fixedStatusCode(code):
code.statusCode
case let .variableStatusCode(_, statusCode):
statusCode
}
}
}
Expand All @@ -77,51 +163,50 @@ internal enum ChatError {
case roomIsReleasing
case roomIsReleased
case presenceOperationRequiresRoomAttach(feature: RoomFeature)
case presenceOperationDisallowedForCurrentRoomStatus(feature: RoomFeature)
case roomInInvalidState(cause: ARTErrorInfo?)
case roomTransitionedToInvalidStateForPresenceOperation(cause: ARTErrorInfo?)

/// The ``ARTErrorInfo.code`` that should be returned for this error.
internal var code: ErrorCode {
internal var codeAndStatusCode: ErrorCodeAndStatusCode {
switch self {
case .inconsistentRoomOptions:
.inconsistentRoomOptions
.fixedStatusCode(.inconsistentRoomOptions)
case let .attachmentFailed(feature, _):
switch feature {
case .messages:
.messagesAttachmentFailed
.fixedStatusCode(.messagesAttachmentFailed)
case .occupancy:
.occupancyAttachmentFailed
.fixedStatusCode(.occupancyAttachmentFailed)
case .presence:
.presenceAttachmentFailed
.fixedStatusCode(.presenceAttachmentFailed)
case .reactions:
.reactionsAttachmentFailed
.fixedStatusCode(.reactionsAttachmentFailed)
case .typing:
.typingAttachmentFailed
.fixedStatusCode(.typingAttachmentFailed)
}
case let .detachmentFailed(feature, _):
switch feature {
case .messages:
.messagesDetachmentFailed
.fixedStatusCode(.messagesDetachmentFailed)
case .occupancy:
.occupancyDetachmentFailed
.fixedStatusCode(.occupancyDetachmentFailed)
case .presence:
.presenceDetachmentFailed
.fixedStatusCode(.presenceDetachmentFailed)
case .reactions:
.reactionsDetachmentFailed
.fixedStatusCode(.reactionsDetachmentFailed)
case .typing:
.typingDetachmentFailed
.fixedStatusCode(.typingDetachmentFailed)
}
case .roomInFailedState:
.roomInFailedState
.fixedStatusCode(.roomInFailedState)
case .roomIsReleasing:
.roomIsReleasing
.fixedStatusCode(.roomIsReleasing)
case .roomIsReleased:
.roomIsReleased
case .roomInInvalidState:
.roomInInvalidState
case .presenceOperationRequiresRoomAttach,
.presenceOperationDisallowedForCurrentRoomStatus:
.nonspecific
.fixedStatusCode(.roomIsReleased)
case .roomTransitionedToInvalidStateForPresenceOperation:
// CHA-RL9c
.variableStatusCode(.roomInInvalidState, statusCode: 500)
case .presenceOperationRequiresRoomAttach:
// CHA-PR3h, CHA-PR10h, CHA-PR6h, CHA-T2g
.variableStatusCode(.roomInInvalidState, statusCode: 400)
}
}

Expand Down Expand Up @@ -177,9 +262,7 @@ internal enum ChatError {
"Cannot perform operation because the room is in a released state."
case let .presenceOperationRequiresRoomAttach(feature):
"To perform this \(Self.descriptionOfFeature(feature)) operation, you must first attach the room."
case let .presenceOperationDisallowedForCurrentRoomStatus(feature):
"This \(Self.descriptionOfFeature(feature)) operation can not be performed given the current room status."
case .roomInInvalidState:
case .roomTransitionedToInvalidStateForPresenceOperation:
"The room operation failed because the room was in an invalid state."
}
}
Expand All @@ -191,14 +274,13 @@ internal enum ChatError {
underlyingError
case let .detachmentFailed(_, underlyingError):
underlyingError
case let .roomInInvalidState(cause):
case let .roomTransitionedToInvalidStateForPresenceOperation(cause):
cause
case .inconsistentRoomOptions,
.roomInFailedState,
.roomIsReleasing,
.roomIsReleased,
.presenceOperationRequiresRoomAttach,
.presenceOperationDisallowedForCurrentRoomStatus:
.presenceOperationRequiresRoomAttach:
nil
}
}
Expand All @@ -208,7 +290,7 @@ internal extension ARTErrorInfo {
convenience init(chatError: ChatError) {
var userInfo: [String: Any] = [:]
// TODO: copied and pasted from implementation of -[ARTErrorInfo createWithCode:status:message:requestId:] because there’s no way to pass domain; revisit in https://github.com/ably-labs/ably-chat-swift/issues/32. Also the ARTErrorInfoStatusCode variable in ably-cocoa is not public.
userInfo["ARTErrorInfoStatusCode"] = chatError.code.statusCode
userInfo["ARTErrorInfoStatusCode"] = chatError.codeAndStatusCode.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.
Expand All @@ -218,7 +300,7 @@ internal extension ARTErrorInfo {

self.init(
domain: errorDomain,
code: chatError.code.rawValue,
code: chatError.codeAndStatusCode.code.rawValue,
userInfo: userInfo
)
}
Expand Down
9 changes: 4 additions & 5 deletions Sources/AblyChat/RoomFeature.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,11 @@ internal protocol FeatureChannel: Sendable, EmitsDiscontinuities {

/// Waits until we can perform presence operations on the contributors of this room without triggering an implicit attach.
///
/// Implements the checks described by CHA-PR3d, CHA-PR3e, CHA-PR3f, and CHA-PR3g (and similar ones described by other functionality that performs contributor presence operations). Namely:
/// Implements the checks described by CHA-PR3d, CHA-PR3e, and CHA-PR3h (and similar ones described by other functionality that performs contributor presence operations). Namely:
///
/// - CHA-RL9, which is invoked by CHA-PR3d, CHA-PR10d, CHA-PR6c, CHA-T2c: If the room is in the ATTACHING status, it waits for the next room status change. If the new status is ATTACHED, it returns. Else, it throws an `ARTErrorInfo` derived from ``ChatError.roomInInvalidState(cause:)``.
/// - CHA-PR3e, CHA-PR11e, CHA-PR6d, CHA-T2d: If the room is in the ATTACHED status, it returns immediately.
/// - CHA-PR3f, CHA-PR11f, CHA-PR6e, CHA-T2e: If the room is in the DETACHED status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationRequiresRoomAttach(feature:)``.
/// - // CHA-PR3g, CHA-PR11g, CHA-PR6f, CHA-T2f: If the room is in any other status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationDisallowedForCurrentRoomStatus(feature:)``.
/// - CHA-RL9, which is invoked by CHA-PR3d, CHA-PR10d, CHA-PR6c, CHA-T2c: If the room is in the ATTACHING status, it waits for the next room status change. If the new status is ATTACHED, it returns. Else, it throws an `ARTErrorInfo` derived from ``ChatError.roomTransitionedToInvalidStateForPresenceOperation(cause:)``.
/// - CHA-PR3e, CHA-PR10e, CHA-PR6d, CHA-T2d: If the room is in the ATTACHED status, it returns immediately.
/// - CHA-PR3h, CHA-PR10h, CHA-PR6h, CHA-T2g: If the room is in any other status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationRequiresRoomAttach(feature:)``.
///
/// - Parameters:
/// - requester: The room feature that wishes to perform a presence operation. This is only used for customising the message of the thrown error.
Expand Down
11 changes: 4 additions & 7 deletions Sources/AblyChat/RoomLifecycleManager.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1225,17 +1225,14 @@ internal actor DefaultRoomLifecycleManager<Contributor: RoomLifecycleContributor
// TODO: decide what to do if nextRoomStatusChange is nil; I believe that this will happen if the current Task is cancelled. For now, will just treat it as an invalid status change. Handle it properly in https://github.com/ably-labs/ably-chat-swift/issues/29
if nextRoomStatusChange?.current != .attached {
// CHA-RL9c
throw .init(chatError: .roomInInvalidState(cause: nextRoomStatusChange?.current.error))
throw .init(chatError: .roomTransitionedToInvalidStateForPresenceOperation(cause: nextRoomStatusChange?.current.error))
}
case .attached:
// CHA-PR3e, CHA-PR11e, CHA-PR6d, CHA-T2d
// CHA-PR3e, CHA-PR10e, CHA-PR6d, CHA-T2d
break
case .detached:
// CHA-PR3f, CHA-PR11f, CHA-PR6e, CHA-T2e
throw .init(chatError: .presenceOperationRequiresRoomAttach(feature: requester))
default:
// CHA-PR3g, CHA-PR11g, CHA-PR6f, CHA-T2f
throw .init(chatError: .presenceOperationDisallowedForCurrentRoomStatus(feature: requester))
// CHA-PR3h, CHA-PR10h, CHA-PR6h, CHA-T2g
throw .init(chatError: .presenceOperationRequiresRoomAttach(feature: requester))
}
}

Expand Down
Loading
Loading