Skip to content

Commit f408536

Browse files
indutnytargos
authored andcommitted
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: #54206Fixes: #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 62bf03b commit f408536

3 files changed

Lines changed: 33 additions & 0 deletions

File tree

‎lib/internal/webstreams/adapters.js‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -424,6 +424,7 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
424424
conststrategy=evaluateStrategyOrFallback(options?.strategy);
425425

426426
letcontroller;
427+
letwasCanceled=false;
427428

428429
functiononData(chunk){
429430
// Copy the Buffer to detach it from the pool.
@@ -448,6 +449,10 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
448449
streamReadable.on('error',()=>{});
449450
if(error)
450451
returncontroller.error(error);
452+
// Was already canceled
453+
if(wasCanceled){
454+
return;
455+
}
451456
controller.close();
452457
});
453458

@@ -459,6 +464,7 @@ function newReadableStreamFromStreamReadable(streamReadable, options = kEmptyObj
459464
pull(){streamReadable.resume();},
460465

461466
cancel(reason){
467+
wasCanceled=true;
462468
destroy(streamReadable,reason);
463469
},
464470
},strategy);
Lines changed: 15 additions & 0 deletions
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+
constr=Readable.from(['data']);
7+
8+
constwrapper=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+
}
Lines changed: 12 additions & 0 deletions
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+
constr=Readable.from([]);
7+
// Cancelling reader while closing should not cause uncaught exceptions
8+
r.on('close',()=>reader.cancel());
9+
10+
constreader=Readable.toWeb(r).getReader();
11+
reader.read();
12+
}

0 commit comments

Comments
 (0)