Skip to content

Commit 783322f

Browse files
indutnytargos
authored andcommittedAug 14, 2024
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 PR-URL: #54206 Fixes: #54205 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Mattias Buelens <mattias@buelens.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent e3e2f22 commit 783322f

File tree

3 files changed

+33
-0
lines changed

3 files changed

+33
-0
lines changed
 

‎lib/internal/webstreams/adapters.js

+6
Original file line numberDiff line numberDiff line change
@@ -459,6 +459,7 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
459459
const strategy = evaluateStrategyOrFallback(options?.strategy);
460460

461461
let controller;
462+
let wasCanceled = false;
462463

463464
function onData(chunk) {
464465
// Copy the Buffer to detach it from the pool.
@@ -480,6 +481,10 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
480481
streamReadable.on('error', () => {});
481482
if (error)
482483
return controller.error(error);
484+
// Was already canceled
485+
if (wasCanceled) {
486+
return;
487+
}
483488
controller.close();
484489
});
485490

@@ -491,6 +496,7 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
491496
pull() { streamReadable.resume(); },
492497

493498
cancel(reason) {
499+
wasCanceled = true;
494500
destroy(streamReadable, reason);
495501
},
496502
}, strategy);
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)
Please sign in to comment.