[ECO-5616] Change non spec errors

Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift

@@ -109,10 +109,14 @@ internal protocol InternalHTTPPaginatedResponseProtocol: AnyObject, Sendable {
     var items: [JSONValue] { get }
     var hasNext: Bool { get }
     var isLast: Bool { get }
-    var statusCode: Int { get }
 
     func next() async throws(ErrorInfo) -> Self?
     func first() async throws(ErrorInfo) -> Self
+
+    var success: Bool { get }
+    var statusCode: Int { get }
+    var errorCode: Int { get }
+    var errorMessage: String? { get }
 }
 
 /// Converts a `@MainActor` callback into one that can be passed as a callback to ably-cocoa.
@@ -202,10 +206,22 @@ internal final class InternalHTTPPaginatedResponseAdapter: InternalHTTPPaginated
         underlying.isLast
     }
 
+    internal var success: Bool {
+        underlying.success
+    }
+
     internal var statusCode: Int {
         underlying.statusCode
     }
 
+    internal var errorCode: Int {
+        underlying.errorCode
+    }
+
+    internal var errorMessage: String? {
+        underlying.errorMessage
+    }
+
     internal func next() async throws(ErrorInfo) -> InternalHTTPPaginatedResponseAdapter? {
         do {
             return try await withCheckedContinuation { (continuation: CheckedContinuation<Result<InternalHTTPPaginatedResponseAdapter?, ARTErrorInfo>, _>) in

Sources/AblyChat/DefaultMessages.swift

@@ -146,33 +146,20 @@ internal final class DefaultMessages<Realtime: InternalRealtimeClientProtocol>:
 
         // (CHA-M5b) If a subscription is added when the underlying realtime channel is in any other state, then its subscription point becomes the attachSerial at the the point of channel attachment.
         return try await withCheckedContinuation { (continuation: CheckedContinuation<Result<String, ErrorInfo>, Never>) in
-            _ = channel.once { [weak self] stateChange in
+            _ = channel.once(.attached) { [weak self] stateChange in
                 guard let self else {
                     return
                 }
-                switch stateChange.current {
-                case .attached:
-                    // CHA-M5c If a channel leaves the ATTACHED state and then re-enters ATTACHED with resumed=false, then it must be assumed that messages have been missed. The subscription point of any subscribers must be reset to the attachSerial
-                    // CHA-M5d If a channel UPDATE event is received and resumed=false, then it must be assumed that messages have been missed. The subscription point of any subscribers must be reset to the attachSerial
-                    if let subscriptionPoint = stateChange.resumed ? channel.properties.channelSerial : channel.properties.attachSerial {
-                        logger.log(message: "Channel is attached, returning serial: \(subscriptionPoint)", level: .debug)
-                        continuation.resume(returning: .success(subscriptionPoint))
-                    } else {
-                        logger.log(message: "Channel is attached, but attachSerial is not defined", level: .error)
-                        continuation.resume(returning: .failure(InternalError.failedToResolveSubscriptionPointBecauseAttachSerialNotDefined.toErrorInfo()))
-                    }
-                case .failed, .suspended:
-                    let error = InternalError.failedToResolveSubscriptionPointBecauseChannelFailedToAttach(cause: stateChange.reason)
-                    logger.log(message: "\(error)", level: .error)
-                    continuation.resume(returning: .failure(error.toErrorInfo()))
-                default:
-                    break
+                // CHA-M5c If a channel leaves the ATTACHED state and then re-enters ATTACHED with resumed=false, then it must be assumed that messages have been missed. The subscription point of any subscribers must be reset to the attachSerial
+                // CHA-M5d If a channel UPDATE event is received and resumed=false, then it must be assumed that messages have been missed. The subscription point of any subscribers must be reset to the attachSerial
+                if let subscriptionPoint = stateChange.resumed ? channel.properties.channelSerial : channel.properties.attachSerial {
+                    logger.log(message: "Channel is attached, returning serial: \(subscriptionPoint)", level: .debug)
+                    continuation.resume(returning: .success(subscriptionPoint))
+                } else {
+                    logger.log(message: "Channel is attached, but attachSerial is not defined", level: .error)
+                    continuation.resume(returning: .failure(InternalError.failedToResolveSubscriptionPointBecauseChannelSerialNotDefined.toErrorInfo()))
                 }
             }
         }.get()
     }
-
-    internal enum MessagesError: Error {
-        case noReferenceToSelf
-    }
 }

Sources/AblyChat/InternalError.swift

@@ -87,25 +87,16 @@ internal enum InternalError {
     /// Error code is `badRequest` (this is our own error, which is not specified by the spec).
     case failedToResolveSubscriptionPointBecauseMessagesInstanceGone
 
-    /// Unable to fetch `historyBeforeSubscribe` because a channel in the `ATTACHED` state has violated our expectations by its `attachSerial` not being populated, so we cannot resolve its "subscription point" per CHA-M5b.
-    ///
-    /// Error code is `badRequest` (this is not specified by the spec, which does not make it explicit that the SDK should throw an error in this scenario).
-    case failedToResolveSubscriptionPointBecauseAttachSerialNotDefined
-
-    /// Unable to fetch `historyBeforeSubscribe` because whilst waiting for a channel to become attached per CHA-M5b in order to resolve its "subscription point".
-    ///
-    /// Error code is `badRequest` (this is not specified by the spec, which does not make it explicit that the SDK should throw an error in this scenario).
-    case failedToResolveSubscriptionPointBecauseChannelFailedToAttach(cause: ErrorInfo?)
+    /// Unable to fetch `historyBeforeSubscribe` because a channel in the `ATTACHED` state has violated our expectations by its `channelSerial` or `attachSerial` not being populated, so we cannot resolve its "subscription point" per CHA-M5b.
+    case failedToResolveSubscriptionPointBecauseChannelSerialNotDefined
 
     /// Attempted to load a resource from the given `path`, expecting to get a single item back, but the returned `PaginatedResult` is empty.
-    ///
-    /// Error code is `badRequest` (this is not specified by the spec, which does not make it explicit that the SDK should throw an error in this scenario).
     case noItemInResponse(path: String)
 
     /// An ably-cocoa `ARTHTTPPaginatedResponse` was received with the given non-200 status code.
     ///
     /// Error code is `badRequest` (this is not specified by the spec, which does not make it explicit that the SDK should throw an error in this scenario).
-    case paginatedResultStatusCode(Int)
+    case failedToGetPaginatedResult(cause: ErrorInfo?)
 
     // Failed to decode a `HeadersValue` from a `JSONValue`.
     ///
@@ -130,9 +121,11 @@ internal enum InternalError {
     internal enum ErrorCode: Int {
         case badRequest = 40000
         case invalidArgument = 40003
+        case notFound = 40400
         case roomDiscontinuity = 102_100
         case roomReleasedBeforeOperationCompleted = 102_106
         case roomExistsWithDifferentOptions = 102_107
+        case channelSerialNotDefined = 102_110
         case roomInInvalidState = 102_112
 
         /// The ``ErrorInfo/statusCode`` that should be returned for this error.
@@ -145,7 +138,10 @@ internal enum InternalError {
                  .roomInInvalidState,
                  .roomExistsWithDifferentOptions:
                 400
-            case .roomDiscontinuity:
+            case .notFound:
+                404
+            case .roomDiscontinuity,
+                 .channelSerialNotDefined:
                 500
             }
         }
@@ -183,13 +179,11 @@ internal enum InternalError {
             .invalidArgument
         case .cannotApplyCreatedMessageEvent:
             .invalidArgument
-        case .failedToResolveSubscriptionPointBecauseAttachSerialNotDefined:
-            .badRequest
-        case .failedToResolveSubscriptionPointBecauseChannelFailedToAttach:
-            .badRequest
+        case .failedToResolveSubscriptionPointBecauseChannelSerialNotDefined:
+            .channelSerialNotDefined
         case .noItemInResponse:
-            .badRequest
-        case .paginatedResultStatusCode:
+            .notFound
+        case .failedToGetPaginatedResult:
             .badRequest
         case .failedToResolveSubscriptionPointBecauseMessagesInstanceGone:
             .badRequest
@@ -286,12 +280,9 @@ internal enum InternalError {
         case .failedToResolveSubscriptionPointBecauseMessagesInstanceGone:
             op = "fetch message history from before subscription"
             reason = "Messages instance has been deallocated"
-        case .failedToResolveSubscriptionPointBecauseAttachSerialNotDefined:
-            op = "fetch message history from before subscription"
-            reason = "channel is attached but attachSerial is not defined"
-        case let .failedToResolveSubscriptionPointBecauseChannelFailedToAttach(cause):
+        case .failedToResolveSubscriptionPointBecauseChannelSerialNotDefined:
             op = "fetch message history from before subscription"
-            reason = "channel failed to attach: \(cause, default: "(nil cause)")"
+            reason = "channel is attached but channelSerial is not defined"
         case .sendMessageReactionEmptyMessageSerial:
             op = "send message reaction"
             reason = "message serial must not be empty"
@@ -301,9 +292,9 @@ internal enum InternalError {
         case let .noItemInResponse(path):
             op = "load resource"
             reason = "paginated result from path \(path) is empty"
-        case let .paginatedResultStatusCode(statusCode):
+        case let .failedToGetPaginatedResult(cause):
             op = "load resource"
-            reason = "received status code \(statusCode)"
+            reason = "reason: \(cause, default: "<no reason>")"
         case let .headersValueJSONDecodingError(error):
             op = "decode headers"
             switch error {
@@ -334,7 +325,7 @@ internal enum InternalError {
             cause
         case let .roomDiscontinuity(cause):
             cause
-        case let .failedToResolveSubscriptionPointBecauseChannelFailedToAttach(cause):
+        case let .failedToGetPaginatedResult(cause):
             cause
         case .jsonValueDecodingError,
              .headersValueJSONDecodingError,
@@ -344,15 +335,14 @@ internal enum InternalError {
              .presenceOperationRequiresRoomAttach,
              .cannotApplyCreatedMessageEvent,
              .unableDeleteReactionWithoutName,
-             .failedToResolveSubscriptionPointBecauseAttachSerialNotDefined,
+             .failedToResolveSubscriptionPointBecauseChannelSerialNotDefined,
              .roomInInvalidStateForAttach,
              .roomInInvalidStateForDetach,
              .cannotApplyMessageEventForDifferentMessage,
              .cannotApplyReactionSummaryEventForDifferentMessage,
              .sendMessageReactionEmptyMessageSerial,
              .deleteMessageReactionEmptyMessageSerial,
              .noItemInResponse,
-             .paginatedResultStatusCode,
              .failedToResolveSubscriptionPointBecauseMessagesInstanceGone:
             nil
         }

Sources/AblyChat/PaginatedResult.swift

@@ -41,9 +41,9 @@ internal final class DefaultPaginatedResult<Underlying: InternalHTTPPaginatedRes
 
     /// Convenience initializer that checks status code and decodes items from the response.
     internal convenience init(response: Underlying) throws(ErrorInfo) {
-        // TODO: We've had this check since the start of the codebase, but it's not specified anywhere; rectify this in https://github.com/ably/ably-chat-swift/issues/453
-        guard response.statusCode == 200 else {
-            throw InternalError.paginatedResultStatusCode(response.statusCode).toErrorInfo()
+        // (CHA-M6b) If the REST API returns an error, then the method must throw its `ErrorInfo` representation.
+        guard response.success else {
+            throw InternalError.failedToGetPaginatedResult(cause: .init(code: response.errorCode, message: response.errorMessage ?? "<no error message in response>", statusCode: response.statusCode)).toErrorInfo()
         }
 
         let items = try response.items.map { jsonValue throws(ErrorInfo) in

Tests/AblyChatTests/IntegrationTests.swift

@@ -181,6 +181,11 @@ struct IntegrationTests {
         try await txRoom.messages.reactions.send(forMessageWithSerial: messageToReact.serial, params: .init(name: "🎉"))
         Self.logAwait("AFTER txRoom.messages.reactions.send (🎉)")
 
+        // Wait a little before requesting clientReactions
+        Self.logAwait("BEFORE Task.sleep (2s for clientReactions)")
+        try await Task.sleep(nanoseconds: 2 * NSEC_PER_SEC)
+        Self.logAwait("AFTER Task.sleep (2s for clientReactions)")
+
         // Before deleting, fetch the reactions summary for txClientID and check its contents
         Self.logAwait("BEFORE rxRoom.messages.reactions.clientReactions")
         let reactionsForClient = try await rxRoom.messages.reactions.clientReactions(
@@ -273,9 +278,9 @@ struct IntegrationTests {
         #expect(reactionRawEvents[2].reaction.messageSerial == messageToReact.serial)
 
         // Wait a little before requesting history
-        Self.logAwait("BEFORE Task.sleep (2s)")
+        Self.logAwait("BEFORE Task.sleep (2s for history)")
         try await Task.sleep(nanoseconds: 2 * NSEC_PER_SEC)
-        Self.logAwait("AFTER Task.sleep (2s)")
+        Self.logAwait("AFTER Task.sleep (2s for history)")
 
         // (7) Fetch historical messages from before subscribing, and check we get txMessageBeforeRxSubscribe
 

Tests/AblyChatTests/Mocks/MockHTTPPaginatedResponse.swift

@@ -3,18 +3,28 @@ import Ably
 
 final class MockHTTPPaginatedResponse: InternalHTTPPaginatedResponseProtocol {
     let items: [JSONValue]
-    let statusCode: Int
     let headers: [String: String]
     let hasNext: Bool
 
+    var success: Bool
+    let statusCode: Int
+    var errorCode: Int
+    var errorMessage: String?
+
     init(
         items: [[String: JSONValue]],
+        success: Bool = true,
         statusCode: Int = 200,
+        errorCode: Int = 0,
+        errorMessage: String? = nil,
         headers: [String: String] = [:],
         hasNext: Bool = false,
     ) {
         self.items = items.map { .object($0) }
+        self.success = success
         self.statusCode = statusCode
+        self.errorCode = errorCode
+        self.errorMessage = errorMessage
         self.headers = headers
         self.hasNext = hasNext
     }

Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift

@@ -184,8 +184,12 @@ final class MockRealtimeChannel: InternalRealtimeChannelProtocol {
         return ARTEventListener()
     }
 
-    func once(_: ARTChannelEvent, callback _: @escaping @MainActor @Sendable (ChannelStateChange) -> Void) -> ARTEventListener {
-        fatalError("Not implemented")
+    func once(_: ARTChannelEvent, callback: @escaping @MainActor @Sendable (ChannelStateChange) -> Void) -> ARTEventListener {
+        stateSubscriptionCallbacks.append(callback)
+        if let stateChangeToEmitForListener {
+            callback(stateChangeToEmitForListener)
+        }
+        return ARTEventListener()
     }
 
     func emitEvent(_ event: ChannelStateChange) {