fix: make queueMessages client option True by default

[ttypic]

Make queueMessages client option True by default

• Add TO3g test verifying queueMessages defaults to true
• Add RTL6c2 check to fail immediately when queueMessages is false and connection is CONNECTING/DISCONNECTED
• Add test for publish failure on CONNECTING state with queueMessages=false

Summary by CodeRabbit

• Changed Behavior

  • The queue_messages option now defaults to True, so messages are queued during connection transitions by default.
  • Publishing behavior around the INITIALIZED/connecting/disconnected states was tightened: messages are queued by default and may be rejected when queuing is explicitly disabled.

• Tests

  • Added tests verifying the default queue_messages value and that publishing fails in INITIALIZED when queue_messages is false.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Refactors realtime send logic to gate on a new state_should_queue set (includes INITIALIZED, DISCONNECTED, CONNECTING), changes Options.default queue_messages from False to True, and adds tests asserting the default and publish behavior when queueing is disabled.

Changes

Cohort / File(s)	Summary
Connection manager logic ably/realtime/connectionmanager.py	Replaces previous send gating with a state_should_queue approach (INITIALIZED, DISCONNECTED, CONNECTING); enforces guard that disallows sending when not CONNECTED and not queueing; enqueues messages when appropriate and returns early for queued/non-queued branches; preserves ack handling via msgSerial and pending queue.
Default options ably/types/options.py	Changes Options.__init__ default for queue_messages from False to True.
Realtime channel checks ably/realtime/realtime_channel.py	Adds ConnectionState.INITIALIZED to allowed publishable connection states in _throw_if_unpublishable_state.
Tests test/ably/realtime/realtimechannel_publish_test.py, test/ably/realtime/realtimeconnection_test.py	Adds tests: one asserting publish fails when queue_messages=False while connection is INITIALIZED/CONNECTING (note: duplicate test definition present), and one async test asserting queue_messages defaults to True on the client and connection manager.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Review state_should_queue membership and intent (INITIALIZED, DISCONNECTED, CONNECTING) to ensure it matches specification.
• Inspect new guards that raise when sending is disallowed and the exact error codes/messages produced.
• Verify enqueue/early-return behavior for ack-required vs non-ack messages and that pending-queue handling (msgSerial, futures) remains correct.
• Check tests for duplication and correctness (two identical test definitions added).

Poem

🐰 I hopped through states both new and true,
Queues now open by default, who knew?
When INITIALIZED we patiently queue,
Awaiting a CONNECTED sky of blue,
hop hop — a tidy change, fresh as dew 🥕

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 72.73% which is insufficient. The required threshold is 80.00%.	You can run @coderabbitai generate docstrings to improve docstring coverage.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the main change: making the queueMessages option default to True, which is the primary modification across all affected files.
Linked Issues check	✅ Passed	The PR addresses all stated objectives: queueMessages defaults to True, TO3g test added, RTL6c2 check implemented, and test for publish failure when queueMessages=false added.
Out of Scope Changes check	✅ Passed	All changes align with PR objectives: default option change, connection state validation logic, and test coverage directly support the queueMessages feature.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch AIT-96/queue-message-fix

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 2b791d9 and 534864e.

📒 Files selected for processing (4)

• ably/realtime/connectionmanager.py (1 hunks)
• ably/types/options.py (1 hunks)
• test/ably/realtime/realtimechannel_publish_test.py (1 hunks)
• test/ably/realtime/realtimeconnection_test.py (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

• test/ably/realtime/realtimeconnection_test.py
• ably/types/options.py

🧰 Additional context used

🧬 Code graph analysis (2)

ably/realtime/connectionmanager.py (3)

ably/realtime/connection.py (2)

• state (101-103)
• state (112-113)

ably/types/connectionstate.py (1)

• ConnectionState (8-16)

ably/types/options.py (2)

• queue_messages (169-170)
• queue_messages (173-174)

test/ably/realtime/realtimechannel_publish_test.py (7)

ably/realtime/connectionmanager.py (2)

• ably (738-739)
• state (742-743)

ably/types/options.py (5)

• use_binary_protocol (161-162)
• use_binary_protocol (165-166)
• queue_messages (169-170)
• queue_messages (173-174)
• auto_connect (250-251)

ably/realtime/realtime.py (1)

• connection (132-134)

ably/util/eventemitter.py (1)

• once_async (169-182)

ably/types/connectionstate.py (1)

• ConnectionState (8-16)

ably/realtime/realtime_channel.py (3)

• state (691-693)
• state (696-697)
• publish (388-488)

ably/util/exceptions.py (1)

• AblyException (9-84)

⏰ 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). (7)

• GitHub Check: check (3.11)
• GitHub Check: check (3.8)
• GitHub Check: check (3.12)
• GitHub Check: check (3.10)
• GitHub Check: check (3.14)
• GitHub Check: check (3.13)
• GitHub Check: check (3.9)

🔇 Additional comments (2)

ably/realtime/connectionmanager.py (1)

185-215: LGTM! Well-structured implementation of RTL6c2 state gating.

The control flow correctly handles:

1. Non-connectable states (CLOSING, CLOSED, FAILED, SUSPENDED) fail immediately at line 188-189.
2. Queue-able states with queue_messages=False and ack-required messages fail per RTL6c2.
3. msgSerial is only assigned after the RTL6c2 check, preventing serial number waste on rejected messages.
4. The existing duplicate-prevention logic at line 220 in _send_protocol_message_on_connected_state handles the case where messages are both in pending_message_queue and queued_messages.

