[ECO-5698] fix: handle normal WebSocket close frames and improve reconnection logic

[ttypic]

resolves #671

• Added local WebSocket proxy for testing (WsProxy) and corresponding tests for immediate reconnection on normal close.
• Fixed missing reconnection on server-sent normal WebSocket close frames in WebSocketTransport.
• Adjusted idle timer handling to avoid accidental reuse.

Summary by CodeRabbit

• Bug Fixes

  • Fixed WebSocket protocol selection to respect TLS (secure vs. standard).
  • Improved handling of normal WebSocket closure so reconnection occurs immediately rather than waiting.
  • Resolved idle timer management so timers are reset rather than duplicated.

• Tests

  • Added an automated test validating immediate reconnection after a normal WebSocket server close.

[coderabbitai]

Warning

Rate limit exceeded

@ttypic has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 23 minutes and 23 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 18552b9f-af82-4a2c-89a1-90d5e31b751a

📥 Commits

Reviewing files that changed from the base of the PR and between 69a7527 and 7cc7742.

📒 Files selected for processing (1)

• test/ably/realtime/realtimechannelmutablemessages_test.py

Walkthrough

Transport now respects TLS when building WS URLs, cancels and resets idle timers instead of layering them, and treats a normal WebSocket read-loop exit as a clean close — disposing resources and deactivating the transport to allow immediate reconnection.

Changes

Cohort / File(s)	Summary
WebSocket transport ably/transport/websockettransport.py	Selects wss/ws in connect() per TLS option; _handle_websocket_connection() handles normal read-loop exit as clean close (dispose + deactivate) instead of silencing; set_idle_timer() cancels existing timer before creating a new one.
Realtime tests / proxy test/ably/realtime/realtimeconnection_test.py	Adds WsProxy test helper and _relay for forwarding; async context manager and close_active_connection() to trigger clean server-side close; new test test_normal_ws_close_triggers_immediate_reconnection verifying immediate reconnect after server-initiated normal close.

Sequence Diagram(s)

mermaid
sequenceDiagram
participant Client as Ably Client
participant Transport as WebSocketTransport
participant Proxy as WsProxy
participant Server as Real WS Server

Client->>Transport: connect()
Transport->>Proxy: open websocket to proxy.endpoint
Proxy->>Server: relay connection
Server-->>Proxy: send CLOSE (1000)
Proxy-->>Transport: forward CLOSE frame
Transport->>Transport: read loop exits normally
Transport->>Transport: dispose resources & deactivate
Transport->>Client: notify disconnected
Client->>Transport: immediate reconnect (no idle delay)
Transport->>Proxy: open new websocket (reconnect)

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 A gentle tap, a tidy close,
No lingering wait, the pathway shows,
Timers reset and sockets clear,
I hop, I nudge, the reconnect's near,
Tests cheer as the network flows.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% 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 clearly identifies the main fix (handling normal WebSocket close frames) and improvement (reconnection logic), matching the primary changes in the changeset.
Linked Issues check	✅ Passed	The changes address the core requirement from issue #671: preventing ConnectionClosedOK from being silenced and ensuring immediate reconnection on normal WebSocket close frames.
Out of Scope Changes check	✅ Passed	All changes are directly related to the linked issue: WebSocket transport fixes, idle timer handling, and test infrastructure for validating the reconnection behavior.

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch ECO-5698/fix-websocket-close

📝 Coding Plan

• Generate coding plan for human review comments

---

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 `@test/ably/realtime/realtimeconnection_test.py`:
- Around line 4-5: The test imports use a websockets 15+ path for the server
(from websockets.asyncio.server import serve as ws_serve) which breaks on
websockets 14; update the import to try the asyncio.server location first and
fall back to the 14-style location (import serve from websockets.server) using a
try/except ImportError pattern so that _ws_connect and ws_serve work across
versions—apply the same fallback pattern used in websockettransport.py to
replace the direct serve import.

🪄 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: 5e88438e-621b-4a71-986c-a86504579c8f

📥 Commits

Reviewing files that changed from the base of the PR and between 5cdfeb3 and 8627a99.

📒 Files selected for processing (2)

• ably/transport/websockettransport.py
• test/ably/realtime/realtimeconnection_test.py

[owenpearson]

nice one