Skip to content

Commit 83269a5

Browse files
authored
fix(client): cancel the SSE stream on teardown to avoid leaking connections (#580)
# Description `readFrom` — used by `parseSseStream` for both the JSON-RPC and REST client transports — only calls `reader.releaseLock()` in its `finally`: ```ts async function* readFrom(stream: ReadableStream<string>) { const reader = stream.getReader(); try { while (true) { const { done, value } = await reader.read(); if (done) break; yield value; } } finally { reader.releaseLock(); // detaches the reader, but never cancels the stream } } ``` When a consumer stops iterating early — a `break`, a `throw`, or the REST transport throwing on an `error` event — the async generator's `return()`/`throw()` runs that `finally`. `releaseLock()` detaches the reader but does **not** cancel the underlying `ReadableStream`, so the fetch body (and its socket) is left open. Repeated early terminations leak connections. ## Fix Cancel the reader on teardown so cancellation propagates to the response body: ```ts } finally { await reader.cancel().catch(() => {}); reader.releaseLock(); } ``` Semantics (verified against the WHATWG Streams behaviour): - **early break / throw** → `cancel()` propagates to the source, closing the connection (the fix). - **normal completion** → the stream is already closed; `cancel()` is a no-op. - **errored source** → `cancel()` rejects with the stored error; it is ignored so the original error still surfaces to the caller. ## Tests Adds regression tests asserting the underlying stream is canceled when the consumer breaks early and when it throws mid-iteration. Both fail without the fix (the source `cancel` callback is never invoked) and pass with it. Full suite green (1370 tests). - [x] Follows the `CONTRIBUTING` guide - [x] PR title uses Conventional Commits (`fix:`) - [x] Tests and linter pass - [ ] Docs updated (not necessary)
1 parent 5833652 commit 83269a5

2 files changed

Lines changed: 91 additions & 0 deletions

File tree

src/sse_utils.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,10 @@ async function* readFrom(stream: ReadableStream<string>): AsyncGenerator<string,
144144
yield value;
145145
}
146146
} finally {
147+
// `releaseLock()` alone leaves the body un-cancelled, leaking the
148+
// connection when a consumer breaks/throws early. `.catch()` swallows the
149+
// rejection cancel() produces on an already-errored stream.
150+
await reader.cancel().catch(() => {});
147151
reader.releaseLock();
148152
}
149153
}

test/sse_utils.spec.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,33 @@ function createMockResponseWithoutAsyncIterator(sseData: string): Response {
5151
});
5252
}
5353

54+
// The teardown tests observe the leak fix by watching the source stream's
55+
// `cancel` callback fire through `parseSseStream`'s internal
56+
// `pipeThrough(TextDecoderStream)`. Some runtimes (notably the Cloudflare
57+
// Workers test pool / workerd) do not propagate a TextDecoderStream cancel to
58+
// the upstream source, so that signal is unobservable there even though the
59+
// fix runs. Probe the capability once and gate the assertions on it — the
60+
// leak matters most under Node/undici, where the probe passes.
61+
async function cancelPropagatesThroughTextDecoder(): Promise<boolean> {
62+
let cancelled = false;
63+
const source = new ReadableStream<Uint8Array>({
64+
start(controller) {
65+
controller.enqueue(new Uint8Array([1]));
66+
},
67+
cancel() {
68+
cancelled = true;
69+
},
70+
});
71+
// Mirror parseSseStream: pipe the response body through a TextDecoderStream.
72+
const body = new Response(source).body;
73+
if (!body) return false;
74+
const reader = body.pipeThrough(new TextDecoderStream()).getReader();
75+
await reader.read();
76+
await reader.cancel().catch(() => {});
77+
return cancelled;
78+
}
79+
const CANCEL_PROPAGATES = await cancelPropagatesThroughTextDecoder();
80+
5481
describe('SSE Utils', () => {
5582
describe('formatSSEEvent', () => {
5683
it('should format a data event', () => {
@@ -177,6 +204,66 @@ describe('SSE Utils', () => {
177204
});
178205
});
179206

207+
describe('parseSseStream teardown', () => {
208+
it.runIf(CANCEL_PROPAGATES)(
209+
'cancels the underlying stream when the consumer stops early',
210+
async () => {
211+
// An early break must cancel the response body, not just release the lock.
212+
let sourceCancelled = false;
213+
const stream = new ReadableStream<Uint8Array>({
214+
start(controller) {
215+
// One event, then stay open — a long-lived SSE connection.
216+
controller.enqueue(new TextEncoder().encode('data: {"id":1}\n\n'));
217+
},
218+
cancel() {
219+
sourceCancelled = true;
220+
},
221+
});
222+
const response = new Response(stream, {
223+
headers: { 'Content-Type': 'text/event-stream' },
224+
});
225+
226+
const seen: SseEvent[] = [];
227+
for await (const event of parseSseStream(response)) {
228+
seen.push(event);
229+
break;
230+
}
231+
232+
expect(seen).toHaveLength(1);
233+
expect(sourceCancelled).toBe(true);
234+
}
235+
);
236+
237+
it.runIf(CANCEL_PROPAGATES)(
238+
'cancels the underlying stream when the consumer throws',
239+
async () => {
240+
let sourceCancelled = false;
241+
const stream = new ReadableStream<Uint8Array>({
242+
start(controller) {
243+
controller.enqueue(new TextEncoder().encode('data: {"id":1}\n\n'));
244+
},
245+
cancel() {
246+
sourceCancelled = true;
247+
},
248+
});
249+
const response = new Response(stream, {
250+
headers: { 'Content-Type': 'text/event-stream' },
251+
});
252+
253+
await expect(
254+
(async () => {
255+
for await (const event of parseSseStream(response)) {
256+
expect(event.data).toBe('{"id":1}');
257+
throw new Error('consumer boom');
258+
}
259+
})()
260+
).rejects.toThrow('consumer boom');
261+
262+
expect(sourceCancelled).toBe(true);
263+
}
264+
);
265+
});
266+
180267
describe('Symmetry: parser understands formatter output', () => {
181268
it('should parse what formatSSEEvent produces', async () => {
182269
const originalData = { kind: 'task', id: '123', status: 'completed' };

0 commit comments

Comments
 (0)