Skip to content

Commit de63904

Browse files
committed
lib: fix unhandled errors in webstream adapters
WebStream's Readable controller does not tolerate `.close()` being called after an `error`. However, when wrapping a Node's Readable stream it is possible that the sequence of events leads to `finished()`'s callback being invoked after such `error`. In order to handle this, in this change we call the `finished()` handler earlier when controller is canceled, and always handle this as an error case. Fix: #54205
1 parent 67f7137 commit de63904

File tree

3 files changed

+33
-2
lines changed

3 files changed

+33
-2
lines changed

lib/internal/webstreams/adapters.js

+6-2
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ const {
6767
createDeferredPromise,
6868
kEmptyObject,
6969
normalizeEncoding,
70+
once,
7071
} = require('internal/util');
7172

7273
const {
@@ -471,7 +472,7 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
471472

472473
streamReadable.pause();
473474

474-
const cleanup = finished(streamReadable, (error) => {
475+
const onFinished = once((error) => {
475476
error = handleKnownInternalErrors(error);
476477

477478
cleanup();
@@ -482,6 +483,7 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
482483
return controller.error(error);
483484
controller.close();
484485
});
486+
const cleanup = finished(streamReadable, onFinished);
485487

486488
streamReadable.on('data', onData);
487489

@@ -491,7 +493,9 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
491493
pull() { streamReadable.resume(); },
492494

493495
cancel(reason) {
494-
destroy(streamReadable, reason);
496+
const error = reason || new ERR_STREAM_PREMATURE_CLOSE();
497+
onFinished(error);
498+
destroy(streamReadable, error);
495499
},
496500
}, strategy);
497501
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
'use strict';
2+
require('../common');
3+
const { Readable } = require('stream');
4+
5+
{
6+
const r = Readable.from(['data']);
7+
8+
const wrapper = Readable.fromWeb(Readable.toWeb(r));
9+
10+
wrapper.on('data', () => {
11+
// Destroying wrapper while emitting data should not cause uncaught
12+
// exceptions
13+
wrapper.destroy();
14+
});
15+
}
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
'use strict';
2+
require('../common');
3+
const { Readable } = require('stream');
4+
5+
{
6+
const r = Readable.from([]);
7+
// Cancelling reader while closing should not cause uncaught exceptions
8+
r.on('close', () => reader.cancel());
9+
10+
const reader = Readable.toWeb(r).getReader();
11+
reader.read();
12+
}

0 commit comments

Comments
 (0)