-
Notifications
You must be signed in to change notification settings - Fork 27
Release/2.1.4 #675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release/2.1.4 #675
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,4 +16,4 @@ | |
| logger.addHandler(logging.NullHandler()) | ||
|
|
||
| api_version = '3' | ||
| lib_version = '2.1.3' | ||
| lib_version = '2.1.4' | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,6 +1,14 @@ | ||||||||||||||||||||||||||||||||
| import asyncio | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| import pytest | ||||||||||||||||||||||||||||||||
| from websockets import connect as _ws_connect | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||
| # websockets 15+ preferred import | ||||||||||||||||||||||||||||||||
| from websockets.asyncio.server import serve as ws_serve | ||||||||||||||||||||||||||||||||
| except ImportError: | ||||||||||||||||||||||||||||||||
| # websockets 14 and earlier fallback | ||||||||||||||||||||||||||||||||
| from websockets.server import serve as ws_serve | ||||||||||||||||||||||||||||||||
|
Comment on lines
+4
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Check how websockets connect is imported in the main codebase
rg -n "from websockets" --type=pyRepository: ably/ably-python Length of output: 869 🏁 Script executed: cat -n ably/transport/websockettransport.py | head -30Repository: ably/ably-python Length of output: 1178 🏁 Script executed: # Check if _ws_connect is used in the test file
rg -n "_ws_connect" test/ably/realtime/realtimeconnection_test.py
# Check websockets version constraints
rg -n "websockets" pyproject.toml setup.py requirements.txt 2>/dev/null || trueRepository: ably/ably-python Length of output: 398 Add try/except fallback for Line 4 imports try:
# websockets 15+ preferred import
from websockets import connect as _ws_connect
except ImportError:
# websockets 14 and earlier fallback
from websockets.client import connect as _ws_connectWithout this, the test will fail to import on Python 3.7/3.8 with websockets 10-14. 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| from ably.realtime.connection import ConnectionEvent, ConnectionState | ||||||||||||||||||||||||||||||||
| from ably.transport.defaults import Defaults | ||||||||||||||||||||||||||||||||
|
|
@@ -10,6 +18,68 @@ | |||||||||||||||||||||||||||||||
| from test.ably.utils import BaseAsyncTestCase | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| async def _relay(src, dst): | ||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||
| async for msg in src: | ||||||||||||||||||||||||||||||||
| await dst.send(msg) | ||||||||||||||||||||||||||||||||
| except Exception: | ||||||||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| class WsProxy: | ||||||||||||||||||||||||||||||||
| """Local WS proxy that forwards to real Ably and lets tests trigger a normal close.""" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| def __init__(self, target_host: str): | ||||||||||||||||||||||||||||||||
| self.target_host = target_host | ||||||||||||||||||||||||||||||||
| self.server = None | ||||||||||||||||||||||||||||||||
| self.port: int | None = None | ||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Python 3.7-3.9 compatibility issue with union type syntax. The 🐛 Option 1: Add annotations import at file topfrom __future__ import annotations🐛 Option 2: Use Optional from typing+from typing import Optional
+
class WsProxy:
"""Local WS proxy that forwards to real Ably and lets tests trigger a normal close."""
def __init__(self, target_host: str):
self.target_host = target_host
self.server = None
- self.port: int | None = None
+ self.port: Optional[int] = None
self._close_event: asyncio.Event | None = NoneNote: Also update line 36 for 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| self._close_event: asyncio.Event | None = None | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| async def _handler(self, client_ws): | ||||||||||||||||||||||||||||||||
| # Create a fresh event for this connection; signal to drop the connection cleanly | ||||||||||||||||||||||||||||||||
| self._close_event = asyncio.Event() | ||||||||||||||||||||||||||||||||
| path = client_ws.request.path # e.g. "/?key=...&format=json" | ||||||||||||||||||||||||||||||||
| target_url = f"wss://{self.target_host}{path}" | ||||||||||||||||||||||||||||||||
|
Comment on lines
+41
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: In websockets 15.x (new asyncio API), the server handler no longer receives async def handler(conn):
path = conn.request.path # e.g. "/chat?room=1"
headers = conn.request.headers
🏁 Script executed: # Find websockets version requirement
git ls-files | grep -E "(requirements|setup|pyproject|poetry)" | head -20Repository: ably/ably-python Length of output: 84 🏁 Script executed: # Search for websockets imports and version info in test file
rg -n "websockets|ServerConnection|WebSocketServerProtocol" test/ably/realtime/realtimeconnection_test.pyRepository: ably/ably-python Length of output: 487 🏁 Script executed: # Check for version constraints in common config files
find . -maxdepth 2 -type f \( -name "*.txt" -o -name "*.toml" -o -name "*.cfg" -o -name "*.yaml" -o -name "*.yml" \) | xargs grep -l "websockets" 2>/dev/null | head -10Repository: ably/ably-python Length of output: 76 🏁 Script executed: # Check websockets version requirement in pyproject.toml
cat pyproject.toml | grep -A 10 -B 2 "websockets"Repository: ably/ably-python Length of output: 550 🏁 Script executed: # Read the test file context around lines 41-42 to understand client_ws
sed -n '1,50p' test/ably/realtime/realtimeconnection_test.pyRepository: ably/ably-python Length of output: 1872 🏁 Script executed: # Search for other handler usages and websockets API calls in the codebase
rg -n "\.request\." --type py test/ably/realtime/realtimeconnection_test.pyRepository: ably/ably-python Length of output: 132 🏁 Script executed: # Check if there are other websockets handlers in the codebase
rg -n "async def.*handler|request\.path|\.path" --type py test/ | head -30Repository: ably/ably-python Length of output: 1638 🏁 Script executed: # Search for websockets version checks or conditionals in the codebase
rg -n "websockets|version|__version__" test/ably/realtime/realtimeconnection_test.pyRepository: ably/ably-python Length of output: 487 🌐 Web query:
💡 Result: In the Python
If you’re using the new asyncio implementation (introduced and migrated via
So for the new API, access the path via the connection’s HTTP request (per the migration guidance):
Sources: Citations:
🏁 Script executed: # Check if there's any version detection or compatibility code already in the codebase
rg -n "websockets.__version__|pkg_resources|importlib" --type py . | head -20Repository: ably/ably-python Length of output: 42 Fix websockets API compatibility: use The code uses 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||
| async with _ws_connect(target_url, ping_interval=None) as server_ws: | ||||||||||||||||||||||||||||||||
| c2s = asyncio.create_task(_relay(client_ws, server_ws)) | ||||||||||||||||||||||||||||||||
| s2c = asyncio.create_task(_relay(server_ws, client_ws)) | ||||||||||||||||||||||||||||||||
| close_task = asyncio.create_task(self._close_event.wait()) | ||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||
| await asyncio.wait([c2s, s2c, close_task], return_when=asyncio.FIRST_COMPLETED) | ||||||||||||||||||||||||||||||||
| finally: | ||||||||||||||||||||||||||||||||
| c2s.cancel() | ||||||||||||||||||||||||||||||||
| s2c.cancel() | ||||||||||||||||||||||||||||||||
| close_task.cancel() | ||||||||||||||||||||||||||||||||
| except Exception: | ||||||||||||||||||||||||||||||||
| pass | ||||||||||||||||||||||||||||||||
|
Comment on lines
+54
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do not swallow proxy handler failures. At Line 54-Line 55, 🐛 Proposed fix- except Exception:
- pass
+ except Exception as exc:
+ raise RuntimeError("WsProxy handler failed") from exc📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
| # After _handler returns the websockets server sends a normal close frame (1000) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| async def close_active_connection(self): | ||||||||||||||||||||||||||||||||
| """Trigger a normal WS close (code 1000) on the currently active client connection. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Signals the handler to exit; the websockets server framework then sends the | ||||||||||||||||||||||||||||||||
| close frame automatically when the handler coroutine returns. | ||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||
| if self._close_event: | ||||||||||||||||||||||||||||||||
| self._close_event.set() | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| @property | ||||||||||||||||||||||||||||||||
| def endpoint(self) -> str: | ||||||||||||||||||||||||||||||||
| """Endpoint string to pass to AblyRealtime (combine with tls=False).""" | ||||||||||||||||||||||||||||||||
| return f"127.0.0.1:{self.port}" | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| async def __aenter__(self): | ||||||||||||||||||||||||||||||||
| self.server = await ws_serve(self._handler, "127.0.0.1", 0, ping_interval=None) | ||||||||||||||||||||||||||||||||
| self.port = self.server.sockets[0].getsockname()[1] | ||||||||||||||||||||||||||||||||
| return self | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| async def __aexit__(self, *args): | ||||||||||||||||||||||||||||||||
| if self.server: | ||||||||||||||||||||||||||||||||
| self.server.close() | ||||||||||||||||||||||||||||||||
| await self.server.wait_closed() | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| class TestRealtimeConnection(BaseAsyncTestCase): | ||||||||||||||||||||||||||||||||
| async def asyncSetUp(self): | ||||||||||||||||||||||||||||||||
| self.test_vars = await TestApp.get_test_vars() | ||||||||||||||||||||||||||||||||
|
|
@@ -399,3 +469,37 @@ async def on_protocol_message(msg): | |||||||||||||||||||||||||||||||
| await asyncio.wait_for(ably.connection.once_async(ConnectionState.CONNECTED), timeout=5) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| await ably.close() | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| async def test_normal_ws_close_triggers_immediate_reconnection(self): | ||||||||||||||||||||||||||||||||
| """Server normal WS close (code 1000) must trigger immediate reconnection. | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| Regression test: ConnectionClosedOK was silently swallowed and deactivate_transport | ||||||||||||||||||||||||||||||||
| was never called, leaving the client disconnected until the idle timer fired. | ||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||
| async with WsProxy(self.test_vars["host"]) as proxy: | ||||||||||||||||||||||||||||||||
| ably = await TestApp.get_ably_realtime( | ||||||||||||||||||||||||||||||||
| disconnected_retry_timeout=500_000, | ||||||||||||||||||||||||||||||||
| suspended_retry_timeout=500_000, | ||||||||||||||||||||||||||||||||
| tls=False, | ||||||||||||||||||||||||||||||||
| realtime_host=proxy.endpoint, | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
|
Comment on lines
+479
to
+485
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Test will not use the proxy — The Use 🐛 Proposed fixFirst, update the ably = await TestApp.get_ably_realtime(
disconnected_retry_timeout=500_000,
suspended_retry_timeout=500_000,
tls=False,
- endpoint=proxy.endpoint,
+ realtime_host="127.0.0.1",
+ port=proxy.port,
)📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| try: | ||||||||||||||||||||||||||||||||
| await asyncio.wait_for( | ||||||||||||||||||||||||||||||||
| ably.connection.once_async(ConnectionState.CONNECTED), timeout=10 | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Simulate server sending a normal WS close frame | ||||||||||||||||||||||||||||||||
| await proxy.close_active_connection() | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Must go CONNECTING quickly — not after the 25 s idle timer | ||||||||||||||||||||||||||||||||
| await asyncio.wait_for( | ||||||||||||||||||||||||||||||||
| ably.connection.once_async(ConnectionState.CONNECTING), timeout=1 | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||
| # Must reconnect immediately — not after the 500 s retry timer | ||||||||||||||||||||||||||||||||
| await asyncio.wait_for( | ||||||||||||||||||||||||||||||||
| ably.connection.once_async(ConnectionState.CONNECTED), timeout=10 | ||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||
| finally: | ||||||||||||||||||||||||||||||||
| await ably.close() | ||||||||||||||||||||||||||||||||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor formatting inconsistency with heading level.
Line 7 uses
### What's Changed(h3), but previous changelog entries (lines 15, 24, 33, etc.) use## What's Changed(h2). Consider using h2 for consistency with the existing format.📝 Suggested fix
📝 Committable suggestion
🤖 Prompt for AI Agents