Skip to content

Commit a3f9677

Browse files
committed
fix: dispose RPC authentication channel and trust deadlines
1 parent 1bd966f commit a3f9677

4 files changed

Lines changed: 128 additions & 16 deletions

File tree

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
import type { DevframeRpcClientFunctions } from 'devframe/types'
2+
import type { DevframeClientRpcHost, DevframeRpcContext, RpcClientEvents } from './rpc'
3+
import { RpcFunctionsCollectorBase } from 'devframe/rpc'
4+
import { createEventEmitter } from 'devframe/utils/events'
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
6+
import { createLiveRpcClientMode } from './rpc-live'
7+
8+
vi.mock('devframe/rpc/client', () => ({
9+
createRpcClient: () => ({ $call: vi.fn(async () => ({ isTrusted: true })) }),
10+
}))
11+
12+
function createMode() {
13+
const clientRpc: DevframeClientRpcHost = new RpcFunctionsCollectorBase<DevframeRpcClientFunctions, DevframeRpcContext>({ rpc: undefined! })
14+
return createLiveRpcClientMode({
15+
transport: 'websocket',
16+
connectionMeta: { backend: 'websocket', websocket: { path: '__ws' } },
17+
events: createEventEmitter<RpcClientEvents>(),
18+
clientRpc,
19+
createChannel: () => ({ post: vi.fn(), on: vi.fn(), close: vi.fn() }),
20+
})
21+
}
22+
23+
describe('trust deadline cleanup', () => {
24+
beforeEach(() => {
25+
vi.useFakeTimers()
26+
vi.stubGlobal('navigator', { userAgent: 'test' })
27+
vi.stubGlobal('location', { origin: 'http://localhost' })
28+
})
29+
30+
afterEach(() => {
31+
vi.useRealTimers()
32+
vi.unstubAllGlobals()
33+
})
34+
35+
it('clears concurrent deadlines as soon as authentication succeeds', async () => {
36+
expect.assertions(4)
37+
const mode = createMode()
38+
const first = mode.ensureTrusted(60_000)
39+
const second = mode.ensureTrusted(30_000)
40+
expect(vi.getTimerCount()).toBe(2)
41+
await mode.requestTrustWithToken('test-token')
42+
await expect(first).resolves.toBe(true)
43+
await expect(second).resolves.toBe(true)
44+
expect(vi.getTimerCount()).toBe(0)
45+
})
46+
47+
it('leaves no deadline behind when already trusted', async () => {
48+
expect.assertions(2)
49+
const mode = createMode()
50+
await mode.requestTrustWithToken('test-token')
51+
await expect(mode.ensureTrusted()).resolves.toBe(true)
52+
expect(vi.getTimerCount()).toBe(0)
53+
})
54+
55+
it('preserves expiry and unlimited trust waits', async () => {
56+
expect.assertions(4)
57+
const mode = createMode()
58+
const unlimited = mode.ensureTrusted(0)
59+
expect(vi.getTimerCount()).toBe(0)
60+
const expiry = expect(mode.ensureTrusted(10)).rejects.toThrow('Timeout waiting for rpc to be trusted')
61+
await vi.advanceTimersByTimeAsync(10)
62+
await expiry
63+
expect(vi.getTimerCount()).toBe(0)
64+
await mode.requestTrustWithToken('test-token')
65+
await expect(unlimited).resolves.toBe(true)
66+
})
67+
})

‎packages/devframe/src/client/rpc-live.ts‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -261,18 +261,21 @@ export function createLiveRpcClientMode(
261261
if (timeout <= 0)
262262
return trustedPromise.promise
263263

264-
let clear = () => {}
265-
await Promise.race([
266-
trustedPromise.promise.then(clear),
267-
new Promise((resolve, reject) => {
268-
const id = setTimeout(() => {
269-
reject(new Error('[devframe] Timeout waiting for rpc to be trusted'))
270-
}, timeout)
271-
clear = () => clearTimeout(id)
272-
}),
273-
])
274-
275-
return isTrusted
264+
let timer: ReturnType<typeof setTimeout> | undefined
265+
try {
266+
await Promise.race([
267+
trustedPromise.promise,
268+
new Promise<never>((_, reject) => {
269+
timer = setTimeout(() => {
270+
reject(new Error('[devframe] Timeout waiting for rpc to be trusted'))
271+
}, timeout)
272+
}),
273+
])
274+
return isTrusted
275+
}
276+
finally {
277+
clearTimeout(timer)
278+
}
276279
}
277280

278281
return {

‎packages/devframe/src/client/rpc.test.ts‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,36 @@ describe('getDevframeRpcClient: connection meta base', () => {
5757
delete (globalThis as any)[DEVFRAME_CONNECTION_KEY]
5858
})
5959

60+
it('closes the authentication broadcast channel with the RPC client', async () => {
61+
expect.assertions(1)
62+
const closeChannel = vi.spyOn(FakeBroadcastChannel.prototype, 'close')
63+
const rpc = await getDevframeRpcClient({
64+
connectionMeta: { backend: 'websocket', websocket: { path: '__ws' } },
65+
otpParam: false,
66+
simpleAuth: false,
67+
webmcp: false,
68+
})
69+
rpc.close?.()
70+
expect(closeChannel).toHaveBeenCalledExactlyOnceWith()
71+
})
72+
73+
it('still closes the transport when closing the authentication channel fails', async () => {
74+
expect.assertions(2)
75+
const failure = new Error('channel cleanup failed')
76+
vi.spyOn(FakeBroadcastChannel.prototype, 'close').mockImplementation(() => {
77+
throw failure
78+
})
79+
const closeTransport = vi.spyOn(FakeWebSocket.prototype, 'close')
80+
const rpc = await getDevframeRpcClient({
81+
connectionMeta: { backend: 'websocket', websocket: { path: '__ws' } },
82+
otpParam: false,
83+
simpleAuth: false,
84+
webmcp: false,
85+
})
86+
expect(() => rpc.close?.()).toThrow(failure)
87+
expect(closeTransport).toHaveBeenCalledExactlyOnceWith()
88+
})
89+
6090
it('publishes the meta annotated with the absolute base it resolved from', async () => {
6191
const served: ConnectionMeta = { backend: 'websocket', websocket: { path: '__ws' } }
6292
vi.stubGlobal('fetch', vi.fn(async () => ({

‎packages/devframe/src/client/rpc.ts‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -445,6 +445,21 @@ export async function getDevframeRpcClient(
445445
}) as F
446446
}
447447

448+
/** Release authentication and transport resources even if another disposer fails. */
449+
function closeRpcClient(): void {
450+
try {
451+
disposeWebMcp?.()
452+
}
453+
finally {
454+
try {
455+
authChannel?.close()
456+
}
457+
finally {
458+
mode.close?.()
459+
}
460+
}
461+
}
462+
448463
const rpc: DevframeRpcClient = {
449464
events,
450465
get isTrusted() {
@@ -495,10 +510,7 @@ export async function getDevframeRpcClient(
495510
streaming: undefined!,
496511
cacheManager,
497512
scope: undefined!,
498-
close: () => {
499-
disposeWebMcp?.()
500-
mode.close?.()
501-
},
513+
close: closeRpcClient,
502514
}
503515

504516
rpc.sharedState = createRpcSharedStateClientHost(rpc)

0 commit comments

Comments
 (0)