From 5c36bbb98ef84959e898329f2ce8701f619cceb4 Mon Sep 17 00:00:00 2001 From: dvcolomban Date: Thu, 6 Aug 2026 17:11:55 +0200 Subject: [PATCH] feat(client): add DevframeRpcClient.close() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `DevframeRpcClient` has no `close()`/`dispose()` of any kind — a connection lives until the process ends, with no way for a caller to tear it down. The server transport already has the symmetric piece: `WsRpcTransport.close()` (attachWsRpcTransport) detaches upgrade routing, force-closes every connected peer, and closes any server it created itself. The client side has never had an equivalent. This bites any caller that races a connection attempt against its own deadline (`Promise.race([connectDevframe(...), timeout])`) — the deadline can't cancel the loser, so a slow-to-resolve attempt becomes a fully connected, unreferenced client with nothing able to close it. ## Changes - `createWsRpcChannel` (rpc/transports/ws-client.ts) returns `close()`, closing the underlying `WebSocket`. Widens its return type to `ChannelOptions & { close: () => void }`, since birpc's own `ChannelOptions` has no teardown of its own. - `createWsRpcClientMode` hoists its channel to a local so it can close it, and exposes `close()` on `DevframeRpcClientMode`. - `createStaticRpcClientMode` gets a no-op `close()` — a static backend has no live socket, every call is a local fetch — so the two modes stay union-compatible. - `DevframeRpcClient.close()` delegates to the mode. No behavior change for anyone not calling it. ## Tests - `ws-client.test.ts` (new): `close()` closes the underlying `WebSocket`. - `rpc-ws-status.test.ts`: `createWsRpcClientMode`'s `close()` closes its socket. - `rpc.test.ts`: `getDevframeRpcClient`'s `close()` closes the socket on a `websocket` backend, and is a no-op (not a throw) on a `static` one. - `rpc-auth-gate.test.ts`: updated its hand-typed `DevframeRpcClientMode` mock for the new required field. `pnpm --filter devframe exec tsc --noEmit` clean. `pnpm exec vitest run --project devframe` — 51 files, 463 tests (was 459), all green. Also ran `--project @devframes/hub --project @devframes/hub-ui` (129 tests) and `tsc --noEmit` across `@devframes/hub`, `@devframes/hub-ui`, `@devframes/nuxt` — the packages consuming `DevframeRpcClient` — clean. `eslint` clean on every touched file. 🤖 Generated with [Claude Code](https://claude.com/claude-code) test: update tsnapi snapshot for DevframeRpcClient.close() CI's exports.test.ts snapshots each package's public .d.ts surface; adding close() to DevframeRpcClient/DevframeRpcClientMode is a real, intentional API change, so devframe/client.snapshot.d.ts needs to reflect it. Verified with a full `pnpm run build` at the repo root first, so tsnapi reads every package's real dist output rather than erroring on a missing one — this is the only snapshot that changed; close() doesn't ripple into any plugin's or @devframes/hub's public surface. `pnpm exec vitest run` — 101 files, 1087 tests, all green (was 1 failing). `pnpm run lint` and `pnpm run typecheck` (25 packages via turbo) both clean. fix(client): make DevframeRpcClient.close() optional Copilot's review on this PR flagged both close() additions (DevframeRpcClient and DevframeRpcClientMode) as a breaking change: adding a required property to an exported, externally-implementable interface breaks any existing consumer that hand-types a mock/adapter against it without close() — as this repo's own rpc-auth-gate.test.ts mock did until now. Made close?: () => void on both interfaces instead. Every factory this repo owns (createWsRpcClientMode, createStaticRpcClientMode) still provides it unconditionally, so nothing here loses close(); only the type requirement is relaxed. getDevframeRpcClient's close now calls mode.close?.() to match. Updated the three call sites that invoked close() directly against the now-optional type (rpc.test.ts, rpc-ws-status.test.ts) to close?.(), and removed the auth-gate mock's close() entirely rather than keep it, since a mode without one is now exactly the scenario this is meant to keep working — a new test asserts rpc.close() against it doesn't throw. Verified with a full `pnpm run build` + `pnpm exec vitest run --project tests -u`: only the client tsnapi snapshot changed (close: () => void -> close?: () => void), nothing else. Full suite: 101 files, 1088 tests (was 1087), all green. `pnpm run typecheck` and `pnpm run lint` clean across all 22 packages. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- .../devframe/src/client/rpc-auth-gate.test.ts | 15 +++++ packages/devframe/src/client/rpc-static.ts | 2 + .../devframe/src/client/rpc-ws-status.test.ts | 9 +++ packages/devframe/src/client/rpc-ws.ts | 56 ++++++++++--------- packages/devframe/src/client/rpc.test.ts | 35 ++++++++++++ packages/devframe/src/client/rpc.ts | 17 ++++++ .../src/rpc/transports/ws-client.test.ts | 40 +++++++++++++ .../devframe/src/rpc/transports/ws-client.ts | 8 ++- .../tsnapi/devframe/client.snapshot.d.ts | 2 + 9 files changed, 158 insertions(+), 26 deletions(-) create mode 100644 packages/devframe/src/rpc/transports/ws-client.test.ts diff --git a/packages/devframe/src/client/rpc-auth-gate.test.ts b/packages/devframe/src/client/rpc-auth-gate.test.ts index 34cb3d51..f6b82606 100644 --- a/packages/devframe/src/client/rpc-auth-gate.test.ts +++ b/packages/devframe/src/client/rpc-auth-gate.test.ts @@ -37,6 +37,8 @@ vi.mock('./rpc-ws', () => ({ call: fakeMode.call as DevframeRpcClientMode['call'], callOptional: fakeMode.callOptional as DevframeRpcClientMode['callOptional'], callEvent: fakeMode.callEvent as DevframeRpcClientMode['callEvent'], + // No `close` here on purpose — `close` is optional precisely so a mode written before it + // existed (this one) still satisfies the interface. })), })) @@ -137,4 +139,17 @@ describe('getDevframeRpcClient — auth bootstrap gates outbound calls', () => { // Sent straight through — no more waiting once bootstrap is over. expect(fakeMode.call).toHaveBeenCalledTimes(1) }) + + it('close() is a no-op, not a throw, against a mode that predates it', async () => { + const { getDevframeRpcClient } = await import('./rpc') + const rpc = await getDevframeRpcClient({ + connectionMeta, + otpParam: false, + simpleAuth: false, + }) + + // The mocked mode above has no `close` at all — exactly the pre-existing-mode case + // `close?:` exists to keep working. + expect(() => rpc.close?.()).not.toThrow() + }) }) diff --git a/packages/devframe/src/client/rpc-static.ts b/packages/devframe/src/client/rpc-static.ts index 4fff7fe1..171f629e 100644 --- a/packages/devframe/src/client/rpc-static.ts +++ b/packages/devframe/src/client/rpc-static.ts @@ -35,5 +35,7 @@ export async function createStaticRpcClientMode( args[0] as string, args.slice(1), ), + // No live socket to close — every call is a local fetch. + close: () => {}, } } diff --git a/packages/devframe/src/client/rpc-ws-status.test.ts b/packages/devframe/src/client/rpc-ws-status.test.ts index 939ef31e..744e165c 100644 --- a/packages/devframe/src/client/rpc-ws-status.test.ts +++ b/packages/devframe/src/client/rpc-ws-status.test.ts @@ -148,4 +148,13 @@ describe('ws client connection status', () => { expect(rpcErrors[0].error).toBeInstanceOf(DevframeConnectionError) expect(rpcErrors[0].method).toBe('demo:method') }) + + it('close() closes the underlying socket', () => { + const { mode, ws } = setup() + const closeSpy = vi.spyOn(ws, 'close') + + mode.close?.() + + expect(closeSpy).toHaveBeenCalledTimes(1) + }) }) diff --git a/packages/devframe/src/client/rpc-ws.ts b/packages/devframe/src/client/rpc-ws.ts index c6c1da5a..bec33415 100644 --- a/packages/devframe/src/client/rpc-ws.ts +++ b/packages/devframe/src/client/rpc-ws.ts @@ -206,34 +206,37 @@ export function createWsRpcClientMode( for (const name of connectionMeta.jsonSerializableMethods ?? []) definitions.set(name, { jsonSerializable: true }) + // Hoisted out of the `createRpcClient` call so `close()` below can reach it — birpc's own + // `ChannelOptions` carries no reference back to what it was built from. + const channel = createWsRpcChannel({ + url, + authToken, + definitions, + ...wsOptions, + onConnected(event) { + // Socket open — the trust handshake (already queued) settles the + // status to `connected`/`unauthorized`. Stay `connecting` until then. + wsOptions.onConnected?.(event) + }, + onError(error) { + setStatus('error', error) + events.emit('connection:error', error) + rejectAllPending(new DevframeConnectionError('connection', '[devframe] Connection to the devframe server failed', { cause: error })) + wsOptions.onError?.(error) + }, + onDisconnected(event) { + // A clean close after we were connected, or a socket that never + // opened — either way calls can no longer be served. + if (status !== 'error') + setStatus('disconnected') + rejectAllPending(new DevframeConnectionError('connection', '[devframe] Disconnected from the devframe server', { cause: connectionError ?? undefined })) + wsOptions.onDisconnected?.(event) + }, + }) const serverRpc = createRpcClient( clientRpc.functions, { - channel: createWsRpcChannel({ - url, - authToken, - definitions, - ...wsOptions, - onConnected(event) { - // Socket open — the trust handshake (already queued) settles the - // status to `connected`/`unauthorized`. Stay `connecting` until then. - wsOptions.onConnected?.(event) - }, - onError(error) { - setStatus('error', error) - events.emit('connection:error', error) - rejectAllPending(new DevframeConnectionError('connection', '[devframe] Connection to the devframe server failed', { cause: error })) - wsOptions.onError?.(error) - }, - onDisconnected(event) { - // A clean close after we were connected, or a socket that never - // opened — either way calls can no longer be served. - if (status !== 'error') - setStatus('disconnected') - rejectAllPending(new DevframeConnectionError('connection', '[devframe] Disconnected from the devframe server', { cause: connectionError ?? undefined })) - wsOptions.onDisconnected?.(event) - }, - }), + channel, rpcOptions, }, ) @@ -402,5 +405,8 @@ export function createWsRpcClientMode( method, ) }, + close: () => { + channel.close() + }, } } diff --git a/packages/devframe/src/client/rpc.test.ts b/packages/devframe/src/client/rpc.test.ts index 2fde3105..f72cbce6 100644 --- a/packages/devframe/src/client/rpc.test.ts +++ b/packages/devframe/src/client/rpc.test.ts @@ -159,4 +159,39 @@ describe('getDevframeRpcClient — connection meta base', () => { // An explicit meta resolves against the client's own base. expect(lastWsUrl()).toBe('ws://localhost:5173/__foo/__ws') }) + + it('close() closes the underlying socket', async () => { + const served: ConnectionMeta = { backend: 'websocket', websocket: { path: '__ws' } } + vi.stubGlobal('fetch', vi.fn(async () => ({ + ok: true, + status: 200, + json: async () => served, + }) as any)) + + const rpc = await getDevframeRpcClient({ baseURL: '/__foo/', otpParam: false }) + const ws = FakeWebSocket.instances.at(-1)! + const closeSpy = vi.spyOn(ws, 'close') + + rpc.close?.() + + expect(closeSpy).toHaveBeenCalledTimes(1) + }) + + it('close() on a static backend is a no-op, not a throw', async () => { + vi.stubGlobal('fetch', vi.fn(async (url: string) => ({ + ok: true, + status: 200, + json: async () => ( + url.includes('__rpc-dump') + ? {} // an empty manifest is a valid (if trivial) StaticRpcManifest + : { backend: 'static' } satisfies ConnectionMeta + ), + }) as any)) + + const rpc = await getDevframeRpcClient({ baseURL: '/__foo/', otpParam: false }) + + expect(() => rpc.close?.()).not.toThrow() + // Static backends never open a socket in the first place. + expect(FakeWebSocket.instances).toHaveLength(0) + }) }) diff --git a/packages/devframe/src/client/rpc.ts b/packages/devframe/src/client/rpc.ts index 3fcf65f9..a393f49b 100644 --- a/packages/devframe/src/client/rpc.ts +++ b/packages/devframe/src/client/rpc.ts @@ -195,6 +195,20 @@ export interface DevframeRpcClient { (namespace: NS): DevframeScopedClientContext> (namespace?: null | ''): DevframeRpcClient } + + /** + * Close the connection. A `static` backend is a no-op (there is no live socket to close); + * a `websocket` backend closes the underlying `WebSocket`, which the server observes as a + * normal disconnect. Mirrors {@link WsRpcTransport.close} on the server side. + * + * There is no corresponding "reconnect" — a closed client is done. Discard it and call + * {@link getDevframeRpcClient} again to reconnect. + * + * Optional so a `DevframeRpcClientMode` implemented before this method existed — a custom + * transport, a hand-typed mock — still satisfies the interface; an absent `close` is treated + * as nothing to close. + */ + close?: () => void } export interface DevframeRpcClientMode { @@ -212,6 +226,8 @@ export interface DevframeRpcClientMode { call: DevframeRpcClient['call'] callEvent: DevframeRpcClient['callEvent'] callOptional: DevframeRpcClient['callOptional'] + /** See {@link DevframeRpcClient.close}. */ + close?: () => void } export async function getDevframeRpcClient( @@ -375,6 +391,7 @@ export async function getDevframeRpcClient( streaming: undefined!, cacheManager, scope: undefined!, + close: () => mode.close?.(), } rpc.sharedState = createRpcSharedStateClientHost(rpc) diff --git a/packages/devframe/src/rpc/transports/ws-client.test.ts b/packages/devframe/src/rpc/transports/ws-client.test.ts new file mode 100644 index 00000000..93f84ddf --- /dev/null +++ b/packages/devframe/src/rpc/transports/ws-client.test.ts @@ -0,0 +1,40 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { createWsRpcChannel } from './ws-client' + +// A minimal fake WebSocket — only what `createWsRpcChannel` touches. +class FakeWebSocket { + static OPEN = 1 + static instances: FakeWebSocket[] = [] + + readyState = FakeWebSocket.OPEN + + constructor(public url: string) { + FakeWebSocket.instances.push(this) + } + + addEventListener(): void {} + removeEventListener(): void {} + send(): void {} + close(): void {} +} + +describe('createWsRpcChannel', () => { + beforeEach(() => { + FakeWebSocket.instances = [] + vi.stubGlobal('WebSocket', FakeWebSocket) + }) + + afterEach(() => { + vi.unstubAllGlobals() + }) + + it('close() closes the underlying socket', () => { + const channel = createWsRpcChannel({ url: 'ws://localhost:5173/__ws' }) + const ws = FakeWebSocket.instances.at(-1)! + const closeSpy = vi.spyOn(ws, 'close') + + channel.close() + + expect(closeSpy).toHaveBeenCalledTimes(1) + }) +}) diff --git a/packages/devframe/src/rpc/transports/ws-client.ts b/packages/devframe/src/rpc/transports/ws-client.ts index 21e69aa2..b749349b 100644 --- a/packages/devframe/src/rpc/transports/ws-client.ts +++ b/packages/devframe/src/rpc/transports/ws-client.ts @@ -27,8 +27,11 @@ const EMPTY_DEFS: ReadonlyMap void } { let url = options.url if (options.authToken) { url = `${url}?${DEVFRAME_AUTH_TOKEN_QUERY_PARAM}=${encodeURIComponent(options.authToken)}` @@ -59,6 +62,9 @@ export function createWsRpcChannel(options: WsRpcChannelOptions): ChannelOptions // method up in `definitions` and pick the right encoder. const pendingRequestMethods = new Map() return { + close: () => { + ws.close() + }, on: (handler: (data: string) => void) => { ws.addEventListener('message', (e) => { handler(e.data) diff --git a/tests/__snapshots__/tsnapi/devframe/client.snapshot.d.ts b/tests/__snapshots__/tsnapi/devframe/client.snapshot.d.ts index e5892068..d10b837b 100644 --- a/tests/__snapshots__/tsnapi/devframe/client.snapshot.d.ts +++ b/tests/__snapshots__/tsnapi/devframe/client.snapshot.d.ts @@ -29,6 +29,7 @@ export interface DevframeRpcClient { (_: NS): DevframeScopedClientContext>; (_?: null | ''): DevframeRpcClient; }; + close?: () => void; } export interface DevframeRpcClientMode { readonly isTrusted: boolean; @@ -41,6 +42,7 @@ export interface DevframeRpcClientMode { call: DevframeRpcClient['call']; callEvent: DevframeRpcClient['callEvent']; callOptional: DevframeRpcClient['callOptional']; + close?: () => void; } export interface DevframeRpcClientOptions extends SetupDevframeConnectionOptions { authToken?: string;