[ECO-5700] fix: ensure channel state updates before connection state listeners notify (RTL3d1)

[ttypic]

Resolves #1201

• Adjusted enactState to update channel states prior to notifying connection state listeners.
• Added a test to validate channel state transitions during reconnection.

Summary by CodeRabbit

• Bug Fixes

  • Reordered connection state handling so channel state is correct during reconnection and terminal cleanup runs reliably.

• Tests

  • Added a test validating channel state immediately after reconnect.
  • Improved test diagnostics to capture stack traces for lingering timer threads, and adjusted test sequencing to better simulate send-blocking during reconnect.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ef782c1a-9731-4790-bcb7-6dd19357756a

📥 Commits

Reviewing files that changed from the base of the PR and between 2758570 and 056fc55.

📒 Files selected for processing (2)

• lib/src/main/java/io/ably/lib/transport/ConnectionManager.java
• lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java

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

• lib/src/main/java/io/ably/lib/transport/ConnectionManager.java
• lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java

---

Walkthrough

Reordered connection state processing so the state handler is enacted before notifying connection listeners; transport cleanup now occurs after enactment/notification. Added a RTL3d1 test asserting channel is attaching on reconnection, adjusted a send-blocking sequence in a test, and improved orphan-timer diagnostics.

Changes

Cohort / File(s)	Summary
Connection state transition lib/src/main/java/io/ably/lib/transport/ConnectionManager.java	In StateChangeAction.enactState() call states.get(...).enact(...) before invoking connection.onConnectionStateChange(change); terminal-state transport cleanup remains but follows the new ordering.
Realtime tests & diagnostics lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java	Added channel_state_is_attaching_on_reconnected() RTL3d1 test; changed channel_attach_retry_failed to block mockTransport.send() earlier; added stringStackTrace(Thread) helper and improved orphan-Timer failure messages; minor import adjustments.

Sequence Diagram(s)

sequenceDiagram
    participant CM as ConnectionManager
    participant SH as StateHandler
    participant CL as ConnectionListener
    participant T as Transport

    CM->>SH: enact(stateIndication, change)
    SH-->>CM: enact complete
    alt state changed (current != previous)
        CM->>CL: onConnectionStateChange(change)
        CL-->>CM: listener callbacks (may call attach())
    end
    alt currentState.terminal
        CM->>T: clearTransport()
    end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰
I hopped through states with careful cheer,
Enact the step, then let ears hear.
Listeners wait, then spring to play—
Reconnects now tiptoe, not relay. 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ 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 summarizes the main change: reordering state updates to occur before connection state listener notification, with clear reference to the RTL3d1 specification.
Linked Issues check	✅ Passed	The code changes directly address issue #1201 by reordering state enaction before listener notification and adding a test validating the channel state during reconnection.
Out of Scope Changes check	✅ Passed	All changes are scoped to the linked issue: ConnectionManager state transition reordering, test modifications for proper sequencing, and a new RTL3d1 compliance test.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch ECO-5700/update-state-change-order

---

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

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/src/main/java/io/ably/lib/transport/ConnectionManager.java`:
- Around line 606-608: The code dispatches state transition work using the
originally requested state (stateIndication.state) instead of the
validated/actual state returned by setState(), causing handlers (states.enact)
to run for an incorrect state; change the call to use the validated current
state (e.g., use change.current or the state from setState() result) when
invoking states.get(...).enact(...), so replace
states.get(stateIndication.state).enact(stateIndication, change) with a call
that uses the validated state identifier (such as
states.get(change.current).enact(stateIndication, change)) to ensure channel
handlers receive the actual transition.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 177b8abb-69b4-4f95-8bb8-9ea97f032d3b

📥 Commits

Reviewing files that changed from the base of the PR and between 3375a5b and 09be0f1.

📒 Files selected for processing (2)

• lib/src/main/java/io/ably/lib/transport/ConnectionManager.java
• lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java`:
- Around line 2656-2704: The test method
channel_state_is_attaching_on_reconnected creates an AblyRealtime instance
(variable ably) but never closes it; wrap the test body that uses ably in a
try-finally and ensure ably.close() (or the appropriate shutdown method on
AblyRealtime) is called in the finally block so the client and its threads are
always cleaned up even if assertions fail or exceptions are thrown.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: becf14d3-2d3d-4a86-a750-5e330f831314

📥 Commits

Reviewing files that changed from the base of the PR and between 09be0f1 and 2758570.

📒 Files selected for processing (2)

• lib/src/main/java/io/ably/lib/transport/ConnectionManager.java
• lib/src/test/java/io/ably/lib/test/realtime/RealtimeChannelTest.java

[SimonWoolf]

not familiar with ably-java but looks plausible