From c824a650e47ec13a237a0276429b5a892eabaf26 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Wed, 27 Nov 2024 17:11:58 -0300 Subject: [PATCH 1/3] Fix typo in spec point IDs Mistake in 9c97883. --- Sources/AblyChat/RoomFeature.swift | 6 +++--- Sources/AblyChat/RoomLifecycleManager.swift | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Sources/AblyChat/RoomFeature.swift b/Sources/AblyChat/RoomFeature.swift index 5b9968f8..0aee6b7a 100644 --- a/Sources/AblyChat/RoomFeature.swift +++ b/Sources/AblyChat/RoomFeature.swift @@ -44,9 +44,9 @@ internal protocol FeatureChannel: Sendable, EmitsDiscontinuities { /// 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: /// /// - 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-PR3e, CHA-PR10e, CHA-PR6d, CHA-T2d: If the room is in the ATTACHED status, it returns immediately. + /// - CHA-PR3f, CHA-PR10f, CHA-PR6e, CHA-T2e: If the room is in the DETACHED status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationRequiresRoomAttach(feature:)``. + /// - // CHA-PR3g, CHA-PR10g, CHA-PR6f, CHA-T2f: If the room is in any other status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationDisallowedForCurrentRoomStatus(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. diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index de05474e..472e4e6d 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -1228,13 +1228,13 @@ internal actor DefaultRoomLifecycleManager Date: Thu, 28 Nov 2024 09:53:52 -0300 Subject: [PATCH 2/3] Decouple error codes from status codes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- Sources/AblyChat/DefaultMessages.swift | 5 +- Sources/AblyChat/Errors.swift | 173 +++++++++++++----- .../DefaultRoomLifecycleManagerTests.swift | 24 +-- Tests/AblyChatTests/DefaultRoomsTests.swift | 2 +- Tests/AblyChatTests/Helpers/Helpers.swift | 6 +- 5 files changed, 151 insertions(+), 59 deletions(-) diff --git a/Sources/AblyChat/DefaultMessages.swift b/Sources/AblyChat/DefaultMessages.swift index 03339058..84836681 100644 --- a/Sources/AblyChat/DefaultMessages.swift +++ b/Sources/AblyChat/DefaultMessages.swift @@ -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" ) ) diff --git a/Sources/AblyChat/Errors.swift b/Sources/AblyChat/Errors.swift index c955e181..b9027c7b 100644 --- a/Sources/AblyChat/Errors.swift +++ b/Sources/AblyChat/Errors.swift @@ -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 } } } @@ -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) } } @@ -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. @@ -218,7 +309,7 @@ internal extension ARTErrorInfo { self.init( domain: errorDomain, - code: chatError.code.rawValue, + code: chatError.codeAndStatusCode.code.rawValue, userInfo: userInfo ) } diff --git a/Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift b/Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift index b6cc7bf9..243a0a11 100644 --- a/Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift +++ b/Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift @@ -145,7 +145,7 @@ struct DefaultRoomLifecycleManagerTests { await #expect { try await manager.performAttachOperation() } throws: { error in - isChatError(error, withCode: .roomIsReleasing) + isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.roomIsReleasing)) } } @@ -160,7 +160,7 @@ struct DefaultRoomLifecycleManagerTests { await #expect { try await manager.performAttachOperation() } throws: { error in - isChatError(error, withCode: .roomIsReleased) + isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.roomIsReleased)) } } @@ -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: @@ -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)) } } @@ -561,7 +561,7 @@ struct DefaultRoomLifecycleManagerTests { await #expect { try await manager.performDetachOperation() } throws: { error in - isChatError(error, withCode: .roomIsReleasing) + isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.roomIsReleasing)) } } @@ -576,7 +576,7 @@ struct DefaultRoomLifecycleManagerTests { await #expect { try await manager.performDetachOperation() } throws: { error in - isChatError(error, withCode: .roomIsReleased) + isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.roomIsReleased)) } } @@ -595,7 +595,7 @@ struct DefaultRoomLifecycleManagerTests { await #expect { try await manager.performDetachOperation() } throws: { error in - isChatError(error, withCode: .roomInFailedState) + isChatError(error, withCodeAndStatusCode: .fixedStatusCode(.roomInFailedState)) } } @@ -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)) } } @@ -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 { @@ -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 @@ -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 @@ -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.")) } } diff --git a/Tests/AblyChatTests/DefaultRoomsTests.swift b/Tests/AblyChatTests/DefaultRoomsTests.swift index 4cebc330..a7291a1b 100644 --- a/Tests/AblyChatTests/DefaultRoomsTests.swift +++ b/Tests/AblyChatTests/DefaultRoomsTests.swift @@ -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 diff --git a/Tests/AblyChatTests/Helpers/Helpers.swift b/Tests/AblyChatTests/Helpers/Helpers.swift index 1d0fc482..289adcd1 100644 --- a/Tests/AblyChatTests/Helpers/Helpers.swift +++ b/Tests/AblyChatTests/Helpers/Helpers.swift @@ -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 { From ba4658d58eddec409dfefbb43b09e8979909228c Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 28 Nov 2024 10:00:20 -0300 Subject: [PATCH 3/3] Implement CHA-PR3h and similar Resolves #151. --- Sources/AblyChat/Errors.swift | 45 ++++++++----------- Sources/AblyChat/RoomFeature.swift | 7 ++- Sources/AblyChat/RoomLifecycleManager.swift | 9 ++-- .../DefaultRoomLifecycleManagerTests.swift | 45 +++++-------------- 4 files changed, 34 insertions(+), 72 deletions(-) diff --git a/Sources/AblyChat/Errors.swift b/Sources/AblyChat/Errors.swift index b9027c7b..ba036c46 100644 --- a/Sources/AblyChat/Errors.swift +++ b/Sources/AblyChat/Errors.swift @@ -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 @@ -38,7 +36,6 @@ public enum ErrorCode: Int { /// 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 @@ -53,12 +50,9 @@ public enum ErrorCode: Int { case roomInFailedState case roomIsReleasing case roomIsReleased - case roomInInvalidState internal var toNumericErrorCode: ErrorCode { switch self { - case .nonspecific: - .nonspecific case .inconsistentRoomOptions: .inconsistentRoomOptions case .messagesAttachmentFailed: @@ -87,8 +81,6 @@ public enum ErrorCode: Int { .roomIsReleasing case .roomIsReleased: .roomIsReleased - case .roomInInvalidState: - .roomInInvalidState } } @@ -96,8 +88,7 @@ public enum ErrorCode: Int { internal var statusCode: Int { // These status codes are taken from the "Chat-specific Error Codes" section of the spec. switch self { - case .nonspecific, - .inconsistentRoomOptions, + case .inconsistentRoomOptions, .roomInFailedState, .roomIsReleasing, .roomIsReleased: @@ -112,9 +103,7 @@ public enum ErrorCode: Int { .presenceDetachmentFailed, .reactionsDetachmentFailed, .occupancyDetachmentFailed, - .typingDetachmentFailed, - // CHA-RL9c - .roomInInvalidState: + .typingDetachmentFailed: 500 } } @@ -122,8 +111,13 @@ public enum ErrorCode: Int { /// 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 {} + switch self { + case .roomInInvalidState: + .roomInInvalidState + } } } } @@ -169,8 +163,7 @@ internal enum ChatError { case roomIsReleasing case roomIsReleased case presenceOperationRequiresRoomAttach(feature: RoomFeature) - case presenceOperationDisallowedForCurrentRoomStatus(feature: RoomFeature) - case roomInInvalidState(cause: ARTErrorInfo?) + case roomTransitionedToInvalidStateForPresenceOperation(cause: ARTErrorInfo?) internal var codeAndStatusCode: ErrorCodeAndStatusCode { switch self { @@ -208,11 +201,12 @@ internal enum ChatError { .fixedStatusCode(.roomIsReleasing) case .roomIsReleased: .fixedStatusCode(.roomIsReleased) - case .roomInInvalidState: - .fixedStatusCode(.roomInInvalidState) - case .presenceOperationRequiresRoomAttach, - .presenceOperationDisallowedForCurrentRoomStatus: - .fixedStatusCode(.nonspecific) + case .roomTransitionedToInvalidStateForPresenceOperation: + // CHA-RL9c + .variableStatusCode(.roomInInvalidState, statusCode: 500) + case .presenceOperationRequiresRoomAttach: + // CHA-PR3h, CHA-PR10h, CHA-PR6h, CHA-T2g + .variableStatusCode(.roomInInvalidState, statusCode: 400) } } @@ -268,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." } } @@ -282,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 } } diff --git a/Sources/AblyChat/RoomFeature.swift b/Sources/AblyChat/RoomFeature.swift index 0aee6b7a..5383b8ff 100644 --- a/Sources/AblyChat/RoomFeature.swift +++ b/Sources/AblyChat/RoomFeature.swift @@ -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-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-PR3f, CHA-PR10f, CHA-PR6e, CHA-T2e: If the room is in the DETACHED status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationRequiresRoomAttach(feature:)``. - /// - // CHA-PR3g, CHA-PR10g, CHA-PR6f, CHA-T2f: If the room is in any other status, it throws an `ARTErrorInfo` derived from ``ChatError.presenceOperationDisallowedForCurrentRoomStatus(feature:)``. + /// - 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. diff --git a/Sources/AblyChat/RoomLifecycleManager.swift b/Sources/AblyChat/RoomLifecycleManager.swift index 472e4e6d..8c61522e 100644 --- a/Sources/AblyChat/RoomLifecycleManager.swift +++ b/Sources/AblyChat/RoomLifecycleManager.swift @@ -1225,17 +1225,14 @@ internal actor DefaultRoomLifecycleManager