Skip to content

Commit 57e59ea

Browse files
Han5991aduh95
authored andcommitted
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 Signed-off-by: sangwook <rewq5991@gmail.com> PR-URL: #62297 Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
1 parent d1c517f commit 57e59ea

2 files changed

Lines changed: 59 additions & 6 deletions

File tree

‎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
functiondone(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
functiondone(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+
constcommon=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+
constoutput=Duplex.fromWeb(newTransformStream());
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+
constwritable=Writable.fromWeb(newWritableStream());
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+
constwritable=Writable.fromWeb(newWritableStream({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)