fix(server): proxy websocket sandbox routes#388
fix(server): proxy websocket sandbox routes#388liuxiaopai-ai wants to merge 1 commit intoalibaba:mainfrom
Conversation
|
@ninan-nn @Pangjiping This should fix issue #383. Could you please take a look when you have a moment? Thanks! |
Thanks for this, I will review this feature. |
|
@codex review |
|
@liuxiaopai-ai This PR has merge conflicts with the base branch. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 13aeb2ca65
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| port, | ||
| exc, | ||
| ) | ||
| await websocket.close(code=status.WS_1008_POLICY_VIOLATION) |
There was a problem hiding this comment.
Accept websocket before closing on backend handshake failure
This error path calls websocket.close() before any websocket.accept(), which in FastAPI/Starlette results in an HTTP denial response instead of a WebSocket close frame, so clients won't actually receive the intended close code (1008 here) and will observe a generic handshake failure instead. That makes backend handshake/connect failures indistinguishable and can break client logic that depends on WebSocket close semantics.
Useful? React with 👍 / 👎.
Summary
/sandboxes/{id}/proxy/{port}and nested pathsuse_server_proxy=trueRoot cause
Issue #383 is real. The server proxy only registered HTTP routes and explicitly treated websocket upgrades as unsupported, so browser websocket handshakes never reached the sandbox backend. Separately,
get_endpoint(..., use_server_proxy=true)returned/proxy/{port}but the server only handled/proxy/{port}/{full_path}.Testing
cd server && uv run pytest tests/test_routes_proxy.pycd server && uv run pytest tests/test_routes*.pycd server && uv run pyright src/api/lifecycle.py tests/test_routes_proxy.pycd server && uv run ruff check src/api/lifecycle.py tests/test_routes_proxy.pyFixes #383