[PUB-1030] LiveObjects behavior under different channel states

[VeskeR]

When the channel is SUSPENDED

• The local copy of the last known LiveObjects state is maintained, ensuring that the API for accessing objects continues to function and returns the latest known data.
• The getRoot() function will return the last known root object.
• Write API functions will throw an error, as the channel is in an invalid state (i.e., messages cannot be sent or queued while suspended).

When the channel is DETACHED or FAILED

• The local LiveObjects state copy is cleared (LiveObjects data update events are not emitted in this case (e.g., counter increments, map key set/delete), as the current state of LiveObjects data is unknown)
• All access and write API functions, including getRoot(), will throw an error indicating that the channel is in an invalid state.

Resolves PUB-1030

[coderabbitai]

Walkthrough

This pull request refactors error handling logic across several LiveObjects modules. The changes replace legacy state mode checks with new API configuration validations. Methods in classes such as BatchContext, BatchContextLiveCounter, BatchContextLiveMap, LiveCounter, LiveMap, LiveObject, and LiveObjects now invoke updated validation functions (e.g., throwIfInvalidAccessApiConfiguration and throwIfInvalidWriteApiConfiguration) with unchanged control flows. Additionally, a new private helper method is introduced in LiveObjects for channel state checks, and the corresponding tests are updated to accommodate the new error handling scheme.

Changes

File(s)	Change Summary
src/plugins/liveobjects/batchcontext.ts	Replaced throwIfMissingStateSubscribeMode with throwIfInvalidAccessApiConfiguration in getRoot.
src/plugins/liveobjects/batchcontextlivecounter.ts, src/plugins/liveobjects/batchcontextlivemap.ts	Updated error handling in batch context classes: read methods now use throwIfInvalidAccessApiConfiguration and write methods use throwIfInvalidWriteApiConfiguration.
src/plugins/liveobjects/livecounter.ts, src/plugins/liveobjects/livemap.ts	Modified validation methods in counter and map operations to replace subscription/publish mode checks with new API configuration validations.
src/plugins/liveobjects/liveobject.ts	Updated the subscribe method to use throwIfInvalidAccessApiConfiguration and revised comments for unsubscribe/off methods to clarify API configuration requirements.
src/plugins/liveobjects/liveobjects.ts	Renamed error validation methods, updated calls in various creation methods, and added a new private method (_throwIfInChannelState) for channel state checks.
test/realtime/live_objects.test.js	Refactored error expectation functions: replaced specific missing state mode functions with generalized functions (expectAccessApiToThrow, expectWriteApiToThrow, and expectAccessBatchApiToThrow) that accept an error message parameter.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant API as LiveObjects/LiveCounter/LiveMap/LiveObject
    participant Validator as Error Validator

    Client->>API: Invoke operation (e.g., getRoot, subscribe, increment)
    API->>Validator: Call new error validation method
    alt Configuration Valid
        Validator-->>API: Return success
        API->>API: Execute operation logic
        API-->>Client: Return result
    else Configuration Invalid
        Validator-->>API: Throw error
        API-->>Client: Error thrown
    end

Loading

Possibly related PRs

• [DTP-1147] Fix flaky live objects tests that relied on STATE/STATE_SYNC messages that client may not have received #1964: Modifies error handling for API configuration checks in similar methods.
• [DTP-1035] Add Batch write API for LiveObjects #1951: Updates error handling in BatchContext, BatchContextLiveCounter, and BatchContextLiveMap with comparable method replacements.
• [DTP-1034] Emit Live Objects lifecycle events #1958: Refactors error handling in the LiveObject class by replacing legacy subscription checks with new validations.

Suggested reviewers

• mschristensen
• zknill
• kaschula

Poem

I'm a bunny on the coding trail,
Hopping through changes without fail.
Error checks now shine so bright,
API configs set things right.
With a twitch of whiskers and joyful cheer,
I celebrate this refactor, hopping near!
🐰 Happy coding, my dear!

Warning

There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure.

🔧 ast-grep (0.31.1)

test/realtime/live_objects.test.js

---

📜 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 6fc1702 and 717ff16.

📒 Files selected for processing (8)

