Skip to content

Stop the broadcaster from losing cancellations on Python 3.11 - #7

Merged
hyperloop-cornell merged 1 commit into
mainfrom
fix/broadcast-cancellation
Oct 14, 2026
Merged

hyperloop-cornell merged 1 commit into
mainfrom
fix/broadcast-cancellation

Conversation

@WesP10

@WesP10 WesP10 commented Oct 1, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Broadcaster._send now uses asyncio.timeout() instead of asyncio.wait_for() for the per-client send timeout.
  • On Python 3.11, wait_for returns normally if it is cancelled just as the wrapped send finishes, so the cancellation is lost. gather then raises a bare CancelledError that anyio doesn't recognise as its own, and the error escapes the cancel scope.
  • In tests this shows up as concurrent.futures.CancelledError from WebSocketTestSession.__exit__ in tests/test_hub_ws.py. Which test fails varies; it is usually test_hub_status_broadcast. Since at least Sep 27 it has failed the hyperloop-gui test-api job in about half of runs.
  • The timeout behaviour is unchanged: a client that doesn't accept a message within SEND_TIMEOUT_SECONDS still raises TimeoutError and is dropped. test_slow_browser_is_dropped still passes.

Merge notes

hyperloop-cornell/hyperloop-gui#23 pins this branch's commit (46e47f2), so merge this PR first and use a merge commit (not squash). Otherwise the pinned commit is not on main.

Test plan

  • Ran tests/test_hub_ws.py 100 times on Python 3.11 (python:3.11-slim, same as CI): 50/100 runs failed before the fix, 0/100 after
  • Full suite on 3.11: 80 passed
  • Same on Python 3.13: 0/100 failures both before and after, so the bug is specific to 3.11's wait_for; full suite 80 passed

🤖 Generated with Claude Code

asyncio.wait_for on 3.11 returns normally when it is cancelled just as the
wrapped send completes. gather then raises a bare CancelledError that anyio
does not recognise as its own, so cancelling a hub or browser handler during a
broadcast could leak the cancellation. This made test_hub_ws fail in about half
of CI runs and blocked deploys. asyncio.timeout handles cancellation correctly
and keeps the same per-client send timeout.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
Copilot AI balanced review requested due to automatic review settings October 1, 2026 21:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@hyperloop-cornell hyperloop-cornell left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

lgtm

simple timeout fix

@hyperloop-cornell
hyperloop-cornell merged commit a10436a into main Oct 14, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants