fix(runtime-sdk): deliver client WebSocket close events to the ASGI app - #158
Conversation
The close handler was assigned to server.onopen (overwriting the open handler assigned on the previous line), so close events never reached the bridge and ASGI apps could never observe client disconnects.
"websocket.close" is the app-to-server message; client disconnects must arrive at the app as "websocket.disconnect", which is the only type frameworks translate into their disconnect exceptions (e.g. Starlette's WebSocketDisconnect).
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
419a2d2 to
e7f5b0e
Compare
whitphx
changed the title
The asgi-prefixed workerd-test case added on this branch hits the same known combination the existing asgi case is already excluded for: pywrangler sync fails to install pyodide packages in the unittest environment on Linux with the Python 3.12 compat config, so the worker boots without pytest. Match the exclusion by prefix so asgi-* cases inherit it.
ryanking13
left a comment
ryanking13
left a comment
There was a problem hiding this comment.
Thanks! I think we need to improve our test architecture a bit to better support more asgi tests, but the changewise looks good to me.
|
@ryanking13 Thanks! I have more changes to propose to this repo in my mind but should I wait for the test architecture improvements to land or skip adding test cases until that? |
You don't need to wait! Those refactor works can go in parallel and you don't need to be blocked by it. It is our job to manage those refactor-chaos not you :) |
2 participants
onclosewas wrongly assigned toserver.onopenwebsocket.close, which per the ASGI spec is the app-to-server direction; client disconnects must arrive aswebsocket.disconnectas the spec says.