Skip to content

[ECO-5423] The rest of errors via chat error#341

Merged
maratal merged 1 commit intomainfrom
301-rest-of-errors-via-ChatError
Aug 18, 2025
Merged

[ECO-5423] The rest of errors via chat error#341
maratal merged 1 commit intomainfrom
301-rest-of-errors-via-ChatError

Conversation

@maratal
Copy link
Copy Markdown
Collaborator

@maratal maratal commented Jul 26, 2025

Closes #301

Related PR - #331

Summary by CodeRabbit

  • Bug Fixes

    • Improved stability and memory management for chat message, reaction, occupancy, and presence event handling, reducing the risk of crashes and retain cycles.
    • Presence events now gracefully handle missing fields by using sensible defaults instead of throwing errors.
  • Improvements

    • Enhanced error reporting for channel attachment issues, providing clearer and more specific error messages.
  • Tests

    • Added a new test to ensure invalid occupancy events emit zero values for connections and presence members.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Jul 26, 2025

Walkthrough

This update refactors subscription closures across several AblyChat components to weakly capture self, preventing retain cycles. Error handling is improved by introducing new ChatError cases and using them for more specific error reporting. Presence and occupancy event parsing is made more lenient, and a new test verifies handling of invalid occupancy events.

Changes

File(s) Change Summary
Sources/AblyChat/DefaultMessageReactions.swift, Sources/AblyChat/DefaultRoomReactions.swift Subscription closures now weakly capture self and safely unwrap it; removed unused error enum in DefaultRoomReactions.
Sources/AblyChat/DefaultMessages.swift Subscription closure captures self weakly; error handling refactored to use new ChatError cases for specificity.
Sources/AblyChat/DefaultOccupancy.swift Subscription closure now weakly captures self; occupancy event parsing is more lenient, defaults to zero values.
Sources/AblyChat/DefaultPresence.swift Removed guard checks for missing fields; now uses default values for missing clientId and timestamp.
Sources/AblyChat/Errors.swift Added new internal ChatError cases for channel attachment errors with localized descriptions and error code mappings.
Tests/AblyChatTests/DefaultRoomOccupancyTests.swift Added async test to verify zero-value emission for invalid occupancy events.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant DefaultOccupancy
    participant Channel

    Client->>DefaultOccupancy: subscribeToOccupancy()
    DefaultOccupancy->>Channel: subscribe(closure)
    Channel-->>DefaultOccupancy: ARTMessage (may be invalid)
    DefaultOccupancy->>DefaultOccupancy: Parse message (defaults to zero if invalid)
    DefaultOccupancy-->>Client: Emit occupancy event (zero or real values)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Possibly related PRs

Suggested reviewers

  • lawrence-forooghian
  • umair-ably

Poem

In the meadow of code, a rabbit did hop,
Weakly capturing self, so leaks finally stop.
Errors now clearer, with messages neat,
Presence and occupancy—defaults can't be beat!
With tests for the odd case, our chat feels robust,
This bunny approves—review it, you must!
🐇✨

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7b2c4e9 and f5c6ff5.