• src/plugins/liveobjects/batchcontext.ts (1 hunks)
• src/plugins/liveobjects/batchcontextlivecounter.ts (1 hunks)
• src/plugins/liveobjects/batchcontextlivemap.ts (2 hunks)
• src/plugins/liveobjects/livecounter.ts (3 hunks)
• src/plugins/liveobjects/livemap.ts (4 hunks)
• src/plugins/liveobjects/liveobject.ts (5 hunks)
• src/plugins/liveobjects/liveobjects.ts (10 hunks)
• test/realtime/live_objects.test.js (4 hunks)

🧰 Additional context used

🧠 Learnings (1)

src/plugins/liveobjects/livecounter.ts (1)

Learnt from: VeskeR
PR: ably/ably-js#1897
File: src/plugins/liveobjects/livecounter.ts:93-93
Timestamp: 2024-11-12T07:31:53.691Z
Learning: In the `LiveCounter` class's `_applyCounterCreate` method, it's intentional to increment the counter's value using `+=` instead of initializing it with `=` because the counter may have a pre-existing non-zero value.

⏰ Context from checks skipped due to timeout of 90000ms (6)

• GitHub Check: test-browser (webkit)
• GitHub Check: test-node (20.x)
• GitHub Check: test-browser (firefox)
• GitHub Check: test-node (18.x)
• GitHub Check: test-browser (chromium)
• GitHub Check: test-node (16.x)

🔇 Additional comments (40)

src/plugins/liveobjects/batchcontext.ts (1)

27-27: Consistent API check modification

The error handling has been updated from checking state mode to validating API configuration. This appears to be part of a broader refactoring across the LiveObjects modules to improve the handling of different channel states.

src/plugins/liveobjects/batchcontextlivemap.ts (4)

15-15: Read operation validation updated

Error handling updated from state subscribe mode check to API configuration validation for this read operation.

---

26-26: Size method validation updated

Consistent with other read operations, the error handling has been updated to use API configuration validation.

---

32-32: Write operation validation updated

Error handling for map write operation changed from state publish mode check to write API configuration validation.

---

39-39: Remove operation validation updated

Consistent with other write operations, the error handling has been updated to use write API configuration validation.

src/plugins/liveobjects/livecounter.ts (3)

126-126: Value access validation updated

Error handling updated from state subscribe mode check to API configuration validation for counter value access.

---

140-140: Increment operation validation updated

Error handling for counter increment operation changed from state publish mode check to write API configuration validation.

---

149-149: Decrement operation validation updated

Consistent with the increment operation, decrement now uses write API configuration validation instead of state publish mode check.

src/plugins/liveobjects/batchcontextlivecounter.ts (3)

18-18: Counter value access validation updated

Error handling updated from state subscribe mode check to API configuration validation for this read operation.

---

24-24: Batch increment validation updated

Consistent with other write operations, the error handling has been updated to use write API configuration validation.

---

31-31: Batch decrement validation updated

Error handling for counter decrement operation within a batch context changed from state publish mode check to write API configuration validation.

src/plugins/liveobjects/livemap.ts (4)

269-269: API validation refactoring looks good.

This change from throwIfMissingStateSubscribeMode() to throwIfInvalidAccessApiConfiguration() aligns with the broader refactoring to improve error handling across LiveObjects. The new method likely performs additional validations beyond just checking subscribe mode.

---

308-308: API validation refactoring looks good.

Similar to the get method, this change appropriately updates the validation call to use the new API configuration validation method.

---

344-344: Write operation validation updated appropriately.

This change from throwIfMissingStatePublishMode() to throwIfInvalidWriteApiConfiguration() ensures consistent validation checks before performing write operations.

---

359-359: Write operation validation updated appropriately.

The removal operation correctly uses the new validation method, maintaining consistency with other write operations.

src/plugins/liveobjects/liveobject.ts (6)

80-80: API validation refactoring looks good.

Updated to use the new validation method which likely performs additional checks beyond just subscription mode.

---

92-92: Improved comment clarity.

The comment now explicitly states that this public API method can be called without specific configuration, which clarifies usage and explains why no validation check is needed.

---

105-105: Improved comment clarity.

