Skip to content

fix(client): reject WebSocket link calls when connect returns a closed socket - #2083

Merged
dinwwwh merged 1 commit into
middleapi:mainfrom
dinwwwh:claude/websocket-closed-hanging-36500a
Sep 25, 2026
Merged

dinwwwh merged 1 commit into
middleapi:mainfrom
dinwwwh:claude/websocket-closed-hanging-36500a

Conversation

@dinwwwh

@dinwwwh dinwwwh commented Sep 25, 2026

Copy link
Copy Markdown
Member

With reconnect disabled (the default), calls through the WebSocket link hung forever when connect returned a socket that was already closing or closed. They now reject immediately with AbortError('WebSocket is already closing or closed').

This happens when the socket is created ahead of time, as in the hibernation docs (connect: () => websocket): if the server is down and the socket closes before the first call, that call never settled. A close listener added after the socket closed never fires (confirmed on Node 24 for both the global WebSocket and ws), and send silently discards data on a closed socket, so the close handling from #2081 never ran.

Fixes

  • Calls on a socket that was already closing or closed reject right away instead of hanging.
  • With reconnect enabled, the link still retries as before, but no longer attaches listeners to the dead socket.

Notes

  • A custom WebSocketLike must report readyState as CONNECTING (0) or OPEN (1) to be used; any other value is treated as closed.

Testing

  • New tests in rpc-link.test.ts cover an already closing and an already closed socket with reconnect disabled (both hang on main) and an already closed socket with reconnect enabled.
  • Checked with a real Node WebSocket closed by a refused connection: the call stays pending on main and rejects immediately with this change.
  • pnpm vitest run packages/client/src/adapters/websocket tests/rpc, eslint, and @orpc/client type-check pass.

…d socket

A socket that is already closing or closed never fires `close` again for
listeners added later, and `send` silently discards data, so calls on it
never settled when reconnect was disabled. Treat such a socket as a failed
connection: calls reject immediately, and with reconnect enabled the link
retries without attaching listeners to the dead socket.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ No new issues found.

Reviewed changes

  • WebSocketLinkTransport.getConnectedPeer readyState guard — after connect resolves, a socket whose readyState is neither CONNECTING (0) nor OPEN (1) now throws AbortError('WebSocket is already closing or closed') before any peer/listener is created, so calls on a pre-closed socket reject instead of hanging.
  • Reconnect path preserved — the throw flows through the existing attempt .catch; with reconnect disabled it rejects (and the rejected this.current is reused), and with reconnect enabled it retries without attaching listeners to the dead socket.
  • Test coverage — rpc-link.test.ts gains a parameterized closing/closed rejection test with reconnect disabled (asserts both calls reject, send 0, connect called once) and a closed-then-open reconnect test (asserts retry, closedSocket.addEventListener 0). Both fail on pre-fix code for the right reasons.

I traced the guard against getConnectedPeer's caching/retry logic and the concurrent-call path — the readyState check sits in the only window where listeners would otherwise miss the close event (JS runs the subsequent synchronous listener registration before any close-event task can fire), so the fix is correctly scoped. Locally: packages/client suite passes (506 tests).

Pullfrog  | View workflow run | Using DeepSeek Flash (free via Pullfrog for OSS) | 𝕏

@pkg-pr-new

pkg-pr-new Bot commented Sep 25, 2026

Copy link
Copy Markdown
More templates

@orpc/ai-sdk

npm i https://pkg.pr.new/@orpc/ai-sdk@2083

@orpc/arktype

npm i https://pkg.pr.new/@orpc/arktype@2083

@orpc/bun

npm i https://pkg.pr.new/@orpc/bun@2083

@orpc/client

npm i https://pkg.pr.new/@orpc/client@2083

@orpc/cloudflare

npm i https://pkg.pr.new/@orpc/cloudflare@2083

@orpc/contract

npm i https://pkg.pr.new/@orpc/contract@2083

@orpc/experimental-effect

npm i https://pkg.pr.new/@orpc/experimental-effect@2083

@orpc/evlog

npm i https://pkg.pr.new/@orpc/evlog@2083

@orpc/hibernation

npm i https://pkg.pr.new/@orpc/hibernation@2083

@orpc/json-schema

npm i https://pkg.pr.new/@orpc/json-schema@2083

@orpc/experimental-lock

npm i https://pkg.pr.new/@orpc/experimental-lock@2083

@orpc/experimental-msw

npm i https://pkg.pr.new/@orpc/experimental-msw@2083

@orpc/nest

npm i https://pkg.pr.new/@orpc/nest@2083

@orpc/next

npm i https://pkg.pr.new/@orpc/next@2083

@orpc/node

npm i https://pkg.pr.new/@orpc/node@2083

@orpc/openapi

npm i https://pkg.pr.new/@orpc/openapi@2083

@orpc/opentelemetry

npm i https://pkg.pr.new/@orpc/opentelemetry@2083

@orpc/pinia-colada

npm i https://pkg.pr.new/@orpc/pinia-colada@2083

@orpc/pino

npm i https://pkg.pr.new/@orpc/pino@2083

@orpc/publisher

npm i https://pkg.pr.new/@orpc/publisher@2083

@orpc/ratelimit

npm i https://pkg.pr.new/@orpc/ratelimit@2083

@orpc/server

npm i https://pkg.pr.new/@orpc/server@2083

@orpc/shared

npm i https://pkg.pr.new/@orpc/shared@2083

@orpc/swr

npm i https://pkg.pr.new/@orpc/swr@2083

@orpc/tanstack-query

npm i https://pkg.pr.new/@orpc/tanstack-query@2083

@orpc/trpc

npm i https://pkg.pr.new/@orpc/trpc@2083

@orpc/valibot

npm i https://pkg.pr.new/@orpc/valibot@2083

@orpc/zod

npm i https://pkg.pr.new/@orpc/zod@2083

commit: 3261cf5

@codecov

codecov Bot commented Sep 25, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@codspeed

codspeed Bot commented Sep 25, 2026

Copy link
Copy Markdown
Contributor

Merging this PR will not alter performance

✅ 30 untouched benchmarks


Comparing dinwwwh:claude/websocket-closed-hanging-36500a (3261cf5) with main (0caff28)

Open in CodSpeed

@dinwwwh
dinwwwh merged commit 3c0a03b into middleapi:main Sep 25, 2026
10 of 11 checks passed
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