📒 Files selected for processing (7)
  • Sources/AblyChat/DefaultMessageReactions.swift (2 hunks)
  • Sources/AblyChat/DefaultMessages.swift (2 hunks)
  • Sources/AblyChat/DefaultOccupancy.swift (1 hunks)
  • Sources/AblyChat/DefaultPresence.swift (2 hunks)
  • Sources/AblyChat/DefaultRoomReactions.swift (1 hunks)
  • Sources/AblyChat/Errors.swift (5 hunks)
  • Tests/AblyChatTests/DefaultRoomOccupancyTests.swift (1 hunks)
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
Learnt from: maratal
PR: ably/ably-chat-swift#165
File: Sources/AblyChat/Reaction.swift:14-14
Timestamp: 2024-12-10T01:59:02.065Z
Learning: For the 'ably-chat-swift' repository, documentation-only PRs should focus exclusively on public methods and properties. Avoid suggesting changes to internal comments or private methods in such PRs.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/DefaultTyping.swift:131-138
Timestamp: 2025-05-12T21:04:36.263Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle closures should capture self weakly with `[weak self]` to avoid potential retain cycles, particularly when calling channel.unsubscribe() within the closure.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/RoomLifecycleManager.swift:611-624
Timestamp: 2025-05-22T20:57:48.147Z
Learning: In AblyChat Swift, when using continuations with subscription callbacks, the continuation object is captured directly by the closure and doesn't depend on `self`. Even with `[weak self]` capture, the continuation will still be resumed properly.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/DefaultOccupancy.swift:10-13
Timestamp: 2025-05-12T21:01:14.109Z
Learning: The `Occupancy` protocol in the Ably Chat Swift SDK is annotated with `@MainActor`, making all conforming types like `DefaultOccupancy` automatically main actor-isolated without requiring explicit annotations on their methods.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription continues to work even if the handle is not stored, as the underlying services maintain references to the callbacks until explicitly unsubscribed via the handle's unsubscribe() method.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription callbacks continue to work even when the handle is not stored, as the underlying services maintain their own storage of callbacks that remain registered until explicitly unsubscribed via the handle's unsubscribe() method.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/SubscriptionHandleStorage.swift:37-40
Timestamp: 2025-05-12T21:02:28.274Z
Learning: In the AblyChat Swift codebase, the `SubscriptionHandle` struct has an `unsubscribe()` method for terminating subscriptions, not a `cancel()` method.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/SubscriptionAsyncSequence.swift:36-41
Timestamp: 2025-06-29T16:02:43.988Z
Learning: In the AblyChat Swift codebase, the fatalError in SubscriptionAsyncSequence.swift's mock async sequence handling (around line 39) is intentional and should not be flagged. This fatalError catches programmer errors when a throwing AsyncSequence is incorrectly passed to the mock initializer, which is meant for testing/development purposes.
Learnt from: maratal
PR: ably/ably-chat-swift#286
File: Sources/AblyChat/RoomReactions.swift:42-56
Timestamp: 2025-05-16T21:35:46.634Z
Learning: In the AblyChat framework, all subscription callbacks are annotated with @MainActor, ensuring they always execute on the main actor thread without requiring additional Task wrappers.
Learnt from: maratal
PR: ably/ably-chat-swift#262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.
Learnt from: maratal
PR: ably/ably-chat-swift#262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.
Sources/AblyChat/DefaultMessageReactions.swift (15)

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultTyping.swift:131-138
Timestamp: 2025-05-12T21:04:36.263Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle closures should capture self weakly with [weak self] to avoid potential retain cycles, particularly when calling channel.unsubscribe() within the closure.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/RoomLifecycleManager.swift:611-624
Timestamp: 2025-05-22T20:57:48.147Z
Learning: In AblyChat Swift, when using continuations with subscription callbacks, the continuation object is captured directly by the closure and doesn't depend on self. Even with [weak self] capture, the continuation will still be resumed properly.

Learnt from: maratal
PR: #293
File: Sources/AblyChat/Message.swift:163-175
Timestamp: 2025-06-14T21:58:57.802Z
Learning: In the AblyChat Swift SDK, the JSON key "reactions" is used for parsing reaction summaries in the Chat SDK layer, while "summary" is used in the ably-cocoa layer. Reactions can be sourced from either the realtime message's summary property or directly from the "reactions" dictionary in the JSON.

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription callbacks continue to work even when the handle is not stored, as the underlying services maintain their own storage of callbacks that remain registered until explicitly unsubscribed via the handle's unsubscribe() method.

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription continues to work even if the handle is not stored, as the underlying services maintain references to the callbacks until explicitly unsubscribed via the handle's unsubscribe() method.

