Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions RELEASE.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Release type: patch

Await cancelled subscription tasks during WebSocket shutdown so their `finally` blocks run before shared state (DB pools, event loop) is torn down.
Original file line number Diff line number Diff line change
Expand Up @@ -92,9 +92,17 @@ async def shutdown(self) -> None:
with suppress(asyncio.CancelledError):
await self.connection_init_timeout_task

cancelled_tasks: list[asyncio.Task] = []
for operation_id in list(self.operations.keys()):
op = self.operations[operation_id]
if op.task:
cancelled_tasks.append(op.task)
await self.cleanup_operation(operation_id)

await self.reap_completed_tasks()
# cleanup_operation cancels but does not await tasks (would block
# the message loop). Safe to await here — no more messages to process.
await asyncio.gather(*cancelled_tasks, return_exceptions=True)

def on_request_accepted(self) -> None:
# handle_request should call this once it has sent the
Expand Down
51 changes: 51 additions & 0 deletions tests/websockets/test_graphql_transport_ws.py
Original file line number Diff line number Diff line change
Expand Up @@ -1216,3 +1216,54 @@ async def test_unexpected_client_disconnects_are_gracefully_handled(

assert not process_errors.called
assert Subscription.active_infinity_subscriptions == 0


async def test_shutdown_awaits_cancelled_subscription_tasks(
http_client: HttpClient,
):
with contextlib.suppress(ImportError):
from tests.http.clients.channels import ChannelsHttpClient

if isinstance(http_client, ChannelsHttpClient):
pytest.skip("Can't patch on_init for this client")

handler = None
cleanup_done_at_shutdown_end = None

def on_init(_handler):
nonlocal handler
if handler:
return
handler = _handler
original_shutdown = _handler.shutdown

async def tracked_shutdown():
nonlocal cleanup_done_at_shutdown_end
await original_shutdown()
cleanup_done_at_shutdown_end = Subscription.active_infinity_subscriptions

_handler.shutdown = tracked_shutdown

with patch.object(DebuggableGraphQLTransportWSHandler, "on_init", on_init):
async with http_client.ws_connect(
"/graphql", protocols=[GRAPHQL_TRANSPORT_WS_PROTOCOL]
) as ws:
await ws.send_message({"type": "connection_init"})
ack: ConnectionAckMessage = await ws.receive_json()
assert ack == {"type": "connection_ack"}

await ws.send_message(
{
"id": "sub1",
"type": "subscribe",
"payload": {"query": 'subscription { infinity(message: "Hi") }'},
}
)
await ws.receive(timeout=2)
assert Subscription.active_infinity_subscriptions == 1

await ws.close()

await asyncio.sleep(0.5)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Short sleep may cause intermittent failures

The existing similar test test_unexpected_client_disconnects_are_gracefully_handled uses asyncio.sleep(1) to wait for server-side shutdown. Using only 0.5 seconds here could make this test intermittently fail on slow CI systems, since the tracked_shutdown coroutine must fully complete (including the new asyncio.gather over cancelled tasks) within that window.

Suggested change
await asyncio.sleep(0.5)
await asyncio.sleep(1)

assert handler is not None
assert cleanup_done_at_shutdown_end == 0
Comment on lines +1267 to +1269
Copy link

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The shutdown sync point is currently a fixed await asyncio.sleep(0.5). This can be flaky on slow CI (shutdown may not have run yet, leaving cleanup_done_at_shutdown_end as None). Prefer synchronizing deterministically (e.g., set an asyncio.Event in tracked_shutdown and await asyncio.wait_for(event.wait(), timeout=...), or poll cleanup_done_at_shutdown_end is not None with wait_for) so the test fails only when the shutdown invariant is actually broken.

Copilot uses AI. Check for mistakes.
Loading