Skip to content

Add a real WebSocket builtin module - #510

Open
dan-distributive wants to merge 1 commit into
mainfrom
websocket-builtin-module
Open

dan-distributive wants to merge 1 commit into
mainfrom
websocket-builtin-module

Conversation

@dan-distributive

Copy link
Copy Markdown
Contributor

Summary

pythonmonkey has never had a WebSocket implementation. dcp-client's own SocketIOTransport.buildOptions() explicitly special-cases this — every platform except pythonmonkey gets transports: ['websocket', 'polling'] and upgrades to a persistent WebSocket within the first round trip or two; pythonmonkey is hard-restricted to ['polling'] forever. That forces pythonmonkey to be the only client that ever sustains HTTP long-polling for an entire connection's lifetime against services like DCP's package manager — a code path essentially unexercised by any other real client, where a real, reproducible session-routing bug lives (confirmed independent of pythonmonkey/dcp-client with plain curl, aiohttp, and Node https).

Rather than work around that bug, this gives pythonmonkey a real WebSocket — a JS class matching the DOM API (builtin_modules/WebSocket.js), backed by a Python aiohttp-based implementation (builtin_modules/WebSocket-internal.py), following the exact existing pattern XMLHttpRequest already uses (loaded dynamically via require(), no C++ rebuild needed).

Two real bugs were found and fixed while building it (documented inline in both files' own docstrings — read those for the reasoning):

  • A race between the 'open' event firing and send/close functions being available (real clients, dcp-client included, often send their first message immediately on open).
  • engine.io-client's real WS transport unconditionally calling this.ws._socket.unref() — a Node ws-library-shaped assumption that doesn't hold for a browser-style WebSocket — worked around with a harmless no-op _socket property.

This alone doesn't change any existing behaviordcp-client's platform check still needs its pythonmonkey exclusion relaxed on its own side (a separate, dcp-client-side change, not part of this PR) before anything actually uses this.

Test plan

  • Verified via a real end-to-end DCP job (job.exec(), real network dispatch to the demo/dcp compute group) after also relaxing dcp-client's platform check locally — real WebSocket connection, real job completion.
  • Not tested against every WebSocket-consuming code path in dcp-client — only the one that motivated this (the package-manager Socket.IO connection).

Note on authorship

Largely produced by Claude (Anthropic's Claude Code) working under my direction. Flagging this since it hasn't had a second human reviewer yet.

🤖 Generated with Claude Code

pythonmonkey has never had a WebSocket implementation. dcp-client's own
SocketIOTransport.buildOptions() explicitly special-cases this: every
platform except pythonmonkey gets transports: ['websocket', 'polling']
and upgrades to a persistent WebSocket within the first round trip or
two; pythonmonkey is hard-restricted to ['polling'] forever, since
`typeof WebSocket` was previously `undefined`. That forces pythonmonkey
to be the only client that ever *sustains* HTTP long-polling for an
entire connection's lifetime against services like DCP's package
manager - a code path essentially unexercised by any other real client,
where a real, reproducible session-routing bug lives (a freshly issued
session id doesn't reliably route to the same backend on the very next
polling request; confirmed independent of pythonmonkey/dcp-client with
plain curl, aiohttp, and Node https).

Rather than work around that bug, this gives pythonmonkey a real
WebSocket implementation - a JS class matching the DOM API
(builtin_modules/WebSocket.js), backed by a Python aiohttp-based
implementation (builtin_modules/WebSocket-internal.py), following the
exact existing pattern XMLHttpRequest already uses (loaded dynamically
via require() at pythonmonkey import time, no C++ rebuild needed).
Two real bugs were found and fixed while building it (documented inline
in WebSocket.js/WebSocket-internal.py's own docstrings):
- A race between the 'open' event firing and send/close functions being
  available, since real clients often send their first message
  immediately on 'open'.
- engine.io-client's real WS transport unconditionally calling
  `this.ws._socket.unref()` (a Node `ws`-library-shaped assumption that
  doesn't hold for a browser-style WebSocket), worked around with a
  harmless no-op `_socket` property.

This alone doesn't change any existing behavior - dcp-client's platform
check still needs its pythonmonkey exclusion relaxed on its own side for
anything to actually use this. Verified via a real end-to-end DCP job
(job.exec(), real network dispatch to the demo/dcp compute group) after
also relaxing that check locally.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

1 participant