Learnt from: maratal
PR: #293
File: Example/AblyChatExample/MessageViews/MessageReactionsSheet.swift:15-31
Timestamp: 2025-06-14T16:18:47.038Z
Learning: In the AblyChat Swift SDK, there are three message reaction types: unique (one reaction per client per message), distinct (one reaction of each type per client per message), and multiple (any number of reactions including repeats, with counts). The MessageReactionsSheet specifically handles unique/distinct reactions where the count is always 1 per client per emoji. Only the "multiple" reaction type would have accumulating counts.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultPresence.swift:269-277
Timestamp: 2025-05-16T22:07:33.286Z
Learning: In Swift, capturing an instance method in a closure's capture list (like [someMethod]) implicitly captures self strongly, as instance methods are functions that take self as their first parameter. This can lead to retain cycles, and should be avoided by using [weak self] instead.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionAsyncSequence.swift:36-41
Timestamp: 2025-06-29T16:02:43.988Z
Learning: In the AblyChat Swift codebase, the fatalError in SubscriptionAsyncSequence.swift's mock async sequence handling (around line 39) is intentional and should not be flagged. This fatalError catches programmer errors when a throwing AsyncSequence is incorrectly passed to the mock initializer, which is meant for testing/development purposes.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionHandleStorage.swift:37-40
Timestamp: 2025-05-12T21:02:28.274Z
Learning: In the AblyChat Swift codebase, the SubscriptionHandle struct has an unsubscribe() method for terminating subscriptions, not a cancel() method.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultPresence.swift:269-277
Timestamp: 2025-05-16T22:07:33.286Z
Learning: In Swift, capturing an instance method directly in a closure's capture list (e.g., [someMethod]) implicitly captures self strongly because instance methods are functions that take self as their first parameter. This creates a potential retain cycle if the closure is stored by an object that self also holds a reference to. To avoid this, use [weak self] instead and call the method through the weakly captured self reference.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/RoomReactions.swift:42-56
Timestamp: 2025-05-16T21:35:46.634Z
Learning: In the AblyChat framework, all subscription callbacks are annotated with @mainactor, ensuring they always execute on the main actor thread without requiring additional Task wrappers.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultOccupancy.swift:66-69
Timestamp: 2025-05-12T21:02:25.928Z
Learning: SubscriptionHandle.unsubscribe is already annotated with @mainactor, so there's no need to wrap the code in MainActor.run when using it.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultOccupancy.swift:10-13
Timestamp: 2025-05-12T21:01:14.109Z
Learning: The Occupancy protocol in the Ably Chat Swift SDK is annotated with @MainActor, making all conforming types like DefaultOccupancy automatically main actor-isolated without requiring explicit annotations on their methods.

Sources/AblyChat/DefaultRoomReactions.swift (15)

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultTyping.swift:131-138
Timestamp: 2025-05-12T21:04:36.263Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle closures should capture self weakly with [weak self] to avoid potential retain cycles, particularly when calling channel.unsubscribe() within the closure.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/RoomLifecycleManager.swift:611-624
Timestamp: 2025-05-22T20:57:48.147Z
Learning: In AblyChat Swift, when using continuations with subscription callbacks, the continuation object is captured directly by the closure and doesn't depend on self. Even with [weak self] capture, the continuation will still be resumed properly.

Learnt from: maratal
PR: #293
File: Sources/AblyChat/Message.swift:163-175
Timestamp: 2025-06-14T21:58:57.802Z
Learning: In the AblyChat Swift SDK, the JSON key "reactions" is used for parsing reaction summaries in the Chat SDK layer, while "summary" is used in the ably-cocoa layer. Reactions can be sourced from either the realtime message's summary property or directly from the "reactions" dictionary in the JSON.

Learnt from: maratal
PR: #293
File: Example/AblyChatExample/MessageViews/MessageReactionsSheet.swift:15-31
Timestamp: 2025-06-14T16:18:47.038Z
Learning: In the AblyChat Swift SDK, there are three message reaction types: unique (one reaction per client per message), distinct (one reaction of each type per client per message), and multiple (any number of reactions including repeats, with counts). The MessageReactionsSheet specifically handles unique/distinct reactions where the count is always 1 per client per emoji. Only the "multiple" reaction type would have accumulating counts.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultPresence.swift:269-277
Timestamp: 2025-05-16T22:07:33.286Z
Learning: In Swift, capturing an instance method in a closure's capture list (like [someMethod]) implicitly captures self strongly, as instance methods are functions that take self as their first parameter. This can lead to retain cycles, and should be avoided by using [weak self] instead.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultPresence.swift:269-277
Timestamp: 2025-05-16T22:07:33.286Z
Learning: In Swift, capturing an instance method directly in a closure's capture list (e.g., [someMethod]) implicitly creates a strong reference to the instance (self) that the method belongs to, because Swift method references bind to their instance. This can create potential retain cycles if the closure outlives the instance. To avoid this, use [weak self] and call the method through the weak reference instead of capturing the method directly.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultPresence.swift:269-277
Timestamp: 2025-05-16T22:07:33.286Z
Learning: In Swift, capturing an instance method directly in a closure's capture list (e.g., [someMethod]) implicitly captures self strongly because instance methods are functions that take self as their first parameter. This creates a potential retain cycle if the closure is stored by an object that self also holds a reference to. To avoid this, use [weak self] instead and call the method through the weakly captured self reference.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultOccupancy.swift:10-13
Timestamp: 2025-05-12T21:01:14.109Z
Learning: The Occupancy protocol in the Ably Chat Swift SDK is annotated with @MainActor, making all conforming types like DefaultOccupancy automatically main actor-isolated without requiring explicit annotations on their methods.

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription callbacks continue to work even when the handle is not stored, as the underlying services maintain their own storage of callbacks that remain registered until explicitly unsubscribed via the handle's unsubscribe() method.

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription continues to work even if the handle is not stored, as the underlying services maintain references to the callbacks until explicitly unsubscribed via the handle's unsubscribe() method.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionHandleStorage.swift:37-40
Timestamp: 2025-05-12T21:02:28.274Z
Learning: In the AblyChat Swift codebase, the SubscriptionHandle struct has an unsubscribe() method for terminating subscriptions, not a cancel() method.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionAsyncSequence.swift:36-41
Timestamp: 2025-06-29T16:02:43.988Z
Learning: In the AblyChat Swift codebase, the fatalError in SubscriptionAsyncSequence.swift's mock async sequence handling (around line 39) is intentional and should not be flagged. This fatalError catches programmer errors when a throwing AsyncSequence is incorrectly passed to the mock initializer, which is meant for testing/development purposes.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:20:38.904Z
Learning: In Swift tests using mock objects like MockRealtimeChannel, explicit subscription cleanup (via unsubscribe() or similar methods) is generally unnecessary since the mocks don't maintain real connections and will be garbage collected after the test completes.