test/ably/realtime/realtimechannel_publish_test.py (1)

288-324: Good test coverage for RTL6c2 CONNECTING state behavior.

The test correctly:

• Disables auto-connect to control the connection lifecycle
• Blocks the connection at CONNECTING state using a no-op coroutine
• Verifies the expected error code (90000) and status code (400)
• Ensures proper cleanup with ably.close()

[coderabbitai]

Actionable comments posted: 1

♻️ Duplicate comments (1)

test/ably/realtime/realtimechannel_publish_test.py (1)

288-325: Make the error-message assertion case-insensitive to match ConnectionState string form.

The setup of the CONNECTING state with queue_messages=False looks good and exercises the new gate correctly. The assertion on the error message is fragile though:

assert 'CONNECTING' in str(exc_info.value)

ConnectionState’s value is lowercase ('connecting'), so the formatted exception string won’t contain the all‑caps 'CONNECTING', and this assertion will fail even when the behavior is correct. A case‑insensitive check is safer:

-        assert 'CONNECTING' in str(exc_info.value)
+        assert 'connecting' in str(exc_info.value).lower()

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 4194243 and 6bbade8.

📒 Files selected for processing (5)

• ably/realtime/connectionmanager.py (2 hunks)
• ably/types/options.py (1 hunks)
• test/ably/realtime/realtimechannel_publish_test.py (1 hunks)
• test/ably/realtime/realtimeconnection_test.py (1 hunks)
• test/ably/utils.py (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (3)

• test/ably/realtime/realtimeconnection_test.py
• ably/types/options.py
• test/ably/utils.py

🧰 Additional context used

🧬 Code graph analysis (1)

ably/realtime/connectionmanager.py (3)

ably/types/connectionstate.py (1)

• ConnectionState (8-16)

ably/util/exceptions.py (1)

• AblyException (9-84)

ably/types/options.py (2)

• queue_messages (169-170)
• queue_messages (173-174)

⏰ 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). (5)

• GitHub Check: check (3.14)
• GitHub Check: check (3.13)
• GitHub Check: check (3.10)
• GitHub Check: check (3.11)
• GitHub Check: check (3.12)

[coderabbitai]

Actionable comments posted: 0

🧹 Nitpick comments (1)

test/ably/realtime/realtimechannel_publish_test.py (1)

288-307: Align test docstring with the actual INITIALIZED scenario.

The test name and setup exercise the INITIALIZED state (auto_connect=False), but the docstring still says “connection is CONNECTING”. For clarity, consider updating the docstring (and/or comment) to say INITIALIZED so it matches the scenario under test.

📜 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.

📥 Commits

Reviewing files that changed from the base of the PR and between 542ab42 and e98c2a1.

📒 Files selected for processing (5)

• ably/realtime/connectionmanager.py (2 hunks)
• ably/realtime/realtime_channel.py (1 hunks)
• ably/types/options.py (1 hunks)
• test/ably/realtime/realtimechannel_publish_test.py (1 hunks)
• test/ably/realtime/realtimeconnection_test.py (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• test/ably/realtime/realtimeconnection_test.py

🧰 Additional context used

🧬 Code graph analysis (2)

ably/realtime/realtime_channel.py (1)

ably/types/connectionstate.py (1)

• ConnectionState (8-16)

test/ably/realtime/realtimechannel_publish_test.py (4)

ably/types/options.py (4)

• use_binary_protocol (161-162)
• use_binary_protocol (165-166)
• queue_messages (169-170)
• queue_messages (173-174)

ably/types/channelsubscription.py (1)

• channel (19-20)

ably/realtime/realtime_channel.py (2)

• get (742-767)
• publish (388-488)

ably/util/exceptions.py (1)

• AblyException (9-84)

⏰ 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). (7)

• GitHub Check: check (3.13)
• GitHub Check: check (3.11)
• GitHub Check: check (3.10)
• GitHub Check: check (3.8)
• GitHub Check: check (3.12)
• GitHub Check: check (3.14)
• GitHub Check: check (3.9)

🔇 Additional comments (3)

ably/types/options.py (1)

27-35: Defaulting queue_messages to True looks correct and localized.

Constructor now defaults queue_messages=True while the rest of the option plumbing is unchanged; this aligns with the new tests and connection manager behavior without introducing extra side effects in this file.

ably/realtime/realtime_channel.py (1)

498-505: Including INITIALIZED in allowed publish states is consistent with new queuing semantics.

Allowing publish in ConnectionState.INITIALIZED and deferring the actual queue/fail decision to ConnectionManager.send_protocol_message matches the new tests and RTL6c2 behavior, while still blocking clearly invalid states (CLOSING/CLOSED/FAILED/SUSPENDED).

ably/realtime/connectionmanager.py (1)

185-213: state_should_queue + RTL6c2 guard now match the intended semantics.

The new state_should_queue set (INITIALIZED, DISCONNECTED, CONNECTING) plus:

• rejecting sends when not CONNECTED and not in that set, and
• raising only when state_should_queue is true and queue_messages is false,

correctly implements “queue when allowed; fail immediately when queueing is disabled” and fixes the previously inverted condition. The queuing branch then enqueues PendingMessage and awaits its future for ACK-required messages, which is consistent with existing pending/queued handling and the new tests.

[owenpearson]

looks good!