Skip to content

Commit 7ce5532

Browse files
committed
stream: fix writev unhandled rejection in fromWeb
When using Duplex.fromWeb() or Writable.fromWeb() with cork()/uncork(), writes are batched into _writev(). If destroy() is called in the same microtask, the underlying WritableStream writer gets aborted, causing SafePromiseAll() to reject with a non-array value (e.g. an AbortError). The done() callback in _writev() of both fromWeb adapter functions unconditionally called error.filter(), assuming the value was always an array. This caused a TypeError that became an unhandled rejection, crashing the process. Fix by separating the resolve and reject handlers of SafePromiseAll: use () => done() on the resolve path (all writes succeeded, no error) and done on the reject path (error passed directly to callback). Fixes: #62199
1 parent bf1aebc commit 7ce5532

File tree

2 files changed

+59
-6
lines changed

2 files changed

+59
-6
lines changed

lib/internal/webstreams/adapters.js

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -314,9 +314,8 @@ function newStreamWritableFromWritableStream(writableStream, options = kEmptyObj
314314

315315
writev(chunks, callback) {
316316
function done(error) {
317-
error = error.filter((e) => e);
318317
try {
319-
callback(error.length === 0 ? undefined : error);
318+
callback(error);
320319
} catch (error) {
321320
// In a next tick because this is happening within
322321
// a promise context, and if there are any errors
@@ -334,7 +333,7 @@ function newStreamWritableFromWritableStream(writableStream, options = kEmptyObj
334333
SafePromiseAll(
335334
chunks,
336335
(data) => writer.write(data.chunk)),
337-
done,
336+
() => done(),
338337
done);
339338
},
340339
done);
@@ -787,9 +786,8 @@ function newStreamDuplexFromReadableWritablePair(pair = kEmptyObject, options =
787786

788787
writev(chunks, callback) {
789788
function done(error) {
790-
error = error.filter((e) => e);
791789
try {
792-
callback(error.length === 0 ? undefined : error);
790+
callback(error);
793791
} catch (error) {
794792
// In a next tick because this is happening within
795793
// a promise context, and if there are any errors
@@ -807,7 +805,7 @@ function newStreamDuplexFromReadableWritablePair(pair = kEmptyObject, options =
807805
SafePromiseAll(
808806
chunks,
809807
(data) => writer.write(data.chunk)),
810-
done,
808+
() => done(),
811809
done);
812810
},
813811
done);
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
'use strict';
2+
3+
// Regression test for https://github.com/nodejs/node/issues/62199
4+
//
5+
// When Duplex.fromWeb is corked, writes are batched into _writev. If destroy()
6+
// is called in the same microtask (after uncork()), writer.ready rejects with a
7+
// non-array value. The done() callback inside _writev unconditionally called
8+
// error.filter(), which throws TypeError on non-arrays. This TypeError became
9+
// an unhandled rejection that crashed the process.
10+
//
11+
// The same bug exists in newStreamWritableFromWritableStream (Writable.fromWeb).
12+
13+
const common = require('../common');
14+
const { Duplex, Writable } = require('stream');
15+
const { TransformStream, WritableStream } = require('stream/web');
16+
17+
// Exact reproduction from the issue report (davidje13).
18+
// Before the fix: process crashes with unhandled TypeError.
19+
// After the fix: stream closes cleanly with no unhandled rejection.
20+
{
21+
const output = Duplex.fromWeb(new TransformStream());
22+
23+
output.on('close', common.mustCall());
24+
25+
output.cork();
26+
output.write('test');
27+
output.write('test');
28+
output.uncork();
29+
output.destroy();
30+
}
31+
32+
// Same bug in Writable.fromWeb (newStreamWritableFromWritableStream).
33+
{
34+
const writable = Writable.fromWeb(new WritableStream());
35+
36+
writable.on('close', common.mustCall());
37+
38+
writable.cork();
39+
writable.write('test');
40+
writable.write('test');
41+
writable.uncork();
42+
writable.destroy();
43+
}
44+
45+
// Regression: normal cork/uncork/_writev success path must still work.
46+
// Verifies that () => done() correctly signals success via callback().
47+
{
48+
const writable = Writable.fromWeb(new WritableStream({ write() {} }));
49+
50+
writable.cork();
51+
writable.write('foo');
52+
writable.write('bar');
53+
writable.uncork();
54+
writable.end(common.mustCall());
55+
}

0 commit comments

Comments
 (0)