Sources/AblyChat/DefaultOccupancy.swift (13)

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultOccupancy.swift:10-13
Timestamp: 2025-05-12T21:01:14.109Z
Learning: The Occupancy protocol in the Ably Chat Swift SDK is annotated with @MainActor, making all conforming types like DefaultOccupancy automatically main actor-isolated without requiring explicit annotations on their methods.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultTyping.swift:131-138
Timestamp: 2025-05-12T21:04:36.263Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle closures should capture self weakly with [weak self] to avoid potential retain cycles, particularly when calling channel.unsubscribe() within the closure.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/RoomLifecycleManager.swift:611-624
Timestamp: 2025-05-22T20:57:48.147Z
Learning: In AblyChat Swift, when using continuations with subscription callbacks, the continuation object is captured directly by the closure and doesn't depend on self. Even with [weak self] capture, the continuation will still be resumed properly.

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription callbacks continue to work even when the handle is not stored, as the underlying services maintain their own storage of callbacks that remain registered until explicitly unsubscribed via the handle's unsubscribe() method.

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription continues to work even if the handle is not stored, as the underlying services maintain references to the callbacks until explicitly unsubscribed via the handle's unsubscribe() method.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionAsyncSequence.swift:36-41
Timestamp: 2025-06-29T16:02:43.988Z
Learning: In the AblyChat Swift codebase, the fatalError in SubscriptionAsyncSequence.swift's mock async sequence handling (around line 39) is intentional and should not be flagged. This fatalError catches programmer errors when a throwing AsyncSequence is incorrectly passed to the mock initializer, which is meant for testing/development purposes.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionHandleStorage.swift:37-40
Timestamp: 2025-05-12T21:02:28.274Z
Learning: In the AblyChat Swift codebase, the SubscriptionHandle struct has an unsubscribe() method for terminating subscriptions, not a cancel() method.

Learnt from: maratal
PR: #165
File: docs-coverage-report:5-8
Timestamp: 2024-12-10T01:59:12.404Z
Learning: In the AblyChat Swift project, documentation coverage should focus on public methods and properties. Private and internal entities do not require documentation.

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/Mocks/MockClients.swift:0-0
Timestamp: 2025-05-16T21:04:26.244Z
Learning: In the ably-chat-swift project, mock implementations (like those in MockClients.swift) are intentionally kept simple, sometimes omitting parameter-based filtering behavior for testing simplicity.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultPresence.swift:269-277
Timestamp: 2025-05-16T22:07:33.286Z
Learning: In Swift, capturing an instance method in a closure's capture list (like [someMethod]) implicitly captures self strongly, as instance methods are functions that take self as their first parameter. This can lead to retain cycles, and should be avoided by using [weak self] instead.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:20:38.904Z
Learning: In Swift tests using mock objects like MockRealtimeChannel, explicit subscription cleanup (via unsubscribe() or similar methods) is generally unnecessary since the mocks don't maintain real connections and will be garbage collected after the test completes.

