Remove InternalError's conformance to Error
#434
Conversation
WalkthroughThe pull request refactors error handling throughout the Ably Chat SDK by replacing the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant ChatAPI as ChatAPI<br/>(public)
participant Adapter as Adapter<br/>(internal)
participant AblyCocoa as Ably Cocoa<br/>(ARTErrorInfo)
Note over Client,AblyCocoa: Error Flow (New)
Client->>ChatAPI: call method
ChatAPI->>Adapter: delegate to adapter
Adapter->>AblyCocoa: make request
AblyCocoa-->>Adapter: throw ARTErrorInfo
Note over Adapter: Convert: ARTErrorInfo<br/>→ ErrorInfo(ablyCocoaError:)
Adapter-->>ChatAPI: throw ErrorInfo
ChatAPI-->>Client: throw ErrorInfo
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes This refactor spans 30+ files with changes to error propagation across protocol definitions, adapters, feature implementations, and tests. While most changes follow a homogeneous pattern of replacing Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
lawrence-forooghian
force-pushed
the
32-do-not-throw-InternalError
branch
from
6b66b2d to
1072f43
Compare
lawrence-forooghian
changed the title
InternalErrorInternalError
github-actions
bot
temporarily deployed
to
staging/pull/434/AblyChat
lawrence-forooghian
force-pushed
the
32-do-not-throw-InternalError
branch
from
1072f43 to
f3e156a
Compare
github-actions
bot
temporarily deployed
to
staging/pull/434/AblyChat
github-actions
bot
temporarily deployed
to
staging/pull/434/AblyChat
lawrence-forooghian
force-pushed
the
32-do-not-throw-InternalError
branch
from
164c5c4 to
bc7d31e
Compare
github-actions
bot
temporarily deployed
to
staging/pull/434/AblyChat
lawrence-forooghian
force-pushed
the
32-do-not-throw-InternalError
branch
from
bc7d31e to
672cb8b
Compare
lawrence-forooghian
changed the title
InternalError
github-actions
bot
temporarily deployed
to
staging/pull/434/AblyChat
lawrence-forooghian
changed the title
InternalError's conformance to Error
Simplify things internally by always just throwing ErrorInfo.
Now that we're just throwing ErrorInfo everywhere, let's simplify how we handle errors in tests; that is, get rid of our usage of `any Error`.
We change our ably-cocoa wrapper types to all use our ErrorInfo types and update the mocks accordingly. This simplifies the tests, which now largely no longer need to create ARTErrorInfo instances.
This simplfies the InternalError type so that it no longer has a nested InternallyThrown enum (that enum's cases become the cases of InternalError). This simplifies the meaning of InternalError to "an error thrown by the Chat SDK itself".
To make it clearer how they're connected.
lawrence-forooghian
force-pushed
the
32-do-not-throw-InternalError
branch
from
672cb8b to
1a43e71
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (7)
Sources/AblyChat/DefaultMessages.swift (2)
148-153: Unresumed continuation if self deallocates — must resume with failure.If self is nil, the continuation never resumes, causing a hang/leak. Resume with a failure ErrorInfo.
- return try await withCheckedContinuation { (continuation: CheckedContinuation<Result<String, ErrorInfo>, Never>) in - _ = channel.once { [weak self] stateChange in - guard let self else { - return - } + return try await withCheckedContinuation { (continuation: CheckedContinuation<Result<String, ErrorInfo>, Never>) in + _ = channel.once { [weak self] stateChange in + guard let self else { + continuation.resume(returning: .failure(MessagesError.noReferenceToSelf.toErrorInfo())) + return + } switch stateChange.current {
98-109: Mark subscriptionStartSerial closure as async—required to match property type and resolve await expression.The property type expects
() async throws(ErrorInfo) -> String, and the closure body containstry await, making theasynckeyword mandatory for compilation.- subscriptionStartSerial: { [weak self] () throws(ErrorInfo) in + subscriptionStartSerial: { [weak self] () async throws(ErrorInfo) in guard let self else { throw MessagesError.noReferenceToSelf.toErrorInfo() }Sources/AblyChat/PaginatedResult.swift (1)
52-54: Bug: passing any Error into a Result expecting ErrorInfo fails to compileThe catch block resumes the continuation with a generic Error, but the Result’s failure type is ErrorInfo. Cast to ErrorInfo (the only type thrown in the do block) to avoid a type mismatch.
- } catch { - continuation.resume(returning: .failure(error)) - } + } catch let error as ErrorInfo { + continuation.resume(returning: .failure(error)) + }Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift (1)
238-244: Tests: assign caught errors as ErrorInfoIn these do/catch blocks the caught error is any Error. Assigning directly to ErrorInfo? will not compile; cast to ErrorInfo. You already do this at Lines 1029–1030.
- var roomAttachError: ErrorInfo? + var roomAttachError: ErrorInfo? do { try await manager.performAttachOperation() } catch { - roomAttachError = error + roomAttachError = error as? ErrorInfo } ... - var roomDetachError: ErrorInfo? + var roomDetachError: ErrorInfo? do { try await manager.performDetachOperation() } catch { - roomDetachError = error + roomDetachError = error as? ErrorInfo } ... - var caughtError: ErrorInfo? + var caughtError: ErrorInfo? do { try await waitToBeAbleToPerformPresenceOperationsResult } catch { - caughtError = error as? ErrorInfo + caughtError = error as? ErrorInfo }Also applies to: 435-441, 1025-1034
Sources/AblyChat/DefaultMessageReactions.swift (1)
96-101: Do not fatalError; throw the specified ErrorInfo per spec (CHA-MR7a)If rawMessageReactions is false, the API must throw an ErrorInfo (code 40000, status 400). fatalError will crash apps and violates the spec.
- guard options.rawMessageReactions else { - // (CHA-MR7a) The attempt to subscribe to raw message reactions must throw an ErrorInfo with code 40000 and status code 400 if the room is not configured to support raw message reactions - // I'm replacing throwing with `fatalError` because it's a programmer error to call this method with invalid options. - fatalError("Room is not configured to support raw message reactions") - } + guard options.rawMessageReactions else { + // (CHA-MR7a) + throw ChatError.roomNotConfiguredForRawReactions.toErrorInfo() + }Sources/AblyChat/Rooms.swift (1)
66-66: Mark DefaultRooms as @mainactor to isolate mutable state.You rely on MainActor semantics in comments; make it explicit to protect roomStates and related state.
-internal class DefaultRooms<RoomFactory: AblyChat.RoomFactory>: Rooms { +@MainActor +internal class DefaultRooms<RoomFactory: AblyChat.RoomFactory>: Rooms {As per coding guidelines.
Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
4-4: Fix actor isolation: annotate the mock with @mainactor and store @mainactor callbacks.You call @mainactor closures (
callback(...)) from non-@mainactor methods (subscribe,emitEvent, etc.). Without actor isolation, this requiresawaitand will fail under strict concurrency checks. Mark the class @mainactor and store @MainActor-typed closures to keep calls synchronous and actor-safe.Apply:
- final class MockRealtimeChannel: InternalRealtimeChannelProtocol { + @MainActor + final class MockRealtimeChannel: InternalRealtimeChannelProtocol { @@ - private var channelSubscriptions: [(String, (ARTMessage) -> Void)] = [] + private var channelSubscriptions: [(String, @MainActor (ARTMessage) -> Void)] = []This change:
- Ensures all mutable state and callbacks are main-actor isolated (as per guidelines).
- Allows direct
callback(...)calls withoutawait.
As per coding guidelinesAlso applies to: 139-152, 164-177, 191-195
🧹 Nitpick comments (18)
Sources/AblyChat/DefaultMessages.swift (1)
1-7: Mark DefaultMessages as @mainactor to isolate mutable state.currentSubscriptionPoint and subscriptionPoints mutate from callbacks; marking the class @mainactor aligns with our concurrency guideline.
-import Ably +import Ably + +@MainActor internal final class DefaultMessages: Messages {Also applies to: 12-14
Tests/AblyChatTests/RoomReactionDTOTests.swift (1)
9-14: Add @SPEC tags to align with established codebase pattern; Xcode version concern is resolved (CI uses 26.0).The test file lacks @SPEC tags despite ErrorInfo tests being good additions. Across the codebase, all other test files extensively use @SPEC, @specOneOf, @specPartial, @specUntested, and @specNotApplicable tags. Adding appropriate tags here would align with the established practice. Additionally, CI is running Xcode 26.0, well above the 16.3+ threshold, so #require(throws: ErrorInfo.self) is stable and the Xcode version verification is unnecessary.
Also applies to: 17-22, 70-75
Tests/AblyChatTests/DefaultRoomsTests.swift (1)
365-374: Simplify error capture in test.Replace manual do/catch with #require(throws:) for brevity and clarity.
- let roomGetError: ErrorInfo? - do { - _ = try await fetchedRoom - roomGetError = nil - } catch { - roomGetError = error as? ErrorInfo - } - - #expect(try #require(roomGetError).hasCodeAndStatusCode(.fixedStatusCode(.roomReleasedBeforeOperationCompleted))) + let thrownError = try await #require(throws: ErrorInfo.self) { + _ = try await fetchedRoom + } + #expect(thrownError.hasCodeAndStatusCode(.fixedStatusCode(.roomReleasedBeforeOperationCompleted)))Sources/AblyChat/ChatAPI.swift (2)
141-155: Precondition checks for empty serial are correct; consider validating name/type if needed.Guarding empty serial is required by spec. If upstream doesn’t validate, you might also enforce name/type presence here and map to ChatError.messageReactionNameRequired/typeRequired for defense-in-depth.
Also applies to: 158-172
175-186: Add the same empty-serial validation to getClientReactions.For symmetry with send/delete and clearer failure before HTTP, validate messageSerial is non-empty.
internal func getClientReactions(forMessageWithSerial messageSerial: String, roomName: String, clientID: String?) async throws(ErrorInfo) -> MessageReactionSummary { + guard !messageSerial.isEmpty else { + throw ChatError.messageReactionInvalidMessageSerial.toErrorInfo() + }Tests/AblyChatTests/DefaultMessageReactionsTests.swift (3)
78-79: Prefer code/status assertions over full ErrorInfo equality.Equality depends on Equatable semantics for ErrorInfo; asserting code (40000) or using hasCodeAndStatusCode is more robust.
- #expect(thrownError == InternalError.other(.chatAPIChatError(.messageReactionInvalidMessageSerial)).toErrorInfo()) + #expect(thrownError?.code == 40000) + // or: #expect(thrownError?.hasCodeAndStatusCode(.fixedStatusCode(.badRequest)) == true)
98-99: Same here: assert by code/status instead of equality.Keeps the test decoupled from ErrorInfo’s equality implementation.
- #expect(thrownError == InternalError.other(.chatAPIChatError(.messageReactionInvalidMessageSerial)).toErrorInfo()) + #expect(thrownError?.code == 40000)
516-517: Asserting error code is sufficient; consider also asserting statusCode for completeness.Optional: add #expect(thrownError?.statusCode == 404).
Sources/AblyChat/ErrorInfo.swift (1)
172-181: Standardize description format for ably-cocoa-backed errors.Consider emitting the same “(status:code) message. See helpHref.” format for all sources to keep logs uniform, while appending ablyCocoaError.localizedDescription as extra context.
- case let .fromAblyCocoa(ablyCocoaError): - ablyCocoaError.localizedDescription + case let .fromAblyCocoa(ablyCocoaError): + "(\(statusCode):\(code)) \(message). See \(helpHref). Underlying: \(ablyCocoaError.localizedDescription)"If you prefer the current localizedDescription-only output, please confirm that log consumers don’t rely on code/status formatting for parsing.
Sources/AblyChat/PaginatedResult.swift (1)
42-45: Consider broader success range for HTTP status codesIf non-200 2xx responses are possible from Ably for paginated endpoints, guarding strictly on 200 will misclassify valid responses as errors. Consider allowing 200...299.
- guard let paginatedResponse, paginatedResponse.statusCode == 200 else { + guard let paginatedResponse, (200...299).contains(paginatedResponse.statusCode) else {Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift (1)
179-181: Async let usage: avoid awaiting in initializer; ensure eventual awaitUsing try await inside async let defeats concurrency and can be confusing. Prefer starting the task without await and awaiting it later (or store a Task and cancel/await explicitly).
- async let _ = try await manager.performAttachOperation() + async let attachTask: Void = manager.performAttachOperation() ... - async let _ = try await manager.performDetachOperation() + async let detachTask: Void = manager.performDetachOperation()Then await where appropriate:
try await attachTask try await detachTaskAlso applies to: 379-380, 351-353, 528-531
Sources/AblyChat/DefaultPresence.swift (1)
21-27: DRY: factor the repeated preflight wait+log into a helperThe same “wait to be able to perform presence ops” + log pattern is repeated across get/get(withParams)/isUserPresent/enter/update/leave. Extract a small helper to reduce duplication and keep log messages uniform.
+ @inline(__always) + private func waitForPresencePreconditions(_ op: StaticString) async throws(ErrorInfo) { + do { + try await roomLifecycleManager.waitToBeAbleToPerformPresenceOperations(requestedByFeature: .presence) + } catch { + logger.log(message: "Error waiting to be able to perform presence \(op) operation: \(error)", level: .error) + throw error + } + }Usage examples:
- // CHA-PR6b to CHA-PR6f - do { try await roomLifecycleManager.waitToBeAbleToPerformPresenceOperations(requestedByFeature: .presence) } catch { ... } + // CHA-PR6b to CHA-PR6f + try await waitForPresencePreconditions("get")Also applies to: 42-48, 64-70, 95-101, 123-129, 151-157
Sources/AblyChat/DefaultMessageReactions.swift (1)
21-31: Count defaulting logic could defer to ChatAPI to avoid divergenceYou pre-default count to 1 for .multiple only when params.type == .multiple. If params.type is nil and the default type is .multiple, count remains nil. ChatAPI already defaults count to 1, so consider removing the local defaulting to keep a single source of truth.
- var count = params.count - if params.type == .multiple, params.count == nil { - count = 1 - } ... - count: count, + count: params.count,Sources/AblyChat/InternalError.swift (1)
240-242: Avoid “Optional(…)” in error message.Format cause only when present for cleaner output.
- case let .channelFailedToAttach(cause): - "Channel failed to attach: \(String(describing: cause))" + case let .channelFailedToAttach(cause): + if let cause { + "Channel failed to attach: \(cause.message)" + } else { + "Channel failed to attach." + }Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (1)
15-18: Make presence-wait result non-optional; remove fatalError guard.This value is always provided by init; simplify and avoid unreachable fatalError.
- private let resultOfWaitToBeAbleToPerformPresenceOperations: Result<Void, ErrorInfo>? + private let resultOfWaitToBeAbleToPerformPresenceOperations: Result<Void, ErrorInfo> @@ - init(attachResult: Result<Void, ErrorInfo>? = nil, detachResult: Result<Void, ErrorInfo>? = nil, roomStatus: RoomStatus? = nil, error: ErrorInfo? = nil, resultOfWaitToBeAbleToPerformPresenceOperations: Result<Void, ErrorInfo> = .success(())) { + init(attachResult: Result<Void, ErrorInfo>? = nil, detachResult: Result<Void, ErrorInfo>? = nil, roomStatus: RoomStatus? = nil, error: ErrorInfo? = nil, resultOfWaitToBeAbleToPerformPresenceOperations: Result<Void, ErrorInfo> = .success(())) { @@ - func waitToBeAbleToPerformPresenceOperations(requestedByFeature: RoomFeature) async throws(ErrorInfo) { - guard let resultOfWaitToBeAbleToPerformPresenceOperations else { - fatalError("resultOfWaitToBeAblePerformPresenceOperations must be set before waitToBeAbleToPerformPresenceOperations is called") - } + func waitToBeAbleToPerformPresenceOperations(requestedByFeature: RoomFeature) async throws(ErrorInfo) { callRecorder.addRecord( signature: "waitToBeAbleToPerformPresenceOperations(requestedByFeature:)", arguments: ["requestedByFeature": "\(requestedByFeature)"], ) try resultOfWaitToBeAbleToPerformPresenceOperations.get() }Also applies to: 60-69
Tests/AblyChatTests/Helpers/Helpers.swift (1)
139-142: Fix typo in comment.Minor spelling nit.
- // Probaly there is a better way of doing this + // Probably there is a better way of doing thisSources/AblyChat/Rooms.swift (1)
276-279: Fix logging typo.Spelling nit in log message.
- logger.log(message: "Recieved request to fail room creation with error \(error)", level: .debug) + logger.log(message: "Received request to fail room creation with error \(error)", level: .debug)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
166-169: Optional: event‑specific on(...) is not filtering; all listeners receive all events.
on(_:callback:)ignores the event parameter andemitEvent(_:)broadcasts to every callback. If tests start relying on event scoping, consider tracking the subscribedARTChannelEvent?alongside the callback and filtering on emit.Example approach:
- Store:
[(event: ARTChannelEvent?, callback: @MainActor (ChannelStateChange) -> Void)]- In
on(event, ...)append withevent, inon(_:)append withnil.- In
emitEvent(_ change: ChannelStateChange), filter bychange.event(if present) matching the storedeventwhen non-nil.Also applies to: 191-195
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (40)
CONTRIBUTING.md(1 hunks)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift(19 hunks)Sources/AblyChat/ChatAPI.swift(10 hunks)Sources/AblyChat/DefaultConnection.swift(2 hunks)Sources/AblyChat/DefaultMessageReactions.swift(2 hunks)Sources/AblyChat/DefaultMessages.swift(4 hunks)Sources/AblyChat/DefaultOccupancy.swift(1 hunks)Sources/AblyChat/DefaultPresence.swift(5 hunks)Sources/AblyChat/DefaultRoomReactions.swift(1 hunks)Sources/AblyChat/DefaultTyping.swift(4 hunks)Sources/AblyChat/ErrorInfo.swift(5 hunks)Sources/AblyChat/Errors.swift(0 hunks)Sources/AblyChat/Headers.swift(2 hunks)Sources/AblyChat/InternalError.swift(1 hunks)Sources/AblyChat/JSONCodable.swift(22 hunks)Sources/AblyChat/Message.swift(6 hunks)Sources/AblyChat/MessageReaction+JSON.swift(5 hunks)Sources/AblyChat/Occupancy.swift(1 hunks)Sources/AblyChat/PaginatedResult.swift(2 hunks)Sources/AblyChat/Room.swift(3 hunks)Sources/AblyChat/RoomLifecycleManager.swift(14 hunks)Sources/AblyChat/RoomReactionDTO.swift(2 hunks)Sources/AblyChat/Rooms.swift(5 hunks)Sources/AblyChat/Subscription.swift(1 hunks)Tests/AblyChatTests/ChatAPITests.swift(2 hunks)Tests/AblyChatTests/DefaultMessageReactionsTests.swift(4 hunks)Tests/AblyChatTests/DefaultMessagesTests.swift(13 hunks)Tests/AblyChatTests/DefaultPresenceTests.swift(12 hunks)Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift(28 hunks)Tests/AblyChatTests/DefaultRoomTests.swift(4 hunks)Tests/AblyChatTests/DefaultRoomsTests.swift(3 hunks)Tests/AblyChatTests/ErrorInfoTests.swift(1 hunks)Tests/AblyChatTests/Helpers/Helpers.swift(2 hunks)Tests/AblyChatTests/Mocks/MockConnection.swift(1 hunks)Tests/AblyChatTests/Mocks/MockRealtime.swift(2 hunks)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift(7 hunks)Tests/AblyChatTests/Mocks/MockRealtimePresence.swift(1 hunks)Tests/AblyChatTests/Mocks/MockRoomFactory.swift(1 hunks)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift(2 hunks)Tests/AblyChatTests/RoomReactionDTOTests.swift(2 hunks)
💤 Files with no reviewable changes (1)
- Sources/AblyChat/Errors.swift
🧰 Additional context used
📓 Path-based instructions (4)
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Use protocol-based design; expose SDK functionality via protocols and prefer associated types with opaque return types (some Protocol) instead of existentials (any Protocol)
Isolate all mutable state to the main actor; mark stateful objects with @mainactor
Public API must use typed throws with ErrorInfo; use InternalError internally and convert at the public API boundary
For public structs emitted by the API, provide an explicit public memberwise initializer
When using AsyncSequence operators in @mainactor contexts, mark operator closures as @sendable
Task, CheckedContinuation, and AsyncThrowingStream do not support typed errors; use Result and call .get() to surface typed errors
Do not use Dictionary.mapValues for typed throws; use ablyChat_mapValuesWithTypedThrow instead
When the compiler struggles with typed throws, explicitly declare the error type on do blocks (e.g., do throws(InternalError))
Specify error types in closures using the throws(ErrorType) syntax (e.g., try items.map { jsonValue throws(InternalError) in ... })
Mark any test-only APIs with testsOnly_ prefix and wrap them in #if DEBUG
Files:
Tests/AblyChatTests/DefaultRoomsTests.swiftSources/AblyChat/JSONCodable.swiftSources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swiftTests/AblyChatTests/Mocks/MockRoomFactory.swiftSources/AblyChat/Subscription.swiftSources/AblyChat/DefaultOccupancy.swiftSources/AblyChat/InternalError.swiftSources/AblyChat/MessageReaction+JSON.swiftSources/AblyChat/RoomReactionDTO.swiftTests/AblyChatTests/Mocks/MockRealtime.swiftTests/AblyChatTests/ChatAPITests.swiftTests/AblyChatTests/ErrorInfoTests.swiftSources/AblyChat/DefaultRoomReactions.swiftTests/AblyChatTests/DefaultPresenceTests.swiftSources/AblyChat/Headers.swiftSources/AblyChat/DefaultMessageReactions.swiftSources/AblyChat/DefaultPresence.swiftSources/AblyChat/RoomLifecycleManager.swiftSources/AblyChat/DefaultConnection.swiftTests/AblyChatTests/Helpers/Helpers.swiftTests/AblyChatTests/DefaultMessageReactionsTests.swiftSources/AblyChat/Room.swiftSources/AblyChat/DefaultMessages.swiftSources/AblyChat/PaginatedResult.swiftTests/AblyChatTests/Mocks/MockRoomLifecycleManager.swiftSources/AblyChat/DefaultTyping.swiftSources/AblyChat/Message.swiftTests/AblyChatTests/RoomReactionDTOTests.swiftTests/AblyChatTests/Mocks/MockRealtimeChannel.swiftTests/AblyChatTests/Mocks/MockConnection.swiftSources/AblyChat/ErrorInfo.swiftTests/AblyChatTests/Mocks/MockRealtimePresence.swiftSources/AblyChat/ChatAPI.swiftTests/AblyChatTests/DefaultMessagesTests.swiftTests/AblyChatTests/DefaultRoomTests.swiftSources/AblyChat/Occupancy.swiftSources/AblyChat/Rooms.swiftTests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift
Tests/AblyChatTests/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Tests/AblyChatTests/**/*.swift: Use test attribution tags to reference spec coverage: @SPEC, @specOneOf(m/n), @specPartial, @specUntested, @specNotApplicable
For Swift Testing #expect(throws:) with typed errors, move typed-throw code into a separate non-typed-throw function until Xcode 16.3+
Files:
Tests/AblyChatTests/DefaultRoomsTests.swiftTests/AblyChatTests/Mocks/MockRoomFactory.swiftTests/AblyChatTests/Mocks/MockRealtime.swiftTests/AblyChatTests/ChatAPITests.swiftTests/AblyChatTests/ErrorInfoTests.swiftTests/AblyChatTests/DefaultPresenceTests.swiftTests/AblyChatTests/Helpers/Helpers.swiftTests/AblyChatTests/DefaultMessageReactionsTests.swiftTests/AblyChatTests/Mocks/MockRoomLifecycleManager.swiftTests/AblyChatTests/RoomReactionDTOTests.swiftTests/AblyChatTests/Mocks/MockRealtimeChannel.swiftTests/AblyChatTests/Mocks/MockConnection.swiftTests/AblyChatTests/Mocks/MockRealtimePresence.swiftTests/AblyChatTests/DefaultMessagesTests.swiftTests/AblyChatTests/DefaultRoomTests.swiftTests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift
Tests/AblyChatTests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place tests under Tests/AblyChatTests/ as the root for test targets
Files:
Tests/AblyChatTests/DefaultRoomsTests.swiftTests/AblyChatTests/Mocks/MockRoomFactory.swiftTests/AblyChatTests/Mocks/MockRealtime.swiftTests/AblyChatTests/ChatAPITests.swiftTests/AblyChatTests/ErrorInfoTests.swiftTests/AblyChatTests/DefaultPresenceTests.swiftTests/AblyChatTests/Helpers/Helpers.swiftTests/AblyChatTests/DefaultMessageReactionsTests.swiftTests/AblyChatTests/Mocks/MockRoomLifecycleManager.swiftTests/AblyChatTests/RoomReactionDTOTests.swiftTests/AblyChatTests/Mocks/MockRealtimeChannel.swiftTests/AblyChatTests/Mocks/MockConnection.swiftTests/AblyChatTests/Mocks/MockRealtimePresence.swiftTests/AblyChatTests/DefaultMessagesTests.swiftTests/AblyChatTests/DefaultRoomTests.swiftTests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift
Tests/AblyChatTests/Mocks/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Put mock implementations under Tests/AblyChatTests/Mocks/
Files:
Tests/AblyChatTests/Mocks/MockRoomFactory.swiftTests/AblyChatTests/Mocks/MockRealtime.swiftTests/AblyChatTests/Mocks/MockRoomLifecycleManager.swiftTests/AblyChatTests/Mocks/MockRealtimeChannel.swiftTests/AblyChatTests/Mocks/MockConnection.swiftTests/AblyChatTests/Mocks/MockRealtimePresence.swift
🧬 Code graph analysis (33)
Tests/AblyChatTests/DefaultRoomsTests.swift (2)
Sources/AblyChat/Rooms.swift (2)
get(60-63)get(164-269)Tests/AblyChatTests/Helpers/Helpers.swift (1)
hasCodeAndStatusCode(8-24)
Sources/AblyChat/JSONCodable.swift (1)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (4)
Tests/AblyChatTests/Mocks/MockRealtime.swift (1)
request(29-45)Tests/AblyChatTests/Mocks/MockAblyCocoaRealtime.swift (23)
request(44-46)attach(86-88)attach(90-92)detach(94-96)detach(98-100)on(138-140)on(142-144)on(376-378)on(380-382)once(146-148)once(150-152)once(384-386)once(388-390)publish(170-172)publish(174-176)publish(178-180)publish(182-184)publish(186-188)publish(190-192)publish(194-196)publish(198-200)publish(202-204)publish(206-208)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (7)
attach(94-102)detach(108-116)on(166-169)on(171-177)once(179-185)once(187-189)publish(208-210)Tests/AblyChatTests/Mocks/MockConnection.swift (1)
on(14-16)
Tests/AblyChatTests/Mocks/MockRoomFactory.swift (2)
Sources/AblyChat/Rooms.swift (1)
createRoom(298-303)Sources/AblyChat/Room.swift (1)
createRoom(251-260)
Sources/AblyChat/Subscription.swift (3)
Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift (3)
unsubscribe(186-188)unsubscribe(215-217)historyBeforeSubscribe(211-213)Sources/AblyChat/Messages.swift (2)
historyBeforeSubscribe(447-449)toHistoryParams(333-335)Sources/AblyChat/ChatAPI.swift (1)
getMessages(31-34)
Sources/AblyChat/DefaultOccupancy.swift (2)
Sources/AblyChat/Logging.swift (3)
log(71-74)log(103-110)log(117-119)Sources/AblyChat/ChatAPI.swift (1)
getOccupancy(134-137)
Sources/AblyChat/InternalError.swift (6)
Tests/AblyChatTests/DefaultRoomTests.swift (1)
_(329-338)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (2)
attach(351-365)detach(367-381)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (2)
attach(94-102)detach(108-116)Sources/AblyChat/Room.swift (2)
attach(377-379)detach(382-384)Example/AblyChatExample/Mocks/MockClients.swift (2)
attach(85-87)detach(89-91)Tests/AblyChatTests/Mocks/MockRoom.swift (2)
attach(47-49)detach(51-53)
Sources/AblyChat/RoomReactionDTO.swift (2)
Sources/AblyChat/JSONCodable.swift (1)
optionalObjectValueForKey(73-87)Sources/AblyChat/Extensions/Dictionary+Extensions.swift (1)
ablyChat_mapValuesWithTypedThrow(3-7)
Tests/AblyChatTests/Mocks/MockRealtime.swift (3)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (1)
request(148-169)Tests/AblyChatTests/Mocks/MockAblyCocoaRealtime.swift (1)
request(44-46)Tests/AblyChatTests/Helpers/Helpers.swift (1)
addRecord(112-116)
Tests/AblyChatTests/ChatAPITests.swift (2)
Sources/AblyChat/ChatAPI.swift (2)
sendMessage(63-78)getMessages(31-34)Tests/AblyChatTests/Helpers/Helpers.swift (1)
createArbitraryError(160-162)
Tests/AblyChatTests/ErrorInfoTests.swift (1)
Tests/AblyChatTests/Helpers/Helpers.swift (1)
hasCodeAndStatusCode(8-24)
Sources/AblyChat/DefaultRoomReactions.swift (3)
Sources/AblyChat/Logging.swift (3)
log(71-74)log(103-110)log(117-119)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (1)
publish(383-397)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
publish(208-210)
Tests/AblyChatTests/DefaultPresenceTests.swift (5)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (5)
enter(277-291)update(293-307)get(225-241)get(243-259)get(447-450)Sources/AblyChat/DefaultPresence.swift (8)
enter(83-85)enter(87-89)enter(92-109)update(111-113)update(115-117)update(120-137)get(18-37)get(39-58)Tests/AblyChatTests/Mocks/MockRealtimePresence.swift (4)
enter(42-47)update(49-54)get(19-25)get(27-33)Tests/AblyChatTests/Helpers/Helpers.swift (1)
hasCodeAndStatusCode(8-24)
Sources/AblyChat/Headers.swift (1)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)
Sources/AblyChat/DefaultMessageReactions.swift (3)
Sources/AblyChat/ChatAPI.swift (3)
sendReactionToMessage(140-155)deleteReactionFromMessage(158-172)getClientReactions(175-186)Sources/AblyChat/Logging.swift (3)
log(71-74)log(103-110)log(117-119)Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)
Sources/AblyChat/DefaultPresence.swift (4)
Sources/AblyChat/RoomLifecycleManager.swift (1)
waitToBeAbleToPerformPresenceOperations(586-624)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (1)
waitToBeAbleToPerformPresenceOperations(60-69)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (6)
get(225-241)get(243-259)get(447-450)enter(277-291)update(293-307)leave(261-275)Tests/AblyChatTests/Mocks/MockRealtimePresence.swift (5)
get(19-25)get(27-33)enter(42-47)update(49-54)leave(35-40)
Sources/AblyChat/RoomLifecycleManager.swift (5)
Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (3)
performAttachOperation(25-31)performDetachOperation(33-39)waitToBeAbleToPerformPresenceOperations(60-69)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (4)
update(293-307)get(225-241)get(243-259)get(447-450)Sources/AblyChat/DefaultPresence.swift (5)
update(111-113)update(115-117)update(120-137)get(18-37)get(39-58)Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
get(80-87)
Tests/AblyChatTests/DefaultMessageReactionsTests.swift (1)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)
Sources/AblyChat/Room.swift (6)
Sources/AblyChat/Rooms.swift (1)
createRoom(298-303)Tests/AblyChatTests/Mocks/MockRoomFactory.swift (1)
createRoom(16-25)Sources/AblyChat/RoomLifecycleManager.swift (4)
performAttachOperation(373-375)performAttachOperation(377-379)performDetachOperation(436-438)performDetachOperation(440-442)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (2)
performAttachOperation(25-31)performDetachOperation(33-39)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (1)
detach(367-381)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
detach(108-116)
Sources/AblyChat/DefaultMessages.swift (3)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/ChatAPI.swift (5)
getMessages(31-34)sendMessage(63-78)updateMessage(82-113)deleteMessage(117-132)getMessage(37-40)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (4)
update(293-307)get(225-241)get(243-259)get(447-450)
Sources/AblyChat/PaginatedResult.swift (2)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (3)
get(225-241)get(243-259)get(447-450)
Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (2)
Sources/AblyChat/RoomLifecycleManager.swift (5)
performAttachOperation(373-375)performAttachOperation(377-379)performDetachOperation(436-438)performDetachOperation(440-442)waitToBeAbleToPerformPresenceOperations(586-624)Sources/AblyChat/DefaultPresence.swift (2)
get(18-37)get(39-58)
Sources/AblyChat/DefaultTyping.swift (1)
Sources/AblyChat/TypingOperationQueue.swift (1)
enqueue(65-83)
Sources/AblyChat/Message.swift (3)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/JSONCodable.swift (1)
objectValueForKey(58-68)Sources/AblyChat/Extensions/Dictionary+Extensions.swift (1)
ablyChat_mapValuesWithTypedThrow(3-7)
Tests/AblyChatTests/RoomReactionDTOTests.swift (1)
Tests/AblyChatTests/Helpers/Helpers.swift (1)
hasInternalErrorCase(45-51)
Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (10)
get(225-241)get(243-259)get(447-450)attach(351-365)detach(367-381)on(187-191)on(399-403)on(405-409)once(411-415)once(417-421)
Tests/AblyChatTests/Mocks/MockConnection.swift (2)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (3)
on(187-191)on(399-403)on(405-409)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (2)
on(166-169)on(171-177)
Tests/AblyChatTests/Mocks/MockRealtimePresence.swift (2)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (6)
get(225-241)get(243-259)get(447-450)leave(261-275)enter(277-291)update(293-307)Sources/AblyChat/DefaultPresence.swift (11)
get(18-37)get(39-58)leave(139-141)leave(143-145)leave(148-165)enter(83-85)enter(87-89)enter(92-109)update(111-113)update(115-117)update(120-137)
Sources/AblyChat/ChatAPI.swift (4)
Sources/AblyChat/JSONCodable.swift (1)
stringValueForKey(130-140)Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (1)
request(148-169)Tests/AblyChatTests/Mocks/MockRealtime.swift (1)
request(29-45)
Tests/AblyChatTests/DefaultMessagesTests.swift (3)
Tests/AblyChatTests/Helpers/Helpers.swift (1)
createArbitraryError(160-162)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
emitEvent(191-195)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (1)
update(293-307)
Tests/AblyChatTests/DefaultRoomTests.swift (6)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (2)
attach(351-365)detach(367-381)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (2)
attach(94-102)detach(108-116)Sources/AblyChat/Room.swift (2)
attach(377-379)detach(382-384)Example/AblyChatExample/Mocks/MockClients.swift (2)
attach(85-87)detach(89-91)Tests/AblyChatTests/Mocks/MockRoom.swift (2)
attach(47-49)detach(51-53)Tests/AblyChatTests/Mocks/MockAblyCocoaRealtime.swift (4)
attach(86-88)attach(90-92)detach(94-96)detach(98-100)
Sources/AblyChat/Rooms.swift (3)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/Room.swift (1)
createRoom(251-260)Tests/AblyChatTests/Mocks/MockRoomFactory.swift (1)
createRoom(16-25)
Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift (5)
Sources/AblyChat/RoomLifecycleManager.swift (5)
performAttachOperation(373-375)performAttachOperation(377-379)performDetachOperation(436-438)performDetachOperation(440-442)waitToBeAbleToPerformPresenceOperations(586-624)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (3)
performAttachOperation(25-31)performDetachOperation(33-39)waitToBeAbleToPerformPresenceOperations(60-69)Tests/AblyChatTests/Helpers/Helpers.swift (2)
hasCodeAndStatusCode(8-24)createArbitraryError(160-162)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
failure(71-73)Sources/AblyChat/DefaultPresence.swift (3)
update(111-113)update(115-117)update(120-137)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Example app, tvOS (Xcode 26.0)
- GitHub Check: Example app, iOS (Xcode 26.0)
- GitHub Check: Xcode,
releaseconfiguration, iOS (Xcode 26.0) - GitHub Check: Xcode,
releaseconfiguration, tvOS (Xcode 26.0) - GitHub Check: Example app, macOS (Xcode 26.0)
- GitHub Check: Xcode, iOS (Xcode 26.0)
- GitHub Check: Xcode,
releaseconfiguration, macOS (Xcode 26.0) - GitHub Check: Xcode, tvOS (Xcode 26.0)
- GitHub Check: Xcode, macOS (Xcode 26.0)
- GitHub Check: SPM,
releaseconfiguration (Xcode 26.0) - GitHub Check: SPM (Xcode 26.0)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
Sources/AblyChat/DefaultTyping.swift (1)
147-174: Consider adding explicit error type todoblock for clarity.The error handling is correct, but per coding guidelines, explicitly declaring the error type on
doblocks improves clarity and consistency.As per coding guidelines.
Apply this diff:
- do { + do throws(ErrorInfo) { try await keyboardOperationQueue.enqueue { [weak self] () throws(ErrorInfo) inTests/AblyChatTests/ChatAPITests.swift (2)
18-29: Consider following the coding guideline for typed throws in test macros.According to the coding guidelines: "For Swift Testing #expect(throws:) with typed errors, move typed-throw code into a separate non-typed-throw function until Xcode 16.3+". The current implementation directly uses typed throws within the
#expect(throws:)closure.Additionally, line 24 uses optional chaining (
thrownError?.source) which may be unnecessary depending on whether#expect(throws:)returns an optional or not.As per coding guidelines.
Consider refactoring to:
func sendMessageHelper() async throws -> Message { try await chatAPI.sendMessage(roomName: roomName, params: .init(text: "hello", headers: [:])) } let thrownError = await #expect(throws: ErrorInfo.self) { try await sendMessageHelper() }
180-193: Inconsistent use of #expect vs #require for error testing.Line 18 uses
#expect(throws:)while line 187 uses#require(throws:)for similar error testing scenarios. Consider using#require(throws:)consistently when the thrown error must be captured for further assertions, as it more clearly communicates the intent and avoids potential optional unwrapping issues.Apply this pattern consistently:
let thrownError = await #require(throws: ErrorInfo.self) { try await chatAPI.sendMessage(roomName: roomName, params: .init(text: "hello", headers: [:])) }Sources/AblyChat/DefaultPresence.swift (1)
18-37: Consider simplifying the catch-log-rethrow pattern.The explicit
do-catchblocks that only log and re-throw add observability but also verbosity. Each presence operation has two such blocks. While this works correctly, you might consider:
- Extracting common error logging logic into a helper function
- Using a single catch at the call site if the distinction between lifecycle and channel errors isn't needed
Example:
internal func get() async throws(ErrorInfo) -> [PresenceMember] { logger.log(message: "Getting presence", level: .debug) try await roomLifecycleManager.waitToBeAbleToPerformPresenceOperations(requestedByFeature: .presence) let members = try await channel.presence.get() return try processPresenceGet(members: members) }Then handle error logging at a higher level or via Swift's new error logging features if needed.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (40)
CONTRIBUTING.md(1 hunks)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift(19 hunks)Sources/AblyChat/ChatAPI.swift(10 hunks)Sources/AblyChat/DefaultConnection.swift(2 hunks)Sources/AblyChat/DefaultMessageReactions.swift(2 hunks)Sources/AblyChat/DefaultMessages.swift(4 hunks)Sources/AblyChat/DefaultOccupancy.swift(1 hunks)Sources/AblyChat/DefaultPresence.swift(5 hunks)Sources/AblyChat/DefaultRoomReactions.swift(1 hunks)Sources/AblyChat/DefaultTyping.swift(4 hunks)Sources/AblyChat/ErrorInfo.swift(5 hunks)Sources/AblyChat/Errors.swift(0 hunks)Sources/AblyChat/Headers.swift(2 hunks)Sources/AblyChat/InternalError.swift(1 hunks)Sources/AblyChat/JSONCodable.swift(22 hunks)Sources/AblyChat/Message.swift(6 hunks)Sources/AblyChat/MessageReaction+JSON.swift(5 hunks)Sources/AblyChat/Occupancy.swift(1 hunks)Sources/AblyChat/PaginatedResult.swift(2 hunks)Sources/AblyChat/Room.swift(3 hunks)Sources/AblyChat/RoomLifecycleManager.swift(14 hunks)Sources/AblyChat/RoomReactionDTO.swift(2 hunks)Sources/AblyChat/Rooms.swift(5 hunks)Sources/AblyChat/Subscription.swift(1 hunks)Tests/AblyChatTests/ChatAPITests.swift(2 hunks)Tests/AblyChatTests/DefaultMessageReactionsTests.swift(4 hunks)Tests/AblyChatTests/DefaultMessagesTests.swift(13 hunks)Tests/AblyChatTests/DefaultPresenceTests.swift(12 hunks)Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift(28 hunks)Tests/AblyChatTests/DefaultRoomTests.swift(4 hunks)Tests/AblyChatTests/DefaultRoomsTests.swift(3 hunks)Tests/AblyChatTests/ErrorInfoTests.swift(1 hunks)Tests/AblyChatTests/Helpers/Helpers.swift(2 hunks)Tests/AblyChatTests/Mocks/MockConnection.swift(1 hunks)Tests/AblyChatTests/Mocks/MockRealtime.swift(2 hunks)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift(7 hunks)Tests/AblyChatTests/Mocks/MockRealtimePresence.swift(1 hunks)Tests/AblyChatTests/Mocks/MockRoomFactory.swift(1 hunks)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift(2 hunks)Tests/AblyChatTests/RoomReactionDTOTests.swift(2 hunks)
💤 Files with no reviewable changes (1)
- Sources/AblyChat/Errors.swift
🧰 Additional context used
📓 Path-based instructions (4)
**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.swift: Use protocol-based design; expose SDK functionality via protocols and prefer associated types with opaque return types (some Protocol) instead of existentials (any Protocol)
Isolate all mutable state to the main actor; mark stateful objects with @mainactor
Public API must use typed throws with ErrorInfo; use InternalError internally and convert at the public API boundary
For public structs emitted by the API, provide an explicit public memberwise initializer
When using AsyncSequence operators in @mainactor contexts, mark operator closures as @sendable
Task, CheckedContinuation, and AsyncThrowingStream do not support typed errors; use Result and call .get() to surface typed errors
Do not use Dictionary.mapValues for typed throws; use ablyChat_mapValuesWithTypedThrow instead
When the compiler struggles with typed throws, explicitly declare the error type on do blocks (e.g., do throws(InternalError))
Specify error types in closures using the throws(ErrorType) syntax (e.g., try items.map { jsonValue throws(InternalError) in ... })
Mark any test-only APIs with testsOnly_ prefix and wrap them in #if DEBUG
Files:
Sources/AblyChat/DefaultOccupancy.swiftSources/AblyChat/JSONCodable.swiftSources/AblyChat/Subscription.swiftSources/AblyChat/Occupancy.swiftTests/AblyChatTests/ChatAPITests.swiftSources/AblyChat/RoomReactionDTO.swiftTests/AblyChatTests/DefaultMessageReactionsTests.swiftTests/AblyChatTests/Mocks/MockRoomFactory.swiftTests/AblyChatTests/Mocks/MockConnection.swiftTests/AblyChatTests/RoomReactionDTOTests.swiftSources/AblyChat/DefaultPresence.swiftTests/AblyChatTests/DefaultRoomTests.swiftTests/AblyChatTests/DefaultPresenceTests.swiftSources/AblyChat/MessageReaction+JSON.swiftTests/AblyChatTests/DefaultRoomsTests.swiftTests/AblyChatTests/Mocks/MockRealtimePresence.swiftTests/AblyChatTests/Mocks/MockRealtime.swiftTests/AblyChatTests/DefaultMessagesTests.swiftSources/AblyChat/DefaultConnection.swiftSources/AblyChat/RoomLifecycleManager.swiftSources/AblyChat/Headers.swiftSources/AblyChat/DefaultRoomReactions.swiftSources/AblyChat/Room.swiftSources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swiftSources/AblyChat/ErrorInfo.swiftTests/AblyChatTests/ErrorInfoTests.swiftSources/AblyChat/ChatAPI.swiftSources/AblyChat/DefaultMessages.swiftSources/AblyChat/PaginatedResult.swiftSources/AblyChat/Message.swiftSources/AblyChat/DefaultMessageReactions.swiftTests/AblyChatTests/Helpers/Helpers.swiftSources/AblyChat/Rooms.swiftTests/AblyChatTests/DefaultRoomLifecycleManagerTests.swiftSources/AblyChat/InternalError.swiftTests/AblyChatTests/Mocks/MockRealtimeChannel.swiftTests/AblyChatTests/Mocks/MockRoomLifecycleManager.swiftSources/AblyChat/DefaultTyping.swift
Tests/AblyChatTests/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Tests/AblyChatTests/**/*.swift: Use test attribution tags to reference spec coverage: @SPEC, @specOneOf(m/n), @specPartial, @specUntested, @specNotApplicable
For Swift Testing #expect(throws:) with typed errors, move typed-throw code into a separate non-typed-throw function until Xcode 16.3+
Files:
Tests/AblyChatTests/ChatAPITests.swiftTests/AblyChatTests/DefaultMessageReactionsTests.swiftTests/AblyChatTests/Mocks/MockRoomFactory.swiftTests/AblyChatTests/Mocks/MockConnection.swiftTests/AblyChatTests/RoomReactionDTOTests.swiftTests/AblyChatTests/DefaultRoomTests.swiftTests/AblyChatTests/DefaultPresenceTests.swiftTests/AblyChatTests/DefaultRoomsTests.swiftTests/AblyChatTests/Mocks/MockRealtimePresence.swiftTests/AblyChatTests/Mocks/MockRealtime.swiftTests/AblyChatTests/DefaultMessagesTests.swiftTests/AblyChatTests/ErrorInfoTests.swiftTests/AblyChatTests/Helpers/Helpers.swiftTests/AblyChatTests/DefaultRoomLifecycleManagerTests.swiftTests/AblyChatTests/Mocks/MockRealtimeChannel.swiftTests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift
Tests/AblyChatTests/**
📄 CodeRabbit inference engine (CLAUDE.md)
Place tests under Tests/AblyChatTests/ as the root for test targets
Files:
Tests/AblyChatTests/ChatAPITests.swiftTests/AblyChatTests/DefaultMessageReactionsTests.swiftTests/AblyChatTests/Mocks/MockRoomFactory.swiftTests/AblyChatTests/Mocks/MockConnection.swiftTests/AblyChatTests/RoomReactionDTOTests.swiftTests/AblyChatTests/DefaultRoomTests.swiftTests/AblyChatTests/DefaultPresenceTests.swiftTests/AblyChatTests/DefaultRoomsTests.swiftTests/AblyChatTests/Mocks/MockRealtimePresence.swiftTests/AblyChatTests/Mocks/MockRealtime.swiftTests/AblyChatTests/DefaultMessagesTests.swiftTests/AblyChatTests/ErrorInfoTests.swiftTests/AblyChatTests/Helpers/Helpers.swiftTests/AblyChatTests/DefaultRoomLifecycleManagerTests.swiftTests/AblyChatTests/Mocks/MockRealtimeChannel.swiftTests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift
Tests/AblyChatTests/Mocks/**/*.swift
📄 CodeRabbit inference engine (CLAUDE.md)
Put mock implementations under Tests/AblyChatTests/Mocks/
Files:
Tests/AblyChatTests/Mocks/MockRoomFactory.swiftTests/AblyChatTests/Mocks/MockConnection.swiftTests/AblyChatTests/Mocks/MockRealtimePresence.swiftTests/AblyChatTests/Mocks/MockRealtime.swiftTests/AblyChatTests/Mocks/MockRealtimeChannel.swiftTests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift
🧬 Code graph analysis (33)
Sources/AblyChat/DefaultOccupancy.swift (2)
Sources/AblyChat/Logging.swift (3)
log(71-74)log(103-110)log(117-119)Sources/AblyChat/ChatAPI.swift (1)
getOccupancy(134-137)
Sources/AblyChat/JSONCodable.swift (1)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)
Sources/AblyChat/Subscription.swift (3)
Example/AblyChatExample/Mocks/MockSubscriptionStorage.swift (1)
historyBeforeSubscribe(211-213)Sources/AblyChat/Messages.swift (2)
historyBeforeSubscribe(447-449)toHistoryParams(333-335)Sources/AblyChat/ChatAPI.swift (1)
getMessages(31-34)
Tests/AblyChatTests/ChatAPITests.swift (2)
Sources/AblyChat/ChatAPI.swift (2)
sendMessage(63-78)getMessages(31-34)Tests/AblyChatTests/Helpers/Helpers.swift (1)
createArbitraryError(160-162)
Sources/AblyChat/RoomReactionDTO.swift (2)
Sources/AblyChat/JSONCodable.swift (1)
optionalObjectValueForKey(73-87)Sources/AblyChat/Extensions/Dictionary+Extensions.swift (1)
ablyChat_mapValuesWithTypedThrow(3-7)
Tests/AblyChatTests/DefaultMessageReactionsTests.swift (1)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)
Tests/AblyChatTests/Mocks/MockRoomFactory.swift (2)
Sources/AblyChat/Rooms.swift (1)
createRoom(298-303)Sources/AblyChat/Room.swift (1)
createRoom(251-260)
Tests/AblyChatTests/Mocks/MockConnection.swift (2)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (3)
on(187-191)on(399-403)on(405-409)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (2)
on(166-169)on(171-177)
Tests/AblyChatTests/RoomReactionDTOTests.swift (1)
Tests/AblyChatTests/Helpers/Helpers.swift (1)
hasInternalErrorCase(45-51)
Sources/AblyChat/DefaultPresence.swift (4)
Sources/AblyChat/RoomLifecycleManager.swift (1)
waitToBeAbleToPerformPresenceOperations(586-624)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (1)
waitToBeAbleToPerformPresenceOperations(60-69)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (6)
get(225-241)get(243-259)get(447-450)enter(277-291)update(293-307)leave(261-275)Tests/AblyChatTests/Mocks/MockRealtimePresence.swift (5)
get(19-25)get(27-33)enter(42-47)update(49-54)leave(35-40)
Tests/AblyChatTests/DefaultRoomTests.swift (5)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (2)
attach(351-365)detach(367-381)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (2)
attach(94-102)detach(108-116)Sources/AblyChat/Room.swift (2)
attach(377-379)detach(382-384)Tests/AblyChatTests/Mocks/MockRoom.swift (2)
attach(47-49)detach(51-53)Example/AblyChatExample/Mocks/MockClients.swift (2)
attach(85-87)detach(89-91)
Tests/AblyChatTests/DefaultPresenceTests.swift (5)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (5)
enter(277-291)update(293-307)get(225-241)get(243-259)get(447-450)Sources/AblyChat/DefaultPresence.swift (8)
enter(83-85)enter(87-89)enter(92-109)update(111-113)update(115-117)update(120-137)get(18-37)get(39-58)Tests/AblyChatTests/Mocks/MockRealtimePresence.swift (4)
enter(42-47)update(49-54)get(19-25)get(27-33)Tests/AblyChatTests/Helpers/Helpers.swift (1)
hasCodeAndStatusCode(8-24)
Tests/AblyChatTests/DefaultRoomsTests.swift (2)
Sources/AblyChat/Rooms.swift (2)
get(60-63)get(164-269)Tests/AblyChatTests/Helpers/Helpers.swift (1)
hasCodeAndStatusCode(8-24)
Tests/AblyChatTests/Mocks/MockRealtimePresence.swift (2)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (6)
get(225-241)get(243-259)get(447-450)leave(261-275)enter(277-291)update(293-307)Sources/AblyChat/DefaultPresence.swift (11)
get(18-37)get(39-58)leave(139-141)leave(143-145)leave(148-165)enter(83-85)enter(87-89)enter(92-109)update(111-113)update(115-117)update(120-137)
Tests/AblyChatTests/Mocks/MockRealtime.swift (3)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (1)
request(148-169)Tests/AblyChatTests/Mocks/MockAblyCocoaRealtime.swift (1)
request(44-46)Tests/AblyChatTests/Helpers/Helpers.swift (1)
addRecord(112-116)
Tests/AblyChatTests/DefaultMessagesTests.swift (3)
Tests/AblyChatTests/Helpers/Helpers.swift (1)
createArbitraryError(160-162)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
emitEvent(191-195)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (1)
update(293-307)
Sources/AblyChat/RoomLifecycleManager.swift (4)
Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (3)
performAttachOperation(25-31)performDetachOperation(33-39)waitToBeAbleToPerformPresenceOperations(60-69)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (4)
update(293-307)get(225-241)get(243-259)get(447-450)Sources/AblyChat/DefaultPresence.swift (5)
update(111-113)update(115-117)update(120-137)get(18-37)get(39-58)Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)
Sources/AblyChat/Headers.swift (1)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)
Sources/AblyChat/DefaultRoomReactions.swift (2)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (1)
publish(383-397)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
publish(208-210)
Sources/AblyChat/Room.swift (3)
Sources/AblyChat/Rooms.swift (1)
createRoom(298-303)Tests/AblyChatTests/Mocks/MockRoomFactory.swift (1)
createRoom(16-25)Sources/AblyChat/RoomLifecycleManager.swift (4)
performAttachOperation(373-375)performAttachOperation(377-379)performDetachOperation(436-438)performDetachOperation(440-442)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (5)
Tests/AblyChatTests/Mocks/MockRealtime.swift (1)
request(29-45)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (9)
attach(94-102)detach(108-116)on(166-169)on(171-177)once(179-185)once(187-189)unsubscribe(160-162)publish(208-210)get(80-87)Sources/AblyChat/Room.swift (2)
attach(377-379)detach(382-384)Tests/AblyChatTests/Mocks/MockConnection.swift (1)
on(14-16)Tests/AblyChatTests/Mocks/MockRealtimePresence.swift (6)
unsubscribe(15-17)get(19-25)get(27-33)enter(42-47)leave(35-40)update(49-54)
Tests/AblyChatTests/ErrorInfoTests.swift (1)
Tests/AblyChatTests/Helpers/Helpers.swift (1)
hasCodeAndStatusCode(8-24)
Sources/AblyChat/ChatAPI.swift (4)
Sources/AblyChat/JSONCodable.swift (1)
stringValueForKey(130-140)Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (1)
request(148-169)Tests/AblyChatTests/Mocks/MockRealtime.swift (1)
request(29-45)
Sources/AblyChat/DefaultMessages.swift (3)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/ChatAPI.swift (5)
getMessages(31-34)sendMessage(63-78)updateMessage(82-113)deleteMessage(117-132)getMessage(37-40)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (4)
update(293-307)get(225-241)get(243-259)get(447-450)
Sources/AblyChat/PaginatedResult.swift (2)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (3)
get(225-241)get(243-259)get(447-450)
Sources/AblyChat/Message.swift (3)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/JSONCodable.swift (1)
objectValueForKey(58-68)Sources/AblyChat/Extensions/Dictionary+Extensions.swift (1)
ablyChat_mapValuesWithTypedThrow(3-7)
Sources/AblyChat/DefaultMessageReactions.swift (3)
Sources/AblyChat/ChatAPI.swift (3)
sendReactionToMessage(140-155)deleteReactionFromMessage(158-172)getClientReactions(175-186)Sources/AblyChat/Logging.swift (3)
log(71-74)log(103-110)log(117-119)Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)
Sources/AblyChat/Rooms.swift (2)
Sources/AblyChat/InternalError.swift (2)
toErrorInfo(25-27)toErrorInfo(278-280)Sources/AblyChat/Room.swift (1)
createRoom(251-260)
Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift (4)
Sources/AblyChat/RoomLifecycleManager.swift (5)
performAttachOperation(373-375)performAttachOperation(377-379)performDetachOperation(436-438)performDetachOperation(440-442)waitToBeAbleToPerformPresenceOperations(586-624)Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (3)
performAttachOperation(25-31)performDetachOperation(33-39)waitToBeAbleToPerformPresenceOperations(60-69)Tests/AblyChatTests/Helpers/Helpers.swift (2)
hasCodeAndStatusCode(8-24)createArbitraryError(160-162)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (1)
failure(71-73)
Sources/AblyChat/InternalError.swift (5)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (2)
attach(351-365)detach(367-381)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (2)
attach(94-102)detach(108-116)Sources/AblyChat/Room.swift (2)
attach(377-379)detach(382-384)Tests/AblyChatTests/Mocks/MockRoom.swift (2)
attach(47-49)detach(51-53)Example/AblyChatExample/Mocks/MockClients.swift (2)
attach(85-87)detach(89-91)
Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (2)
Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (10)
get(225-241)get(243-259)get(447-450)attach(351-365)detach(367-381)on(187-191)on(399-403)on(405-409)once(411-415)once(417-421)Tests/AblyChatTests/Mocks/MockChannels.swift (1)
get(13-21)
Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (2)
Sources/AblyChat/RoomLifecycleManager.swift (5)
performAttachOperation(373-375)performAttachOperation(377-379)performDetachOperation(436-438)performDetachOperation(440-442)waitToBeAbleToPerformPresenceOperations(586-624)Sources/AblyChat/DefaultPresence.swift (2)
get(18-37)get(39-58)
Sources/AblyChat/DefaultTyping.swift (1)
Sources/AblyChat/TypingOperationQueue.swift (1)
enqueue(65-83)
🪛 LanguageTool
CONTRIBUTING.md
[style] ~49-~49: Consider a more concise word here.
Context: ...errors should be of type ErrorInfo. - In order to throw a Chat-specific error (i.e. if yo...
(IN_ORDER_TO_PREMIUM)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
- GitHub Check: Xcode,
releaseconfiguration, macOS (Xcode 26.0) - GitHub Check: Example app, tvOS (Xcode 26.0)
- GitHub Check: Xcode,
releaseconfiguration, tvOS (Xcode 26.0) - GitHub Check: Xcode, iOS (Xcode 26.0)
- GitHub Check: Xcode,
releaseconfiguration, iOS (Xcode 26.0) - GitHub Check: Example app, macOS (Xcode 26.0)
- GitHub Check: Example app, iOS (Xcode 26.0)
- GitHub Check: Xcode, macOS (Xcode 26.0)
- GitHub Check: Xcode, tvOS (Xcode 26.0)
- GitHub Check: SPM (Xcode 26.0)
- GitHub Check: check-documentation
🔇 Additional comments (76)
Sources/AblyChat/DefaultTyping.swift (3)
18-18: LGTM: Operation queue error type updated correctly.The operation queue now uses
ErrorInfoas its generic error type, consistent with the PR's goal to unify error handling throughout the codebase.
114-129: LGTM: Keystroke method correctly updated to use ErrorInfo.The method signature and closure error type declarations properly follow the coding guidelines by explicitly specifying
throws(ErrorInfo). Error propagation frompublishStartedEvent()is handled correctly.
131-131: LGTM: Method signature correctly updated.The signature change from
throws(InternalError)tothrows(ErrorInfo)is correct and aligns with the updated channel protocol signatures.Sources/AblyChat/DefaultConnection.swift (2)
13-15: LGTM! Simplified error sourcing.The direct access to
realtime.connection.errorReasonis cleaner than constructing the error, and aligns with the ErrorInfo migration across the codebase.
36-42: LGTM! Consistent error handling.The direct assignment of
stateChange.reasonto the error field is consistent with the ErrorInfo migration pattern.CONTRIBUTING.md (2)
46-49: LGTM! Clear guidance on error handling.The updated documentation correctly explains the pattern of creating
InternalErrorand converting toErrorInfofor public APIs, which aligns with the coding guidelines and the implementation.
56-57: LGTM! Examples updated correctly.The typed-throws examples now correctly demonstrate using
ErrorInfoinstead ofInternalError, which matches the new public API error surface.Sources/AblyChat/MessageReaction+JSON.swift (2)
16-23: LGTM! Consistent error conversion.The changes from
toInternalError()totoErrorInfo()are correct and consistent across all three reaction type decodings (unique, distinct, multiple). This aligns with the new error handling model whereJSONValueDecodingErroris converted toErrorInfofor propagation.Also applies to: 33-40, 50-57
73-77: LGTM! Function signatures updated correctly.The initializer signatures now throw
ErrorInfoinstead ofInternalError, which is appropriate for these internal JSON decoding operations and aligns with the error handling migration.Also applies to: 89-95
Sources/AblyChat/ErrorInfo.swift (4)
7-24: LGTM! Well-structured Source enum.The
Sourceenum clearly represents the three origins ofErrorInfoproperties: ably-cocoa errors, internal errors, and direct initialization. The naming is clear and the structure is appropriate.
26-35: LGTM! Correct property mappings.The
codeandstatusCodeproperties correctly map fromInternalError.codeAndStatusCode, using.rawValuefor the error code enum. The implementation properly handles all three source cases.Also applies to: 70-79
37-46: LGTM! Appropriate nil handling.The
hrefandrequestIDproperties correctly returnnilfor internal errors (which don't provide these fields) while properly accessing them from ably-cocoa errors.Also applies to: 81-90
170-182: LGTM! Enhanced error descriptions.The description for internal errors now includes the full
InternalErrorrepresentation, which aids debugging while maintaining a consistent format with other error sources.Sources/AblyChat/InternalError.swift (5)
3-36: LGTM! Well-designed internal error representation.The
InternalErrorenum provides a comprehensive set of error cases with clear naming. The design correctly prevents direct throwing by not conforming toError, ensuring onlyErrorInfocan be thrown publicly. TheOtherenum provides a flexible container for miscellaneous internal errors.
39-123: LGTM! Type-safe error code design.The nested enum structure for
ErrorCodeelegantly distinguishes between errors with fixed status codes and those with variable status codes. This design provides compile-time safety and makes the status code logic explicit. The error codes and status codes align with the spec as noted in the comments.
128-187: LGTM! Complete and correct error code mapping.The
codeAndStatusCodecomputed property exhaustively maps allInternalErrorcases to their appropriate error codes and status codes. The spec references (e.g., CHA-RL9c, CHA-PR3h) ensure correctness, and the exhaustive switch ensures maintainability.
211-267: LGTM! Clear and comprehensive error messages.The error messages are well-crafted and informative. The
descriptionOfFeaturehelper provides user-friendly feature names. Note that.channelFailedToAttachincludes the cause in both the message string (line 241) and thecauseproperty (lines 252-253), which ensures clarity in user-facing messages while also providing structured access to the underlying error.
270-311: LGTM! Clean conversion protocol.The
ConvertibleToInternalErrorprotocol with its conveniencetoErrorInfo()method provides a clean pathway for converting various internal error types toErrorInfo. The conformances consistently wrap errors inInternalError.other, maintaining a clear error hierarchy.Sources/AblyChat/JSONCodable.swift (1)
9-10: LGTM: Correct migration to ErrorInfo for internal protocols.The protocol signatures and default implementations correctly use
throws(ErrorInfo), aligning with the PR's goal of standardizing onErrorInfoas the only thrown error type. The pattern of calling.toErrorInfo()onJSONValueDecodingErrorvalues before throwing is consistent throughout.Also applies to: 26-27, 38-44
Sources/AblyChat/DefaultOccupancy.swift (1)
61-64: LGTM: Appropriate simplification of error propagation.Removing the do-catch block is correct since it wasn't performing any error transformation. With
chatAPI.getOccupancy(roomName:)now throwingErrorInfoand the method signature declaringthrows(ErrorInfo), errors propagate automatically.Sources/AblyChat/RoomReactionDTO.swift (1)
43-46: LGTM: Correct typed throws implementation.The init methods correctly declare
throws(ErrorInfo), and line 63 properly uses the typed throw syntax in the closure passed toablyChat_mapValuesWithTypedThrow. The explicitthrows(ErrorInfo)annotation ensures type safety as per the coding guidelines.Also applies to: 62-66
Tests/AblyChatTests/Mocks/MockRealtime.swift (1)
11-11: LGTM: Mock updated to match ErrorInfo migration.The migration from
ARTErrorInfotoErrorInfois correct and consistent with the production code changes. Removing the error conversion logic is appropriate sincepaginatedCallbacknow throwsErrorInfodirectly.Also applies to: 22-22, 29-45
Sources/AblyChat/DefaultRoomReactions.swift (1)
18-28: LGTM: Correct simplification aligning with ErrorInfo migration.Removing the do-catch block is appropriate since it was only performing error conversion. With both
RoomReactionDTO.init(name:metadata:headers:)andchannel.publish(_:data:extras:)now throwingErrorInfo, and the method signature declaringthrows(ErrorInfo), errors propagate automatically through the call chain.Tests/AblyChatTests/DefaultRoomTests.swift (2)
154-177: LGTM! Error type migration in attach tests is correct.The test properly migrates from
InternalErrortoErrorInfoin the test arguments and removes the now-unnecessarymapErrortransformation. The direct comparison on line 175 is cleaner than the previous approach.
185-208: LGTM! Error type migration in detach tests is correct.Consistent with the attach test changes, this properly migrates to
ErrorInfoand simplifies the assertion by removing themapErrortransformation.Sources/AblyChat/Occupancy.swift (1)
125-130: LGTM! Internal error type updated correctly.The initializer signature properly migrates from
InternalErrortoErrorInfo, consistent with the PR's unified error handling approach.Tests/AblyChatTests/Mocks/MockRoomFactory.swift (1)
16-25: LGTM! Mock interface updated to match real implementation.The signature change from
throws(InternalError)tothrows(ErrorInfo)correctly aligns the mock with the updatedRoomFactoryprotocol.Sources/AblyChat/Headers.swift (1)
86-99: LGTM! Error conversion updated correctly.The changes properly migrate the error handling from
InternalErrortoErrorInfo, using the appropriatetoErrorInfo()conversion method on line 97.Tests/AblyChatTests/ChatAPITests.swift (1)
187-193: Test correctly validates ErrorInfo propagation.The refactored test properly verifies that the
ErrorInfothrown by the mock is propagated through thechatAPI.getMessagescall without transformation.Tests/AblyChatTests/DefaultMessageReactionsTests.swift (2)
78-78: LGTM: Consistent error construction pattern.The updated error construction using
InternalError.other(...).toErrorInfo()correctly converts internal errors to the public ErrorInfo type at the test boundary.Also applies to: 98-98
501-517: LGTM: Error type and code updated correctly.The mock now throws
ErrorInfodirectly with code40400(Ably's error code format embedding the HTTP status). The assertion correctly validates this new error code.Tests/AblyChatTests/ErrorInfoTests.swift (1)
53-57: LGTM: Error type and assertion updates.The enum case rename from
internallyThrowntootherand the use ofhasCodeAndStatusCodealign with the PR's unified ErrorInfo approach.Sources/AblyChat/PaginatedResult.swift (2)
32-55: LGTM: Error handling updated to ErrorInfo.The error handling flow correctly migrates from
InternalErrortoErrorInfo, usingErrorInfo(ablyCocoaError:)for Ably Cocoa errors and.toErrorInfo()for internal error conversions.
77-92: LGTM: Control flow simplification.Removing the do-catch blocks is appropriate since
handleResponsealready converts all errors toErrorInfo, making the extra wrapping unnecessary with typed throws.Tests/AblyChatTests/Mocks/MockConnection.swift (1)
7-16: LGTM: Mock types updated for ErrorInfo surface.The mock correctly adopts
ErrorInfoand the wrapper typeConnectionStateChange, maintaining consistency with the internal protocol changes.Sources/AblyChat/ChatAPI.swift (3)
31-40: LGTM: Public API methods updated to typed throws(ErrorInfo).All public-facing async methods correctly adopt
throws(ErrorInfo), fulfilling the coding guideline that "Public API must use typed throws with ErrorInfo."As per coding guidelines.
Also applies to: 63-78, 82-113, 117-132, 134-137
140-155: LGTM: Guard statements properly convert errors.The guard statements correctly throw
ErrorInfousing.toErrorInfo()conversion, ensuring the internalChatErroris converted at the public API boundary.As per coding guidelines.
Also applies to: 158-172
196-217: LGTM: Internal helpers consistently use ErrorInfo.The internal request helpers correctly adopt
throws(ErrorInfo)throughout, including closure type annotations likethrows(ErrorInfo) in, maintaining consistency with the unified error surface.Also applies to: 219-229
Tests/AblyChatTests/DefaultRoomsTests.swift (2)
177-181: LGTM: Test expectations updated for ErrorInfo.The test assertions correctly expect
ErrorInfo.selfand usehasCodeAndStatusCodeto verify specific error codes, aligning with the unified ErrorInfo surface.Also applies to: 218-221
365-373: LGTM: Error handling refined to ErrorInfo type.The error variable is appropriately typed as
ErrorInfo?with explicit casting, providing type-safe error verification in tests.Tests/AblyChatTests/DefaultMessagesTests.swift (2)
179-192: LGTM: Error handling updated to ErrorInfo.The test correctly uses
ErrorInfo.createArbitraryError()and typed throws, with proper assertions. This pattern is consistently applied throughout the test file.
340-340: LGTM: ChannelStateChange includes resumed parameter.All ChannelStateChange initializations correctly include the
resumedparameter, consistent with the wrapper type introduced in this PR.Also applies to: 381-381, 385-385, 424-424
Tests/AblyChatTests/DefaultPresenceTests.swift (1)
86-106: LGTM: Correct internal error conversion pattern.The test correctly creates an
InternalError, converts it toErrorInfoviatoErrorInfo(), and verifies the result includes the expected code, status, and cause. This aligns with the coding guideline to use InternalError internally and convert at the public API boundary.Based on coding guidelines.
Tests/AblyChatTests/Mocks/MockRoomLifecycleManager.swift (1)
6-68: LGTM: Mock updated to ErrorInfo-based signatures.All error-related interfaces in the mock have been correctly updated to use
ErrorInfo, matching the realRoomLifecycleManagerimplementation and aligning with the PR's objective to make ErrorInfo the only thrown error type.Sources/AblyChat/DefaultPresence.swift (1)
202-216: LGTM: processPresenceGet updated to throw ErrorInfo.The method signature correctly updated to
throws(ErrorInfo)and the map closure uses typed throws, aligning with the PR's objective to make ErrorInfo the standard error type.Sources/AblyChat/DefaultMessageReactions.swift (1)
20-34: LGTM: Simplified error propagation.These methods correctly allow errors to propagate naturally as
ErrorInfowithout explicit catch-log-rethrow blocks. This is cleaner than the pattern in DefaultPresence.swift and works well when error-specific logging isn't required at this level.The approach is appropriate here since:
- Success logging is sufficient for observability
- Error details are already logged at lower levels or caught by callers
- The code is more readable without nested do-catch blocks
Also applies to: 37-49, 142-151
Sources/AblyChat/Subscription.swift (1)
84-117: LGTM! Error type consistently updated to ErrorInfo.The
subscriptionStartSerialclosure signature has been correctly updated to throwErrorInfothroughout the struct (property, initializer parameter, and usage). The direct propagation inhistoryBeforeSubscribeis appropriate since no error conversion is needed.Sources/AblyChat/Rooms.swift (4)
89-124: LGTM! RoomMapEntry correctly migrated to ErrorInfo.The enum's associated types and the
waitForRoom()method have been properly updated to useErrorInfoinstead ofInternalError. The pattern matching adjustments are consistent with the updated case structure.
164-269: LGTM! Proper ErrorInfo conversion at public API boundary.The method correctly implements the pattern of creating
InternalErrorinternally (line 170-174) and converting toErrorInfoviatoErrorInfo()before throwing. All async task results and public method signatures properly useErrorInfo.As per coding guidelines.
272-287: LGTM! Helper functions consistently use ErrorInfo.The
makeCreationFailureFunctionshelper correctly usesErrorInfoin both thefailCreationclosure parameter and thethrowAnySignalledCreationFailurethrow signature.
298-357: LGTM! Consistent error conversion in release flow.The
createRoommethod signature correctly throwsErrorInfo. In the release method (line 339), the internal error is properly converted toErrorInfoviatoErrorInfo()before being passed to the failure handler.Tests/AblyChatTests/RoomReactionDTOTests.swift (2)
8-22: LGTM! Tests correctly updated to expect ErrorInfo.The test methods properly use
#require(throws: ErrorInfo.self)to capture the thrown error and then verify the wrapped internal error case usinghasInternalErrorCase(). This is the correct pattern for testing the ErrorInfo wrapper.
69-75: LGTM! Consistent error testing pattern.The
ExtrasTestsfollow the same correct pattern asDataTests, properly expectingErrorInfoand verifying the wrapped internal error case.Tests/AblyChatTests/Mocks/MockRealtimePresence.swift (1)
19-54: LGTM! Mock signatures correctly updated to throw ErrorInfo.All five presence methods have been updated to throw
ErrorInfoinstead ofInternalError, correctly matching the protocol changes shown in the relevant code snippets fromInternalAblyCocoaTypes.swift.Sources/AblyChat/Room.swift (2)
240-261: LGTM! Factory methods correctly throw ErrorInfo.The
RoomFactoryprotocol andDefaultRoomFactoryimplementation properly useErrorInfoin theircreateRoommethod signatures, consistent with the public API requirements.As per coding guidelines.
263-384: LGTM! Room initialization and lifecycle methods use ErrorInfo.The
DefaultRoominitializer correctly throwsErrorInfo, and theattach()/detach()methods properly propagateErrorInfofrom the lifecycle manager without any conversion needed.Sources/AblyChat/RoomLifecycleManager.swift (3)
7-8: LGTM! Protocol signatures correctly updated to typed throws with ErrorInfo.The protocol methods now consistently use
throws(ErrorInfo), aligning with the PR objective to makeErrorInfothe only error thrown in the codebase.Also applies to: 25-25
197-206: LGTM! Channel state event handling correctly updated.The signature change to accept
ChannelStateChange(which wrapsErrorInfo?instead ofARTErrorInfo?) is consistent with the unified error handling approach. The error propagation at line 205 correctly uses the wrappedErrorInfo?.
216-216: LGTM! Correct error conversion pattern.The pattern of creating
InternalErrorand converting via.toErrorInfo()at the boundary is correctly applied here and throughout the file.Sources/AblyChat/Message.swift (2)
170-197: LGTM! Message initialization correctly migrated to ErrorInfo.The
JSONObjectDecodableconformance now usesthrows(ErrorInfo)throughout, with consistent error conversion via.toErrorInfo()at line 181. The use ofablyChat_mapValuesWithTypedThrowat lines 190-192 correctly specifies the error type per the coding guidelines.
224-266: LGTM! Public extension methods correctly use ErrorInfo.The
with(_:)methods correctly throwErrorInfoand use.toErrorInfo()for error conversion at lines 227, 248, and 253. This aligns with the coding guideline that public API must use typed throws with ErrorInfo.Tests/AblyChatTests/DefaultRoomLifecycleManagerTests.swift (2)
51-51: LGTM! Test infrastructure correctly updated to use ErrorInfo.The test helpers and error creation are consistently updated:
createChannelhelper now acceptsErrorInfo?instead ofARTErrorInfo?- Error creation uses
ErrorInfo.createArbitraryError()throughout- All mock setup correctly uses the new error types
Also applies to: 227-227, 424-424
724-732: LGTM! Channel state change construction correctly updated.Tests now construct
ChannelStateChangeinstances withErrorInfo?for thereasonparameter. This correctly reflects the new wrapper type introduced in the implementation.Also applies to: 754-762
Tests/AblyChatTests/Helpers/Helpers.swift (2)
4-25: LGTM! Test helper correctly moved to ErrorInfo extension.The
hasCodeAndStatusCodehelper is now an extension method onErrorInfo, providing a cleaner API for test assertions. The implementation correctly checks for the.internalErrorsource case and validates the wrapped error's properties.
159-162: LGTM! Arbitrary error creation correctly produces ErrorInfo.The helper method now creates
ErrorInfodirectly instead ofInternalError, which is consistent with the PR objective of makingErrorInfothe primary error type throughout the codebase.Sources/AblyChat/AblyCocoaExtensions/InternalAblyCocoaTypes.swift (3)
24-24: LGTM! Protocol signatures correctly unified around ErrorInfo.All async throwing methods in the internal protocols now consistently use
throws(ErrorInfo), creating a uniform error handling surface across the Ably Cocoa adapters. TheerrorReasonproperties correctly exposeErrorInfo?instead of the underlying Ably Cocoa error type.Also applies to: 58-86
509-545: LGTM! Wrapper types correctly introduced for state changes.The new
ChannelStateChangeandConnectionStateChangestructs provide clean wrappers around Ably Cocoa's state change types, translatingARTErrorInfo?toErrorInfo?for thereasonfield. The initializers correctly use.init(optionalAblyCocoaError:)for error conversion. This is a key part of unifying error handling across the SDK.
187-191: LGTM! Callbacks correctly use wrapper types.The callback signatures are correctly updated to use
ChannelStateChangeandConnectionStateChangeinstead of the Ably Cocoa types. ThetoAblyCocoaCallbackwrapper correctly translates the events by constructing the wrapper types in the callback body.Also applies to: 399-421
Sources/AblyChat/DefaultMessages.swift (3)
98-108: LGTM! Subscription closure correctly updated to throw ErrorInfo.The
subscriptionStartSerialclosure signature changed fromthrows(InternalError)tothrows(ErrorInfo), with error conversion correctly applied at line 100 using.toErrorInfo().
118-137: LGTM! Public-facing methods correctly throw ErrorInfo.All methods (
history,send,update,delete,get) now consistently usethrows(ErrorInfo), delegating to theChatAPIwhich already uses this error type. This creates a clean, typed error boundary.
139-174: LGTM! Internal resolution method correctly updated.The
resolveSubscriptionStartmethod signature and implementation are correctly updated:
- Method throws
ErrorInfo(line 139)CheckedContinuationtype updated toResult<String, ErrorInfo>(line 148)- Error conversions via
.toErrorInfo()consistently applied (lines 162, 168)Tests/AblyChatTests/Mocks/MockRealtimeChannel.swift (5)
16-17: LGTM! Property types correctly updated.The properties now use the new wrapper types (
ChannelStateChangeandErrorInfo) that align with the updated internal protocol signatures.
27-48: LGTM! Initializer correctly updated.The initializer parameters and property assignments have been consistently updated to use the new types.
61-88: LGTM! Error handling types correctly updated.The
AttachOrDetachBehaviorandAttachOrDetachResultenums have been consistently updated to useErrorInfothroughout, with proper typed throws syntax.
94-136: LGTM! Method signatures correctly updated to throw ErrorInfo.The
attach(),detach(), andperformBehavior()methods now properly use typed throws withErrorInfo, matching the updated internal protocol signatures shown in the relevant code snippets.
164-195: LGTM! State subscription methods consistently updated.All state subscription callback signatures have been correctly updated to use
ChannelStateChangeinstead ofARTChannelStateChange, matching the updated internal protocol shown in the relevant code snippets.
This removes
InternalError's conformance toError, so that the only error thrown in the codebase is nowErrorInfo. It also does a few more error-related refactors. See commit messages.Summary by CodeRabbit
Release Notes