Skip to content

Commit 1af4a4a

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 b4ea323 commit 1af4a4a

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
@@ -313,9 +313,8 @@ function newStreamWritableFromWritableStream(writableStream, options = kEmptyObj
313313

314314
writev(chunks, callback) {
315315
function done(error) {
316-
error = error.filter((e) => e);
317316
try {
318-
callback(error.length === 0 ? undefined : error);
317+
callback(error);
319318
} catch (error) {
320319
// In a next tick because this is happening within
321320
// a promise context, and if there are any errors
@@ -333,7 +332,7 @@ function newStreamWritableFromWritableStream(writableStream, options = kEmptyObj
333332
SafePromiseAll(
334333
chunks,
335334
(data) => writer.write(data.chunk)),
336-
done,
335+
() => done(),
337336
done);
338337
},
339338
done);
@@ -775,9 +774,8 @@ function newStreamDuplexFromReadableWritablePair(pair = kEmptyObject, options =
775774

776775
writev(chunks, callback) {
777776
function done(error) {
778-
error = error.filter((e) => e);
779777
try {
780-
callback(error.length === 0 ? undefined : error);
778+
callback(error);
781779
} catch (error) {
782780
// In a next tick because this is happening within
783781
// a promise context, and if there are any errors
@@ -795,7 +793,7 @@ function newStreamDuplexFromReadableWritablePair(pair = kEmptyObject, options =
795793
SafePromiseAll(
796794
chunks,
797795
(data) => writer.write(data.chunk)),
798-
done,
796+
() => done(),
799797
done);
800798
},
801799
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)