Repository navigation
Test WebSocket callbacks before the first HTTP request - #4085
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing. Review the Copilot code review billing settings.
T4rk1n
left a comment
There was a problem hiding this comment.
Looks good. Confirmed the test passes on the rc2 code and fails with the same WebSocketCallbackError from #4082 when the websocket branch in DashMiddleware.__call__ is disabled, so it guards the right path. One small note inline, not blocking. No CHANGELOG entry needed for a test-only change.
| from starlette.testclient import TestClient | ||
|
|
||
| app = Dash(__name__, backend="fastapi") | ||
| app.layout = html.Div([html.Button(id="ws-btn"), html.Div(id="ws-out")]) |
There was a problem hiding this comment.
If this test ever regresses, _setup_server() never runs, so ws-out.children stays in _callback.GLOBAL_CALLBACK_MAP and gets pulled into whatever app the next test builds. Can you pop it in a finally (or a small fixture), like tests/unit/test_main_module_alias.py does, so one failure doesn't cascade?
There was a problem hiding this comment.
The test now removes ws-out.children from GLOBAL_CALLBACK_MAP and GLOBAL_CALLBACK_LIST in a finally, like test_main_module_alias.py. With _setup_server patched to a no-op, only this test fails now, and a test that runs after it sees no leftover callback (0d0fc1d).
|



#4082 reported that with the FastAPI backend, a WebSocket callback request sent to a fresh process before any HTTP request was rejected with "does not have websocket=True". Callbacks registered with the global
@callbackare only copied intoapp.callback_mapby_setup_server(), a before_request hook, and on 4.5.0rc1 the middleware did not run the hooks for WebSocket scopes.That bug is already fixed on dev by #4075 (in 4.5.0rc2):
DashMiddleware._gate_websocketruns the before_request hooks on the WebSocket handshake, so_setup_server()runs before the request is validated. The reproducer from the issue now passes.This PR only adds a regression test for the path the issue hit: a global
@callback(..., websocket=True)and a WebSocketcallback_requestsent before any HTTP request. The existing tests do not cover it.test_fabr003checks that the hooks gate the handshake, andtests/websocket/test_ws_stream.pycallsapp._setup_server()by hand before connecting, which is the workaround from the issue.@app.callbackwrites intoapp.callback_mapdirectly, so only the global@callbackshows the problem.The new test is
test_fabr006_websocket_callback_before_any_http_requestintests/backend_tests/test_fastapi_before_request.py. It passes on dev. With the websocket branch inDashMiddleware.__call__removed, it fails with the sameWebSocketCallbackErroras in the issue.Contributor Checklist
pytest tests/backend_tests/test_fastapi_before_request.py tests/websocket/test_ws_stream.py -k "not wsst004", plus black and flake8 on the changed file)Closes #4082