GeezerVison is showing github.com — stream: fix writev unhandled rejection in fromWeb · nodejs/node@3ed7835
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 |
|---|
@@ -313,9 +313,8 @@ function newStreamWritableFromWritableStream(writableStream, options = kEmptyObj | 313 | 313 |
| 314 | 314 | writev(chunks, callback) { | 315 | 315 | function done(error) { | 316 | | - error = error.filter((e) => e); | 317 | 316 | try { | 318 | | - callback(error.length === 0 ? undefined : error); | | 317 | + callback(error); | 319 | 318 | } catch (error) { | 320 | 319 | // In a next tick because this is happening within | 321 | 320 | // a promise context, and if there are any errors | @@ -333,7 +332,7 @@ function newStreamWritableFromWritableStream(writableStream, options = kEmptyObj | 333 | 332 | SafePromiseAll( | 334 | 333 | chunks, | 335 | 334 | (data) => writer.write(data.chunk)), | 336 | | - done, | | 335 | + () => done(), | 337 | 336 | done); | 338 | 337 | }, | 339 | 338 | done); | @@ -775,9 +774,8 @@ function newStreamDuplexFromReadableWritablePair(pair = kEmptyObject, options = | 775 | 774 |
| 776 | 775 | writev(chunks, callback) { | 777 | 776 | function done(error) { | 778 | | - error = error.filter((e) => e); | 779 | 777 | try { | 780 | | - callback(error.length === 0 ? undefined : error); | | 778 | + callback(error); | 781 | 779 | } catch (error) { | 782 | 780 | // In a next tick because this is happening within | 783 | 781 | // a promise context, and if there are any errors | @@ -795,7 +793,7 @@ function newStreamDuplexFromReadableWritablePair(pair = kEmptyObject, options = | 795 | 793 | SafePromiseAll( | 796 | 794 | chunks, | 797 | 795 | (data) => writer.write(data.chunk)), | 798 | | - done, | | 796 | + () => done(), | 799 | 797 | done); | 800 | 798 | }, | 801 | 799 | 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 | +} |
|