Skip to content

Commit 90d131e

Browse files
RenKoya1claude
andcommitted
fix(client): cancel the SSE stream on teardown to avoid leaking connections
`readFrom` (used by `parseSseStream` for both the JSON-RPC and REST client transports) only called `reader.releaseLock()` in its `finally`. When a consumer stops iterating early — a `break`, a `throw`, or a REST transport that throws on an `error` event — the async generator's `return()`/`throw()` runs that `finally`, which detaches the reader but never cancels the underlying `ReadableStream`. The fetch body (and its socket) is left open, so repeated early terminations leak connections. Cancel the reader on teardown so cancellation propagates to the response body. On normal completion the stream is already closed and `cancel()` is a no-op; on an errored stream `cancel()` rejects with the same error, which is ignored so the original error still surfaces. Adds regression tests asserting the underlying stream is canceled when the consumer breaks early and when it throws mid-iteration. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 5833652 commit 90d131e

2 files changed

Lines changed: 88 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: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,30 @@ 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 decoded = new ReadableStream<Uint8Array>({
64+
start(controller) {
65+
controller.enqueue(new Uint8Array([1]));
66+
},
67+
cancel() {
68+
cancelled = true;
69+
},
70+
}).pipeThrough(new TextDecoderStream());
71+
const reader = decoded.getReader();
72+
await reader.read();
73+
await reader.cancel().catch(() => {});
74+
return cancelled;
75+
}
76+
const CANCEL_PROPAGATES = await cancelPropagatesThroughTextDecoder();
77+
5478
describe('SSE Utils', () => {
5579
describe('formatSSEEvent', () => {
5680
it('should format a data event', () => {
@@ -177,6 +201,66 @@ describe('SSE Utils', () => {
177201
});
178202
});
179203

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

0 commit comments

Comments
 (0)