Sources/AblyChat/DefaultPresence.swift (12)

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/Mocks/MockClients.swift:0-0
Timestamp: 2025-05-16T21:04:26.244Z
Learning: In the ably-chat-swift project, mock implementations (like those in MockClients.swift) are intentionally kept simple, sometimes omitting parameter-based filtering behavior for testing simplicity.

Learnt from: maratal
PR: #249
File: Tests/AblyChatTests/DefaultMessagesTests.swift:45-48
Timestamp: 2025-05-22T19:17:21.392Z
Learning: The let doIt = {} closures in DefaultMessagesTests.swift are temporary workarounds for compiler crashes (related to issue #233) and will be removed once Xcode 16.3 is released, so they should not be flagged for missing async/throws annotations.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultOccupancy.swift:10-13
Timestamp: 2025-05-12T21:01:14.109Z
Learning: The Occupancy protocol in the Ably Chat Swift SDK is annotated with @MainActor, making all conforming types like DefaultOccupancy automatically main actor-isolated without requiring explicit annotations on their methods.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultTyping.swift:131-138
Timestamp: 2025-05-12T21:04:36.263Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle closures should capture self weakly with [weak self] to avoid potential retain cycles, particularly when calling channel.unsubscribe() within the closure.

Learnt from: maratal
PR: #165
File: Sources/AblyChat/Reaction.swift:14-14
Timestamp: 2024-12-10T01:59:02.065Z
Learning: For the 'ably-chat-swift' repository, documentation-only PRs should focus exclusively on public methods and properties. Avoid suggesting changes to internal comments or private methods in such PRs.

Learnt from: maratal
PR: #293
File: Example/AblyChatExample/ContentView.swift:94-99
Timestamp: 2025-06-14T21:47:20.509Z
Learning: In the Ably Chat Swift example app, using fatalError for missing clientID is intentional and appropriate since it represents a programmer error rather than a recoverable runtime condition. The clientID is considered an essential configuration requirement.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultTyping.swift:11-14
Timestamp: 2025-05-12T21:03:31.914Z
Learning: The Typing protocol in the Ably Chat Swift SDK is annotated with @MainActor, making all conforming types like DefaultTyping automatically main actor-isolated without requiring explicit annotations on their methods.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultTyping.swift:11-14
Timestamp: 2025-05-12T21:03:31.914Z
Learning: The Typing protocol in the Ably Chat Swift SDK is annotated with @MainActor, making all conforming types like DefaultTyping automatically main actor-isolated without requiring explicit annotations on their methods.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/RoomLifecycleManager.swift:611-624
Timestamp: 2025-05-22T20:57:48.147Z
Learning: In AblyChat Swift, when using continuations with subscription callbacks, the continuation object is captured directly by the closure and doesn't depend on self. Even with [weak self] capture, the continuation will still be resumed properly.

Learnt from: maratal
PR: #293
File: Tests/AblyChatTests/Mocks/MockRealtime.swift:41-47
Timestamp: 2025-06-14T15:18:17.427Z
Learning: In the MockRealtime class in Tests/AblyChatTests/Mocks/MockRealtime.swift, the body parameter in the request method is constrained to dictionary or array types, but currently only dictionary types are used. Arrays are not yet used for this method, so the current implementation assumes dictionary-only for simplicity.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Sources/AblyChat/DefaultMessages.swift (11)

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultTyping.swift:131-138
Timestamp: 2025-05-12T21:04:36.263Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle closures should capture self weakly with [weak self] to avoid potential retain cycles, particularly when calling channel.unsubscribe() within the closure.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/RoomLifecycleManager.swift:611-624
Timestamp: 2025-05-22T20:57:48.147Z
Learning: In AblyChat Swift, when using continuations with subscription callbacks, the continuation object is captured directly by the closure and doesn't depend on self. Even with [weak self] capture, the continuation will still be resumed properly.

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription continues to work even if the handle is not stored, as the underlying services maintain references to the callbacks until explicitly unsubscribed via the handle's unsubscribe() method.

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/ContentView.swift:203-209
Timestamp: 2025-05-12T21:11:08.937Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle doesn't need to be retained/stored to keep subscriptions active. The subscription callbacks continue to work even when the handle is not stored, as the underlying services maintain their own storage of callbacks that remain registered until explicitly unsubscribed via the handle's unsubscribe() method.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionAsyncSequence.swift:36-41
Timestamp: 2025-06-29T16:02:43.988Z
Learning: In the AblyChat Swift codebase, the fatalError in SubscriptionAsyncSequence.swift's mock async sequence handling (around line 39) is intentional and should not be flagged. This fatalError catches programmer errors when a throwing AsyncSequence is incorrectly passed to the mock initializer, which is meant for testing/development purposes.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionHandleStorage.swift:37-40
Timestamp: 2025-05-12T21:02:28.274Z
Learning: In the AblyChat Swift codebase, the SubscriptionHandle struct has an unsubscribe() method for terminating subscriptions, not a cancel() method.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #249
File: Tests/AblyChatTests/DefaultMessagesTests.swift:45-48
Timestamp: 2025-05-22T19:17:21.392Z
Learning: The let doIt = {} closures in DefaultMessagesTests.swift are temporary workarounds for compiler crashes (related to issue #233) and will be removed once Xcode 16.3 is released, so they should not be flagged for missing async/throws annotations.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/RoomReactions.swift:42-56
Timestamp: 2025-05-16T21:35:46.634Z
Learning: In the AblyChat framework, all subscription callbacks are annotated with @mainactor, ensuring they always execute on the main actor thread without requiring additional Task wrappers.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:20:38.904Z
Learning: In Swift tests using mock objects like MockRealtimeChannel, explicit subscription cleanup (via unsubscribe() or similar methods) is generally unnecessary since the mocks don't maintain real connections and will be garbage collected after the test completes.

Sources/AblyChat/Errors.swift (5)

Learnt from: maratal
PR: #293
File: Example/AblyChatExample/ContentView.swift:94-99
Timestamp: 2025-06-14T21:47:20.509Z
Learning: In the Ably Chat Swift example app, using fatalError for missing clientID is intentional and appropriate since it represents a programmer error rather than a recoverable runtime condition. The clientID is considered an essential configuration requirement.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionAsyncSequence.swift:36-41
Timestamp: 2025-06-29T16:02:43.988Z
Learning: In the AblyChat Swift codebase, the fatalError in SubscriptionAsyncSequence.swift's mock async sequence handling (around line 39) is intentional and should not be flagged. This fatalError catches programmer errors when a throwing AsyncSequence is incorrectly passed to the mock initializer, which is meant for testing/development purposes.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultTyping.swift:131-138
Timestamp: 2025-05-12T21:04:36.263Z
Learning: In the AblyChat Swift codebase, SubscriptionHandle closures should capture self weakly with [weak self] to avoid potential retain cycles, particularly when calling channel.unsubscribe() within the closure.

Learnt from: maratal
PR: #165
File: docs-coverage-report:5-8
Timestamp: 2024-12-10T01:59:12.404Z
Learning: In the AblyChat Swift project, documentation coverage should focus on public methods and properties. Private and internal entities do not require documentation.

Learnt from: maratal
PR: #165
File: Sources/AblyChat/Reaction.swift:14-14
Timestamp: 2024-12-10T01:59:02.065Z
Learning: For the 'ably-chat-swift' repository, documentation-only PRs should focus exclusively on public methods and properties. Avoid suggesting changes to internal comments or private methods in such PRs.

Tests/AblyChatTests/DefaultRoomOccupancyTests.swift (10)

Learnt from: maratal
PR: #286
File: Sources/AblyChat/DefaultOccupancy.swift:10-13
Timestamp: 2025-05-12T21:01:14.109Z
Learning: The Occupancy protocol in the Ably Chat Swift SDK is annotated with @MainActor, making all conforming types like DefaultOccupancy automatically main actor-isolated without requiring explicit annotations on their methods.

Learnt from: maratal
PR: #293
File: Example/AblyChatExample/Mocks/MockRealtime.swift:348-384
Timestamp: 2025-06-14T16:19:29.327Z
Learning: The MockRealtime class in Example/AblyChatExample/Mocks/MockRealtime.swift is specifically for example app construction, not for testing. The actual test mocks are located in the Tests directory. If example app mocks are unused, fatalError calls are appropriate to catch accidental usage.

Learnt from: maratal
PR: #249
File: Tests/AblyChatTests/DefaultMessagesTests.swift:45-48
Timestamp: 2025-05-22T19:17:21.392Z
Learning: The let doIt = {} closures in DefaultMessagesTests.swift are temporary workarounds for compiler crashes (related to issue #233) and will be removed once Xcode 16.3 is released, so they should not be flagged for missing async/throws annotations.

Learnt from: maratal
PR: #286
File: Sources/AblyChat/SubscriptionAsyncSequence.swift:36-41
Timestamp: 2025-06-29T16:02:43.988Z
Learning: In the AblyChat Swift codebase, the fatalError in SubscriptionAsyncSequence.swift's mock async sequence handling (around line 39) is intentional and should not be flagged. This fatalError catches programmer errors when a throwing AsyncSequence is incorrectly passed to the mock initializer, which is meant for testing/development purposes.

Learnt from: maratal
PR: #293
File: Tests/AblyChatTests/Mocks/MockRealtime.swift:41-47
Timestamp: 2025-06-14T15:18:17.427Z
Learning: In the MockRealtime class in Tests/AblyChatTests/Mocks/MockRealtime.swift, the body parameter in the request method is constrained to dictionary or array types, but currently only dictionary types are used. Arrays are not yet used for this method, so the current implementation assumes dictionary-only for simplicity.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:20:38.904Z
Learning: In Swift tests using mock objects like MockRealtimeChannel, explicit subscription cleanup (via unsubscribe() or similar methods) is generally unnecessary since the mocks don't maintain real connections and will be garbage collected after the test completes.

Learnt from: maratal
PR: #286
File: Example/AblyChatExample/Mocks/MockClients.swift:0-0
Timestamp: 2025-05-16T21:04:26.244Z
Learning: In the ably-chat-swift project, mock implementations (like those in MockClients.swift) are intentionally kept simple, sometimes omitting parameter-based filtering behavior for testing simplicity.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: In the ably-chat-swift project, MockRealtimeChannel processes messages synchronously when subscribe is called, immediately delivering the configured messages to the callback.

Learnt from: maratal
PR: #262
File: Tests/AblyChatTests/DefaultRoomReactionsTests.swift:0-0
Timestamp: 2025-05-23T16:29:39.712Z
Learning: The ably-chat-swift project uses Swift Testing framework (#expect macros) rather than XCTest for test assertions.

⏰ 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). (31)
  • GitHub Check: Xcode, macOS (Xcode 16.3)
  • GitHub Check: Xcode, iOS (Xcode 16.2)
  • GitHub Check: Example app, tvOS (Xcode 16.2)
  • GitHub Check: Xcode, iOS (Xcode 16.3)
  • GitHub Check: Xcode, iOS (Xcode 16.1)
  • GitHub Check: Example app, iOS (Xcode 16.2)
  • GitHub Check: Xcode, macOS (Xcode 16.2)
  • GitHub Check: Xcode, tvOS (Xcode 16.2)
  • GitHub Check: Example app, iOS (Xcode 16.3)
  • GitHub Check: Example app, tvOS (Xcode 16.1)
  • GitHub Check: Xcode, tvOS (Xcode 16.3)
  • GitHub Check: Example app, macOS (Xcode 16.3)
  • GitHub Check: Example app, iOS (Xcode 16.1)
  • GitHub Check: Xcode, release configuration, tvOS (Xcode 16.3)
  • GitHub Check: Xcode, release configuration, tvOS (Xcode 16.1)
  • GitHub Check: Xcode, release configuration, tvOS (Xcode 16.2)
  • GitHub Check: Xcode, release configuration, iOS (Xcode 16.3)
  • GitHub Check: Xcode, release configuration, macOS (Xcode 16.1)
  • GitHub Check: Xcode, release configuration, iOS (Xcode 16.2)
  • GitHub Check: SPM, release configuration (Xcode 16.2)
  • GitHub Check: Xcode, tvOS (Xcode 16.1)
  • GitHub Check: Xcode, release configuration, macOS (Xcode 16.3)
  • GitHub Check: Xcode, release configuration, iOS (Xcode 16.1)
  • GitHub Check: Xcode, macOS (Xcode 16.1)
  • GitHub Check: SPM, release configuration (Xcode 16.3)
  • GitHub Check: SPM (Xcode 16.1)
  • GitHub Check: Xcode, release configuration, macOS (Xcode 16.2)
  • GitHub Check: SPM (Xcode 16.3)
  • GitHub Check: SPM (Xcode 16.2)
  • GitHub Check: SPM, release configuration (Xcode 16.1)
  • GitHub Check: Generate code coverage
🔇 Additional comments (16)
Sources/AblyChat/Errors.swift (4)

178-179: LGTM: Well-defined error cases added

The new error cases follow the existing pattern and provide better semantic meaning than hardcoded error messages. The naming is descriptive and the associated value for channelFailedToAttach appropriately captures the underlying cause.


214-217: LGTM: Appropriate status code mappings

Both new error cases correctly map to .badRequest status code, which is appropriate for client-side channel state issues.


274-277: LGTM: Clear and informative error descriptions

The localized descriptions provide clear context for debugging. Using String(describing: cause) appropriately handles the optional ARTErrorInfo value.


288-289: LGTM: Proper cause property implementation

The cause property correctly handles the new error cases - returning the associated cause for channelFailedToAttach and nil for attachSerialIsNotDefined.

Also applies to: 301-302

Sources/AblyChat/DefaultRoomReactions.swift (1)

59-62: LGTM: Proper weak self capture pattern implemented

This change correctly implements the established pattern for subscription closures in the AblyChat codebase. The weak capture of self prevents retain cycles, and the guard statement safely handles the case where self has been deallocated.

Sources/AblyChat/DefaultMessageReactions.swift (2)

67-70: LGTM: Consistent weak self capture pattern

The subscription closure correctly implements weak self capture with proper guard unwrapping, preventing potential retain cycles.


108-111: LGTM: Consistent pattern applied across subscription methods

Both subscription methods now consistently use weak self capture, ensuring uniform memory safety across the class.

Sources/AblyChat/DefaultOccupancy.swift (3)

48-51: LGTM: Proper weak self capture implemented

Consistent application of the established weak self capture pattern for subscription closures.


54-57: LGTM: More resilient occupancy data parsing

The change from strict parsing to more lenient handling improves resilience to malformed occupancy messages while still extracting useful data when available.


59-60: LGTM: Good traceability with requirement references

The inline comments referencing CHA-O4g improve code traceability to specification requirements.

Sources/AblyChat/DefaultMessages.swift (3)

82-85: LGTM: Consistent weak self capture pattern

The subscription closure correctly implements the established weak self capture pattern, maintaining consistency across the codebase.


204-204: LGTM: Improved error handling with semantic error case

The use of ARTErrorInfo(chatError: .attachSerialIsNotDefined) provides better semantics and maintainability compared to hardcoded error construction.


208-210: LGTM: Clean error handling with cause propagation

The use of ARTErrorInfo(chatError: .channelFailedToAttach(cause:)) properly captures the underlying error cause and provides cleaner, more maintainable error handling.

Sources/AblyChat/DefaultPresence.swift (2)

310-313: Good resilient approach to handle missing presence data.

The nil-coalescing operators provide sensible fallbacks for missing clientId and timestamp fields, making presence parsing more fault-tolerant. This aligns well with the spec requirements (CHA-M4k1, CHA-M4k3) and prevents failures when optional fields are missing.


324-327: Consistent fallback handling matches the get operation.

The nil-coalescing operators here mirror the same resilient approach used in processPresenceGet, ensuring consistent behavior when handling missing presence data across both get and subscription operations.

Tests/AblyChatTests/DefaultRoomOccupancyTests.swift (1)

70-100: Well-structured test for invalid occupancy event handling.

This test properly validates the fault-tolerant behavior when invalid occupancy data is received. The test setup with an empty ARTMessage effectively simulates invalid data, and the assertions correctly verify that zero values are emitted for both connections and presence members, which aligns with the CHA-O4g specification.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 301-rest-of-errors-via-ChatError

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@maratal maratal changed the base branch from main to 337-message-validation-for-all-features July 26, 2025 12:55
@maratal maratal requested a review from umair-ably July 26, 2025 12:55
@maratal maratal changed the title The rest of errors via chat error [ECO-5423] The rest of errors via chat error Jul 26, 2025
Base automatically changed from 337-message-validation-for-all-features to main August 18, 2025 11:27
@maratal maratal merged commit 98015c2 into main Aug 18, 2025
41 checks passed
@maratal maratal deleted the 301-rest-of-errors-via-ChatError branch August 18, 2025 11:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

Make all thrown errors go via ChatError

2 participants