Skip to content

Commit

Permalink
Decouple error codes from status codes
Browse files Browse the repository at this point in the history 
This is in preparation for implementing CHA-PR3h, in which a given
numerical error code no longer necessarily implies a specific status
code (specifically, RoomInInvalidState may now have a 400 or 500 status
code depending on the context in which it is thrown).

The internal API introduced here is a bit convoluted and verbose, but
I’ve done it this way for two reasons:

1. It reflects the way the spec is written; most of the time a numeric
   code has a status code written alongside it

2. For the numerical error codes that _do_ always imply a certain status
   code, I want to be able to get that status code still, so that the
   existing Messages code that throws an error with the
   messagesAttachmentFailed code, as well as the isChatError(…) test
   helper, continue to work as they currently do.
  • Loading branch information
@lawrence-forooghian
lawrence-forooghian committed Dec 4, 2024
1 parent c824a65 commit 89677f6
Show file tree
Hide file tree
Showing 5 changed files with 151 additions and 59 deletions.
5 changes: 3 additions & 2 deletions Sources/AblyChat/DefaultMessages.swift
View file
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,
message: "Channel failed to attach"
)
)
Expand Down
173 changes: 132 additions & 41 deletions Sources/AblyChat/Errors.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,30 +36,122 @@ 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 nonspecific
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
case roomInInvalidState

internal var toNumericErrorCode: ErrorCode {
switch self {
case .nonspecific:
.nonspecific
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
case .roomInInvalidState:
.roomInInvalidState
}
}

/// 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
}
}
}

/// Has a case for each of the ``ErrorCode`` cases that do not imply a fixed status code.
internal enum CaseThatImpliesVariableStatusCode {
internal var toNumericErrorCode: ErrorCode {
switch self {}
}
}
}

