Skip to content

Commit 64fa136

Browse files
fix(client): listen() honors RequestOptions.signal
listen(filter, options?) is typed RequestOptions but only options.timeout was honored; options.signal was silently ignored. The auto-open path forwards connect's options into listen(), so a connect-time signal was also dropped. Honor options.signal in the state machine: an already-aborted signal rejects synchronously (mirrors request()); an abort while `opening` rejects the pending listen() promise with the signal's reason and tears the wire down; an abort while `open` closes the subscription. The abort listener is removed by settle() once the subscription has closed.
1 parent c3ec4a8 commit 64fa136

2 files changed

Lines changed: 84 additions & 0 deletions

File tree

packages/client/src/client/client.ts

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1260,6 +1260,10 @@ export class Client extends Protocol<ClientContext> {
12601260
);
12611261
}
12621262

1263+
// Honor RequestOptions.signal exactly as request() does: an
1264+
// already-aborted signal rejects synchronously before any setup.
1265+
options?.signal?.throwIfAborted();
1266+
12631267
if (this._onParkedNotification === undefined) {
12641268
this._onParkedNotification = raw => this._listenFirstLook(raw);
12651269
}
@@ -1281,6 +1285,7 @@ export class Client extends Protocol<ClientContext> {
12811285
// leak the entry.
12821286
let listenMessageId: number | undefined;
12831287
let ackTimer: ReturnType<typeof setTimeout> | undefined;
1288+
let onCallerAbort: (() => void) | undefined;
12841289
let resolveOpening!: (honored: SubscriptionFilter) => void;
12851290
let rejectOpening!: (error: Error) => void;
12861291
const opening = new Promise<SubscriptionFilter>((resolve, reject) => {
@@ -1303,6 +1308,9 @@ export class Client extends Protocol<ClientContext> {
13031308
return;
13041309
}
13051310
state = 'closed';
1311+
if (onCallerAbort !== undefined) {
1312+
options?.signal?.removeEventListener('abort', onCallerAbort);
1313+
}
13061314
if (listenMessageId !== undefined) {
13071315
this._listenState.delete(listenMessageId);
13081316
}
@@ -1347,6 +1355,22 @@ export class Client extends Protocol<ClientContext> {
13471355
void wireTeardown().catch(() => {});
13481356
}, ackTimeout);
13491357

1358+
// RequestOptions.signal aborts the subscription at any point in its
1359+
// lifecycle (mirrors request()'s cancel path). While `opening`, settle
1360+
// rejects the pending listen() promise with the signal's reason; while
1361+
// `open`, it transitions to `closed` and tears the wire down. The
1362+
// listener is removed by `settle()` once the subscription has closed.
1363+
if (options?.signal) {
1364+
const callerSignal = options.signal;
1365+
onCallerAbort = () => {
1366+
if (state === 'closed') return;
1367+
const reason = callerSignal.reason;
1368+
settle({ error: reason instanceof Error ? reason : new Error(String(reason ?? 'Aborted')) });
1369+
void wireTeardown().catch(() => {});
1370+
};
1371+
callerSignal.addEventListener('abort', onCallerAbort, { once: true });
1372+
}
1373+
13501374
try {
13511375
// The per-subscription state is registered BEFORE the request is
13521376
// sent (`onBeforeSend`) so a synchronously-delivered ack (an

packages/client/test/client/listen.test.ts

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,66 @@ describe('Client.listen()', () => {
285285
await client.close();
286286
});
287287

288+
it('options.signal aborted while opening: listen() rejects fast with the signal reason', async () => {
289+
const [clientTx, serverTx] = InMemoryTransport.createLinkedPair();
290+
const written: JSONRPCMessage[] = [];
291+
serverTx.onmessage = m => {
292+
written.push(m);
293+
const req = m as { id?: number | string; method?: string };
294+
if (req.method === 'server/discover' && req.id !== undefined) {
295+
void serverTx.send({
296+
jsonrpc: '2.0',
297+
id: req.id,
298+
result: {
299+
resultType: 'complete',
300+
supportedVersions: [MODERN],
301+
capabilities: {},
302+
serverInfo: { name: 's', version: '1' }
303+
}
304+
});
305+
}
306+
// No ack for subscriptions/listen — stays in `opening`.
307+
};
308+
await serverTx.start();
309+
const client = new Client({ name: 'c', version: '1' }, { versionNegotiation: { mode: 'auto' } });
310+
await client.connect(clientTx);
311+
const ac = new AbortController();
312+
const t0 = Date.now();
313+
const pending = client.listen({ toolsListChanged: true }, { signal: ac.signal });
314+
ac.abort(new Error('caller-abort'));
315+
const error = await pending.catch(e => e as Error);
316+
expect((error as Error).message).toBe('caller-abort');
317+
expect(Date.now() - t0).toBeLessThan(1000);
318+
// wireTeardown sent notifications/cancelled referencing the listen id.
319+
await flush();
320+
const listenId = (written.find(m => (m as { method?: string }).method === 'subscriptions/listen') as { id: number | string }).id;
321+
const cancelled = written.find(m => (m as { method?: string }).method === 'notifications/cancelled') as
322+
| { params: { requestId: unknown } }
323+
| undefined;
324+
expect(cancelled?.params.requestId).toBe(listenId);
325+
// No leaked state.
326+
expect((client as unknown as { _listenState: Map<unknown, unknown> })._listenState.size).toBe(0);
327+
expect((client as unknown as { _responseHandlers: Map<unknown, unknown> })._responseHandlers.size).toBe(0);
328+
await client.close();
329+
});
330+
331+
it('options.signal aborted while open: closes the subscription (notifications/cancelled sent)', async () => {
332+
const { clientTx, written } = await scriptedModern();
333+
const client = new Client({ name: 'c', version: '1' }, { versionNegotiation: { mode: 'auto' } });
334+
await client.connect(clientTx);
335+
const ac = new AbortController();
336+
const sub = await client.listen({ toolsListChanged: true }, { signal: ac.signal });
337+
const listenId = (written.find(m => (m as { method?: string }).method === 'subscriptions/listen') as { id: number | string }).id;
338+
written.length = 0;
339+
ac.abort();
340+
await flush();
341+
expect(written).toEqual([{ jsonrpc: '2.0', method: 'notifications/cancelled', params: { requestId: listenId } }]);
342+
// close() after signal-abort is idempotent.
343+
await sub.close();
344+
expect(written).toHaveLength(1);
345+
await client.close();
346+
});
347+
288348
it('rejects with NotConnected (as a rejected promise, no setup) when no transport is connected', async () => {
289349
const { clientTx } = await scriptedModern();
290350
const client = new Client({ name: 'c', version: '1' }, { versionNegotiation: { mode: 'auto' } });

0 commit comments

Comments
 (0)