Skip to content

Commit 2af59be

Browse files
authored
fix(deno): Handle reader.closed rejection from releaseLock() in streaming (#20187)
This PR replaces `reader.closed.finally(() => onDone())` with `reader.closed.then(() => onDone(), () => onDone())` in `monitorStream`. Per the WHATWG Streams spec, `reader.releaseLock()` rejects `reader.closed` when the promise is still pending. `.finally()` propagates that rejection as an unhandled promise rejection, while `.then(f, f)` suppresses it by handling both the fulfilled and rejected cases. I was not able to reproduce the error directly on my deno version but this should prevent the issue. Closes: #20177
1 parent f1932c9 commit 2af59be

File tree

2 files changed

+60
-2
lines changed

2 files changed

+60
-2
lines changed

packages/deno/src/utils/streaming.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -81,8 +81,10 @@ function monitorStream(
8181
onDone: () => void,
8282
): ReadableStream<Uint8Array<ArrayBufferLike>> {
8383
const reader = stream.getReader();
84-
// oxlint-disable-next-line typescript/no-floating-promises
85-
reader.closed.finally(() => onDone());
84+
reader.closed.then(
85+
() => onDone(),
86+
() => onDone(),
87+
);
8688
return new ReadableStream({
8789
async start(controller) {
8890
let result: ReadableStreamReadResult<Uint8Array<ArrayBufferLike>>;
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// <reference lib="deno.ns" />
2+
3+
import { assertEquals } from 'https://deno.land/std@0.212.0/assert/mod.ts';
4+
5+
Deno.test('reader.closed.then(f, f) suppresses rejection when releaseLock is called on an open stream', async () => {
6+
// Reproduces the bug from GitHub issue #20177:
7+
// In monitorStream, reader.releaseLock() is called while the source stream
8+
// is still open (e.g. the error path when controller.enqueue() throws).
9+
// Per WHATWG Streams spec, this rejects reader.closed with a TypeError.
10+
// Using .then(onDone, onDone) handles both cases; .finally() would propagate
11+
// the rejection as unhandled.
12+
13+
let onDoneCalled = false;
14+
15+
const stream = new ReadableStream<Uint8Array>({
16+
start(controller) {
17+
controller.enqueue(new TextEncoder().encode('data'));
18+
// intentionally not closing — stream stays open
19+
},
20+
});
21+
22+
const reader = stream.getReader();
23+
24+
// This is the exact pattern from monitorStream (line 84 in streaming.ts).
25+
// With .finally(() => onDone()), this would propagate the rejection.
26+
reader.closed.then(
27+
() => {
28+
onDoneCalled = true;
29+
},
30+
() => {
31+
onDoneCalled = true;
32+
},
33+
);
34+
35+
await reader.read();
36+
37+
// This is what monitorStream does on the error path (line 98) when
38+
// controller.enqueue() throws — releaseLock while the source is still open.
39+
reader.releaseLock();
40+
41+
let unhandledRejection: PromiseRejectionEvent | undefined;
42+
const handler = (e: PromiseRejectionEvent): void => {
43+
e.preventDefault();
44+
unhandledRejection = e;
45+
};
46+
globalThis.addEventListener('unhandledrejection', handler);
47+
48+
try {
49+
await new Promise(resolve => setTimeout(resolve, 50));
50+
51+
assertEquals(onDoneCalled, true, 'onDone should have been called via the rejection handler');
52+
assertEquals(unhandledRejection, undefined, 'should not have caused an unhandled promise rejection');
53+
} finally {
54+
globalThis.removeEventListener('unhandledrejection', handler);
55+
}
56+
});

0 commit comments

Comments
 (0)