GeezerVison is showing github.com — stream: fix writev unhandled rejection in fromWeb · nodejs/node@57e59ea
File treeExpand file treeCollapse file tree Open diff view settings Expand file treeCollapse file tree Open diff view settings Collapse file Copy file name to clipboardExpand all lines: lib/internal/webstreams/adapters.js +4-6Lines changed: 4 additions & 6 deletions | Original file line number | Diff line number | Diff line change |
|---|
@@ -314,9 +314,8 @@ function newStreamWritableFromWritableStream(writableStream, options = kEmptyObj | 314 | 314 |
| 315 | 315 | writev(chunks, callback) { | 316 | 316 | function done(error) { | 317 | | - error = error.filter((e) => e); | 318 | 317 | try { | 319 | | - callback(error.length === 0 ? undefined : error); | | 318 | + callback(error); | 320 | 319 | } catch (error) { | 321 | 320 | // In a next tick because this is happening within | 322 | 321 | // a promise context, and if there are any errors | @@ -334,7 +333,7 @@ function newStreamWritableFromWritableStream(writableStream, options = kEmptyObj | 334 | 333 | SafePromiseAll( | 335 | 334 | chunks, | 336 | 335 | (data) => writer.write(data.chunk)), | 337 | | - done, | | 336 | + () => done(), | 338 | 337 | done); | 339 | 338 | }, | 340 | 339 | done); | @@ -787,9 +786,8 @@ function newStreamDuplexFromReadableWritablePair(pair = kEmptyObject, options = | 787 | 786 |
| 788 | 787 | writev(chunks, callback) { | 789 | 788 | function done(error) { | 790 | | - error = error.filter((e) => e); | 791 | 789 | try { | 792 | | - callback(error.length === 0 ? undefined : error); | | 790 | + callback(error); | 793 | 791 | } catch (error) { | 794 | 792 | // In a next tick because this is happening within | 795 | 793 | // a promise context, and if there are any errors | @@ -807,7 +805,7 @@ function newStreamDuplexFromReadableWritablePair(pair = kEmptyObject, options = | 807 | 805 | SafePromiseAll( | 808 | 806 | chunks, | 809 | 807 | (data) => writer.write(data.chunk)), | 810 | | - done, | | 808 | + () => done(), | 811 | 809 | done); | 812 | 810 | }, | 813 | 811 | done); | |
Collapse file Copy file name to clipboard +55Lines changed: 55 additions & 0 deletions | Original file line number | Diff line number | Diff line change |
|---|
| | 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 | +} |
|