fix(client): reject calls when websocket is not open instead of sending into a closed socket#1581
Conversation
…f sending into a closed socket
📝 WalkthroughWalkthroughWebSocket send now enforces open-state at send time by adding WEBSOCKET_OPEN, expanding the websocket type, waiting out CONNECTING, and throwing if readyState is not OPEN. Tests add removeEventListener mocks, align open-state in a send test, and add a ChangesWebSocket readiness enforcement
Possibly related PRs
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Code Review
This pull request adds a check to ensure the WebSocket connection is open before sending messages, throwing an error if it is not, and includes tests to verify this behavior. The review feedback correctly points out a potential hang if the WebSocket fails to connect initially, as the 'untilOpen' promise only listens for the 'open' event. To resolve this, it is recommended to also listen for the 'close' event to prevent requests from hanging indefinitely.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — Prevents LinkWebsocketClient from sending frames into a closed or connecting socket by checking readyState before send(), rejecting the call when the socket isn't open. This avoids ghost requests and hung promises with reconnecting wrappers like partysocket.
- Guard
websocket.send()withreadyStatecheck — afterawait untilOpen,link-client.tsthrows ifreadyState !== WEBSOCKET_OPEN, so frames never reach a dead socket. - Two new tests + one existing-test fix — "rejects instead of sending when socket is not open" (fresh CLOSED socket), "rejects instead of sending when socket closes between requests" (OPEN → CLOSED transition), and the existing "waits until open" test now sets
readyState = 1beforeonOpen()to satisfy the new guard.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
…ng sends immediately
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — Adds close event listener during the CONNECTING wait so requests that arrive while the socket is connecting don't hang forever when the initial connection fails, and provides cleanup via removeEventListener.
- Wait for
closeas well asopenduringCONNECTING— the send callback now listens for both events (with cleanup) so a failed initial connect resolves the wait and falls through to the "not open" rejection instead of hanging. - Four new tests in
non-open socketsdescribe block — covers closed-from-start, closed-between-requests, reconnect-window with successful reopen, and failed-reconnect-while-connecting scenarios. removeEventListenerwired into mocks — existing websocket mocks and the newcreateWebSockethelper gainremoveEventListenerto match the updatedPick<>type.
DeepSeek Pro (free via Pullfrog for OSS) | 𝕏
|
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
More templates
@orpc/ai-sdk
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-iterator
@orpc/hey-api
@orpc/interop
@orpc/json-schema
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/otel
@orpc/experimental-pino
@orpc/experimental-publisher
@orpc/experimental-publisher-durable-object
@orpc/experimental-ratelimit
@orpc/react
@orpc/react-query
@orpc/experimental-react-swr
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fastify
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
Please provide more detail and repro for this |
|
@dinwwwh It happens consistently on prod apps that I sadly can't show here. A repro is trivial, you probably don't even need it to understand immediately what's going on. :
The root cause is that oRPC's ws link client was written with the native (non-reconnecting) WebSocket in mind. In that case, it is correct to have Fundamentally, the issue is in oRPC's link client, it can't be properly fixed on partysocket side nor in the retry logic. |
Why "fires as many frames to the server as there were retries"?, retry should only happen one time and want until queued-frame send to server and server response. |
|
There are still some issues when relying on something like PartySocket for retries, but from what I can tell this PR only addresses the Messages no longer enter the queue because |
|
@dinwwwh Because the call doesn't stay pending until the buffered frame is delivered, oRPC rejects it first, on every failed reconnect attempt.
and partysocket emits a close event on every failed reconnect attempt, not just once. The retry can't "wait for the queued frame" because from the client peer's perspective the call already failed. And it can't not reject on close either, cause for in-flight calls on a genuinely dead connection that rejection is correct. So the fix has to be at the send boundary: never hand a frame to a socket that isn't OPEN (wait while CONNECTING, reject otherwise). Then a frame's lifetime cannot outlive the call id it belongs to, and the buffer-replay issue disappears regardless of partysocket settings. |
With the fix in this PR, you mechanically NEVER rely on partysocket for retries. It either waits in oRPC or fails in oRPC. |
Do you mean that when a reconnect attempt fails, PartySocket keeps the queued messages instead of clearing them? If so, the condition for reproducing this issue would be allowing PartySocket to retry multiple times. Each failed reconnect attempt emits another |
|
Yes, failed attempts emit close but never clear the queue. Everything in it is replayed onto the next successful connection. That's not a PartySocket bug though. Buffering across reconnects is its intended feature, and it works fine for protocols where messages are connection-independent. oRPC's protocol isn't: request ids are only valid while ClientPeer remembers them, and peer.close() forgets them on every close event. So a buffered frame outlives the call it belongs to. PartySocket can't know that, and no setting fixes it: buffering replays dead ids, and maxEnqueuedMessages: 0 drops the frame while the call hangs forever. Only oRPC knows whether a frame's id is still alive, so the check has to live in oRPC: never hand a frame to a non-OPEN socket, wait while CONNECTING, reject otherwise. Then frames can't sit anywhere oRPC doesn't control, and any reconnecting wrapper works out of the box. |
|
Thanks, this fixes some problems, but I think we still need a more powerful retry mechanism in v2 - something that actually understands the protocol. |
|
It's probably adjacent to the current fix, no matter who does the retrying, oRPC must not give the socket a message for a request it has already dropped. Without that guarantee at the send level, a v2 retry would hit the same problems. Thanks for the merge! |

Problem
LinkWebsocketClientcallswebsocket.send()without checking the socket state. With a plain WebSocket this doesn't matter, but with reconnecting sockets like partysocket (which the docs recommend) it breaks in two ways:peer.close()), so the server ends up with ghost requests, e.g. duplicate event-iterator subscriptions streaming everything twice (or N times), and mutations that get re-executed.maxEnqueuedMessages: 0on partysocket's side: the frame is silently dropped, but the peer still awaits a response forever. The call never settles, so caller retry logic never runs.Easy to reproduce: subscribe to an event iterator, kill the server for ~20s while the client retries, bring it back. Each retry leaves one buffered REQUEST frame, and on reconnect they all become live server-side subscriptions.
Fix
Each send now dispatches on
readyState:OPEN=> send, as before.CONNECTING=> wait for the nextopen/closeevent, then re-check. This covers the initial connect (replacing the constructor-leveluntilOpen) and also reconnect windows on reconnecting sockets: a call made while an attempt is in flight is sent once the connection opens, or rejected if the attempt fails, instead of erroring out during a routinely-occurring ~50ms window.CLOSING/CLOSED=> throw.peer.request()already handles a throwing send by rejecting the call, so callers get an immediate error instead of a hang or a ghost frame.Native WebSocket behavior is unchanged:
CONNECTINGonly happens before the first open, and after a close all pending calls are rejected anyway. The guard only turns a send into a dead socket from a silent no-op into a proper rejection.One behavioral note: frames can no longer be buffered across a reconnect by the socket wrapper. I think that's correct: request ids are connection-scoped, so replaying frames onto a new connection is never safe, and callers already have to handle rejection since close rejects everything in flight. (The wait-during-CONNECTING path is safe for the same reason in reverse: if a call is rejected while its frame is waiting,
ClientPeer's id-guard drops the frame instead of sending it.)