Similar to the previous comment update, this clarifies that unsubscribeAll doesn't require specific configuration checks.

---

110-110: Improved comment clarity.

This comment update maintains consistency with the other public API methods.

---

121-121: Improved comment clarity.

Consistent approach to documenting that this public method doesn't require configuration checks.

---

132-132: Improved comment clarity.

Final update for the offAll method, maintaining consistency with other public API methods.

src/plugins/liveobjects/liveobjects.ts (11)

74-74: API validation refactoring looks good.

Updated to use the new validation method in the getRoot method, aligning with the broader refactoring.

---

88-88: API validation refactoring looks good.

The batch method now uses the updated validation method for write operations.

---

110-110: API validation refactoring looks good.

The createMap method appropriately uses the updated validation method.

---

142-142: API validation refactoring looks good.

The createCounter method consistently uses the updated validation method.

---

165-165: Improved comment clarity.

The comment now explicitly states that this public API method can be called without specific configuration.

---

176-176: Improved comment clarity.

Consistent with other public methods, clarifying that no configuration check is needed.

---

187-187: Improved comment clarity.

Final update for the offAll method, maintaining consistency with other methods.

---

287-289: Simplified error handling for detached/failed states.

The handling of the 'detached' and 'failed' states has been streamlined by directly calling the reset methods, making the code more concise and readable.

---

322-325: Enhanced validation with channel state check.

The renamed method now includes validation for both channel mode and channel state, making error handling more robust. The use of a separate helper method for channel state validation is a good separation of concerns.

---

330-333: Enhanced validation with additional states for write operations.

Similar to the access configuration check, but includes 'suspended' state for write operations, which is appropriate as write operations should not proceed when the channel is suspended.

---

496-500: Well-designed helper method for channel state validation.

This private helper method encapsulates the logic for checking if the channel is in a specific state, enhancing code modularity and reusability. The implementation is clean and efficient, throwing an appropriate error when needed.

test/realtime/live_objects.test.js (8)

4150-4163: Well-structured refactoring of access API validation.

This new helper function expectAccessApiToThrow consolidates the error validation logic for access APIs, improving code maintainability and reducing duplication. The function properly tests various LiveObjects access methods against the provided error message while correctly exempting unsubscribe methods from throwing.

---

4165-4180: Clean implementation of write API validation.

The expectWriteApiToThrow function neatly separates write API validation from access API validation, creating a clear distinction between the two types of operations. The implementation correctly tests all write methods and maintains the same exemption for unsubscribe methods.

---

4183-4190: Good batch context access API validation.

This helper appropriately tests batch-specific access API methods, providing consistent error validation within batch contexts. The comment correctly notes that this should only be called inside batch methods.

---

4193-4199: Consistent implementation for batch write operations.

The expectWriteBatchApiToThrow function maintains the same pattern as the other validation helpers, focusing specifically on batch write operations. This consistency makes the code more readable and maintainable.

---

4201-4245: Comprehensive error testing for channel modes.

The first two scenarios in channelConfigurationScenarios thoroughly test error handling when required channel modes are missing. The tests cover both attached and pre-attached states, ensuring robust validation across different lifecycle states.

---

4247-4299: Well-designed tests for invalid channel states.

These scenarios test how the LiveObjects API behaves when the channel is in DETACHED or FAILED states. The tests appropriately check both access and write operations, ensuring complete coverage of the error handling logic.

---

4301-4324: Good SUSPENDED state validation.

This scenario specifically tests write API operations when the channel is in a SUSPENDED state, which is an important edge case to validate. The test properly focuses only on write operations, which is appropriate for this state.

---

4328-4335: Enhanced test setup with flexible modes configuration.

The test setup has been improved to allow dynamic modification of channel modes, which is crucial for testing the different error conditions. The comment about modifying the underlying modes array provides helpful context for future developers.

✨ Finishing Touches

• 📝 Generate Docstrings (Beta)

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

🪧 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.
  • Generate unit testing code for this file.
  • 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 generate unit testing code for this file.
  • @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 generate unit testing code.
  • @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.

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. (Beta)
• @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.

[mschristensen]

Nice, LGTM