Skip to content

Commit 07360bc

Browse files
author
easonysliu
committed
stream: fix TypeError in writev when writer.ready rejects
The `done` callback in the `writev` handlers of both `newStreamWritableFromWritableStream` and `newStreamDuplexFromReadableWritablePair` unconditionally called `error.filter()`, assuming `error` is always an array from `SafePromiseAll`. However, `done` is also used as the rejection handler for `writer.ready`, which passes a single error value, not an array. This caused a `TypeError: error.filter is not a function` which became an unhandled rejection, crashing the process. Fix by checking whether `error` is an array before filtering, and also use `ArrayIsArray`/`ArrayPrototypeFilter` from primordials to follow Node.js internal conventions. Fixes: #62199
1 parent a62f641 commit 07360bc

File tree

2 files changed

+73
-4
lines changed

2 files changed

+73
-4
lines changed

lib/internal/webstreams/adapters.js

Lines changed: 17 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const {
4+
ArrayIsArray,
45
ArrayPrototypeFilter,
56
ArrayPrototypeMap,
67
Boolean,
@@ -313,9 +314,15 @@ function newStreamWritableFromWritableStream(writableStream, options = kEmptyObj
313314

314315
writev(chunks, callback) {
315316
function done(error) {
316-
error = error.filter((e) => e);
317+
// When error comes from SafePromiseAll, it is an array of
318+
// errors (one per chunk). When it comes from writer.ready
319+
// rejection, it is a single error. Normalize to handle both.
320+
if (ArrayIsArray(error)) {
321+
error = ArrayPrototypeFilter(error, Boolean);
322+
error = error.length === 0 ? undefined : error;
323+
}
317324
try {
318-
callback(error.length === 0 ? undefined : error);
325+
callback(error);
319326
} catch (error) {
320327
// In a next tick because this is happening within
321328
// a promise context, and if there are any errors
@@ -775,9 +782,15 @@ function newStreamDuplexFromReadableWritablePair(pair = kEmptyObject, options =
775782

776783
writev(chunks, callback) {
777784
function done(error) {
778-
error = error.filter((e) => e);
785+
// When error comes from SafePromiseAll, it is an array of
786+
// errors (one per chunk). When it comes from writer.ready
787+
// rejection, it is a single error. Normalize to handle both.
788+
if (ArrayIsArray(error)) {
789+
error = ArrayPrototypeFilter(error, Boolean);
790+
error = error.length === 0 ? undefined : error;
791+
}
779792
try {
780-
callback(error.length === 0 ? undefined : error);
793+
callback(error);
781794
} catch (error) {
782795
// In a next tick because this is happening within
783796
// a promise context, and if there are any errors
Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
// Flags: --no-warnings --expose-internals
2+
'use strict';
3+
4+
// Regression test for https://github.com/nodejs/node/issues/62199
5+
// Verifies that the writev done callback in webstream adapters does not
6+
// throw a TypeError when the error comes from writer.ready rejection
7+
// (a single error) rather than SafePromiseAll (an array of errors).
8+
9+
const common = require('../common');
10+
11+
const {
12+
TransformStream,
13+
WritableStream,
14+
} = require('stream/web');
15+
16+
const {
17+
newStreamWritableFromWritableStream,
18+
newStreamDuplexFromReadableWritablePair,
19+
} = require('internal/webstreams/adapters');
20+
21+
const { Duplex } = require('stream');
22+
23+
// Test 1: Duplex.fromWeb writev path should not cause unhandled rejection
24+
{
25+
const output = Duplex.fromWeb(new TransformStream());
26+
output.on('error', common.mustCall());
27+
output.cork();
28+
output.write('test');
29+
output.write('test');
30+
output.uncork();
31+
output.destroy();
32+
}
33+
34+
// Test 2: newStreamDuplexFromReadableWritablePair writev path
35+
{
36+
const transform = new TransformStream();
37+
const duplex = newStreamDuplexFromReadableWritablePair(transform);
38+
duplex.on('error', common.mustCall());
39+
duplex.cork();
40+
duplex.write('test');
41+
duplex.write('test');
42+
duplex.uncork();
43+
duplex.destroy();
44+
}
45+
46+
// Test 3: newStreamWritableFromWritableStream writev path
47+
{
48+
const writableStream = new WritableStream();
49+
const writable = newStreamWritableFromWritableStream(writableStream);
50+
writable.on('error', common.mustCall());
51+
writable.cork();
52+
writable.write('test');
53+
writable.write('test');
54+
writable.uncork();
55+
writable.destroy();
56+
}

0 commit comments

Comments
 (0)