forked from ably/ably-chat-swift
-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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-4952] Start implementing room lifecycle spec #7
Closed
ruancomelli
wants to merge
1
commit into
base-sha/90b9ac142351df378dc3a31696e5d0bcaf26cb66
from
head-sha/c70ee445fe5f793e076147c64c592f77ae0a193a/2024-09-26T09-36-56/533446
+1,250
−5
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,37 +16,163 @@ 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 | ||
|
||
case messagesAttachmentFailed = 102_001 | ||
case presenceAttachmentFailed = 102_002 | ||
case reactionsAttachmentFailed = 102_003 | ||
case occupancyAttachmentFailed = 102_004 | ||
case typingAttachmentFailed = 102_005 | ||
|
||
case messagesDetachmentFailed = 102_050 | ||
case presenceDetachmentFailed = 102_051 | ||
case reactionsDetachmentFailed = 102_052 | ||
case occupancyDetachmentFailed = 102_053 | ||
case typingDetachmentFailed = 102_054 | ||
|
||
case roomInFailedState = 102_101 | ||
case roomIsReleasing = 102_102 | ||
case roomIsReleased = 102_103 | ||
|
||
/// 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 | ||
// TODO: These are currently a guess, revisit once outstanding spec question re status codes is answered (https://github.com/ably/specification/pull/200#discussion_r1755222945), and also revisit in https://github.com/ably-labs/ably-chat-swift/issues/32 | ||
switch self { | ||
case .inconsistentRoomOptions: | ||
case .inconsistentRoomOptions, | ||
.messagesDetachmentFailed, | ||
.presenceDetachmentFailed, | ||
.reactionsDetachmentFailed, | ||
.occupancyDetachmentFailed, | ||
.typingDetachmentFailed, | ||
.roomInFailedState, | ||
.roomIsReleasing, | ||
.roomIsReleased: | ||
400 | ||
case .messagesAttachmentFailed, | ||
.presenceAttachmentFailed, | ||
.reactionsAttachmentFailed, | ||
.occupancyAttachmentFailed, | ||
.typingAttachmentFailed: | ||
// TODO: This is currently a best guess based on the limited status code information given in the spec at time of writing (i.e. CHA-RL1h4); it's not clear to me whether these error codes are always meant to have the same status code. Revisit once aforementioned spec question re status codes answered. | ||
500 | ||
} | ||
} | ||
} | ||
|
||
/** | ||
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 attachmentFailed(feature: RoomFeature, underlyingError: ARTErrorInfo) | ||
case detachmentFailed(feature: RoomFeature, 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 let .attachmentFailed(feature, _): | ||
switch feature { | ||
case .messages: | ||
.messagesAttachmentFailed | ||
case .occupancy: | ||
.occupancyAttachmentFailed | ||
case .presence: | ||
.presenceAttachmentFailed | ||
case .reactions: | ||
.reactionsAttachmentFailed | ||
case .typing: | ||
.typingAttachmentFailed | ||
} | ||
case let .detachmentFailed(feature, _): | ||
switch feature { | ||
case .messages: | ||
.messagesDetachmentFailed | ||
case .occupancy: | ||
.occupancyDetachmentFailed | ||
case .presence: | ||
.presenceDetachmentFailed | ||
case .reactions: | ||
.reactionsDetachmentFailed | ||
case .typing: | ||
.typingDetachmentFailed | ||
} | ||
case .roomInFailedState: | ||
.roomInFailedState | ||
case .roomIsReleasing: | ||
.roomIsReleasing | ||
case .roomIsReleased: | ||
.roomIsReleased | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Extract repetitive code in localizedDescription into a separate method The switch statements for attachment and detachment failures contain similar code. Consider extracting this logic into a separate method to reduce duplication and improve maintainability. |
||
|
||
/// A helper type for parameterising the construction of error messages. | ||
private enum AttachOrDetach { | ||
case attach | ||
case detach | ||
} | ||
|
||
private static func localizedDescription( | ||
forFailureOfOperation operation: AttachOrDetach, | ||
feature: RoomFeature | ||
) -> String { | ||
let featureDescription = switch feature { | ||
case .messages: | ||
"messages" | ||
case .occupancy: | ||
"occupancy" | ||
case .presence: | ||
"presence" | ||
case .reactions: | ||
"reactions" | ||
case .typing: | ||
"typing" | ||
} | ||
|
||
let operationDescription = switch operation { | ||
case .attach: | ||
"attach" | ||
case .detach: | ||
"detach" | ||
} | ||
|
||
return "The \(featureDescription) feature failed to \(operationDescription)." | ||
} | ||
|
||
/// The ``ARTErrorInfo.localizedDescription`` that should be returned for this error. | ||
internal var localizedDescription: String { | ||
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 let .attachmentFailed(feature, _): | ||
Self.localizedDescription(forFailureOfOperation: .attach, feature: feature) | ||
case let .detachmentFailed(feature, _): | ||
Self.localizedDescription(forFailureOfOperation: .detach, feature: feature) | ||
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 .attachmentFailed(_, underlyingError): | ||
underlyingError | ||
case let .detachmentFailed(_, underlyingError): | ||
underlyingError | ||
case .inconsistentRoomOptions, | ||
.roomInFailedState, | ||
.roomIsReleasing, | ||
.roomIsReleased: | ||
nil | ||
} | ||
} | ||
} | ||
|
@@ -58,6 +184,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, | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
/// The features offered by a chat room. | ||
internal enum RoomFeature { | ||
case messages | ||
case presence | ||
case reactions | ||
case occupancy | ||
case typing | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Simplify the switch statement in the statusCode property
Consider grouping the cases that return the same value to reduce repetition and improve readability. For example, you could use a single case with a pattern that matches all relevant error codes.