/**
* 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 @@ -80,48 +172,47 @@ internal enum ChatError {
case presenceOperationDisallowedForCurrentRoomStatus(feature: RoomFeature)
case roomInInvalidState(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
.fixedStatusCode(.roomIsReleased)
case .roomInInvalidState:
.roomInInvalidState
.fixedStatusCode(.roomInInvalidState)
case .presenceOperationRequiresRoomAttach,
.presenceOperationDisallowedForCurrentRoomStatus:
.nonspecific
.fixedStatusCode(.nonspecific)
}
}

Expand Down Expand Up @@ -208,7 +299,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 +309,7 @@ internal extension ARTErrorInfo {

self.init(
domain: errorDomain,
code: chatError.code.rawValue,
code: chatError.codeAndStatusCode.code.rawValue,
userInfo: userInfo
)
}
Expand Down
24 changes: 12 additions & 12 deletions Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ struct DefaultRoomLifecycleManagerTests {
await #expect {
try await manager.performAttachOperation()
} throws: { error in
isChatError(error, withCode: .roomIsReleasing)
isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.roomIsReleasing))
}
}

Expand All @@ -160,7 +160,7 @@ struct DefaultRoomLifecycleManagerTests {
await #expect {
try await manager.performAttachOperation()
} throws: { error in
isChatError(error, withCode: .roomIsReleased)
isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.roomIsReleased))
}
}

Expand Down Expand Up @@ -373,7 +373,7 @@ struct DefaultRoomLifecycleManagerTests {
}

for error in await [suspendedRoomStatusChange.error, manager.roomStatus.error, roomAttachError] {
#expect(isChatError(error, withCode: .messagesAttachmentFailed, cause: contributorAttachError))
#expect(isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.messagesAttachmentFailed), cause: contributorAttachError))
}

// and:
Expand Down Expand Up @@ -454,7 +454,7 @@ struct DefaultRoomLifecycleManagerTests {
}

for error in await [failedStatusChange.error, manager.roomStatus.error, roomAttachError] {
#expect(isChatError(error, withCode: .messagesAttachmentFailed, cause: contributorAttachError))
#expect(isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.messagesAttachmentFailed), cause: contributorAttachError))
}
}

Expand Down Expand Up @@ -561,7 +561,7 @@ struct DefaultRoomLifecycleManagerTests {
await #expect {
try await manager.performDetachOperation()
} throws: { error in
isChatError(error, withCode: .roomIsReleasing)
isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.roomIsReleasing))
}
}

Expand All @@ -576,7 +576,7 @@ struct DefaultRoomLifecycleManagerTests {
await #expect {
try await manager.performDetachOperation()
} throws: { error in
isChatError(error, withCode: .roomIsReleased)
isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.roomIsReleased))
}
}

Expand All @@ -595,7 +595,7 @@ struct DefaultRoomLifecycleManagerTests {
await #expect {
try await manager.performDetachOperation()
} throws: { error in
isChatError(error, withCode: .roomInFailedState)
isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.roomInFailedState))
}
}

Expand Down Expand Up @@ -694,7 +694,7 @@ struct DefaultRoomLifecycleManagerTests {
let failedStatusChange = try #require(await maybeFailedStatusChange)

for maybeError in [maybeRoomDetachError, failedStatusChange.error] {
#expect(isChatError(maybeError, withCode: .presenceDetachmentFailed, cause: contributor1DetachError))
#expect(isChatError(maybeError, withCodeAndStatusCode: .fixedStatusCode(.presenceDetachmentFailed), cause: contributor1DetachError))
}
}

Expand Down Expand Up @@ -1095,7 +1095,7 @@ struct DefaultRoomLifecycleManagerTests {

#expect(roomStatus.isFailed)
for error in [roomStatus.error, failedStatusChange.error] {
#expect(isChatError(error, withCode: .presenceDetachmentFailed, cause: contributor1DetachError))
#expect(isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.presenceDetachmentFailed), cause: contributor1DetachError))
}

for contributor in contributors {
Expand Down Expand Up @@ -2096,7 +2096,7 @@ struct DefaultRoomLifecycleManagerTests {
}

let expectedCause = ARTErrorInfo(chatError: .attachmentFailed(feature: .messages, underlyingError: contributorAttachError)) // using our knowledge of CHA-RL1h4
#expect(isChatError(caughtError, withCode: .roomInInvalidState, cause: expectedCause))
#expect(isChatError(caughtError, withCodeAndStatusCode: .fixedStatusCode(.roomInInvalidState), cause: expectedCause))
}

// @specPartial CHA-PR3e - Tests the wait described in the spec point, but not that the feature actually performs this wait nor the side effect. TODO change this to a specOneOf once the feature is implemented
Expand Down Expand Up @@ -2137,7 +2137,7 @@ struct DefaultRoomLifecycleManagerTests {
}

// Then: It throws a presenceOperationRequiresRoomAttach error for that feature (which we just check via its error code, i.e. `.nonspecific`, and its message)
#expect(isChatError(caughtError, withCode: .nonspecific, message: "To perform this messages operation, you must first attach the room."))
#expect(isChatError(caughtError, withCodeAndStatusCode: .fixedStatusCode(.nonspecific), message: "To perform this messages operation, you must first attach the room."))
}

// @specPartial CHA-PR3f - Tests the wait described in the spec point, but not that the feature actually performs this wait. TODO change this to a specOneOf once the feature is implemented
Expand All @@ -2162,6 +2162,6 @@ struct DefaultRoomLifecycleManagerTests {
}

// Then: It throws a presenceOperationDisallowedForCurrentRoomStatus error for that feature (which we just check via its error code, i.e. `.nonspecific`, and its message)
#expect(isChatError(caughtError, withCode: .nonspecific, message: "This messages operation can not be performed given the current room status."))
#expect(isChatError(caughtError, withCodeAndStatusCode: .fixedStatusCode(.nonspecific), message: "This messages operation can not be performed given the current room status."))
}
}
2 changes: 1 addition & 1 deletion Tests/AblyChatTests/DefaultRoomsTests.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ struct DefaultRoomsTests {
}

// Then: It throws an inconsistentRoomOptions error
#expect(isChatError(caughtError, withCode: .inconsistentRoomOptions))
#expect(isChatError(caughtError, withCodeAndStatusCode: .fixedStatusCode(.inconsistentRoomOptions)))
}

// MARK: - Release a room
Expand Down
6 changes: 3 additions & 3 deletions Tests/AblyChatTests/Helpers/Helpers.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,14 @@ import Ably
/**
Tests whether a given optional `Error` is an `ARTErrorInfo` in the chat error domain with a given code and cause. Can optionally pass a message and it will check that it matches.
*/
func isChatError(_ maybeError: (any Error)?, withCode code: AblyChat.ErrorCode, cause: ARTErrorInfo? = nil, message: String? = nil) -> Bool {
func isChatError(_ maybeError: (any Error)?, withCodeAndStatusCode codeAndStatusCode: AblyChat.ErrorCodeAndStatusCode, cause: ARTErrorInfo? = nil, message: String? = nil) -> Bool {
guard let ablyError = maybeError as? ARTErrorInfo else {
return false
}

return ablyError.domain == AblyChat.errorDomain as String
&& ablyError.code == code.rawValue
&& ablyError.statusCode == code.statusCode
&& ablyError.code == codeAndStatusCode.code.rawValue
&& ablyError.statusCode == codeAndStatusCode.statusCode
&& ablyError.cause == cause
&& {
guard let message else {
Expand Down

0 comments on commit 89677f6

Please sign in to comment.