Skip to content

Commit efd40ea

Browse files
committed
stream: forward errored to callback
Refs: #39356 PR-URL: #39364 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent 8306051 commit efd40ea

4 files changed

Lines changed: 36 additions & 22 deletions

File tree

‎lib/internal/streams/writable.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -515,12 +515,12 @@ function errorBuffer(state) {
515515
const{ chunk, callback }=state.buffered[n];
516516
constlen=state.objectMode ? 1 : chunk.length;
517517
state.length-=len;
518-
callback(newERR_STREAM_DESTROYED('write'));
518+
callback(state.errored??newERR_STREAM_DESTROYED('write'));
519519
}
520520

521521
constonfinishCallbacks=state[kOnFinished].splice(0);
522522
for(leti=0;i<onfinishCallbacks.length;i++){
523-
onfinishCallbacks[i](newERR_STREAM_DESTROYED('end'));
523+
onfinishCallbacks[i](state.errored??newERR_STREAM_DESTROYED('end'));
524524
}
525525

526526
resetBuffer(state);

‎test/parallel/test-stream-writable-destroy.js‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -351,33 +351,35 @@ const assert = require('assert');
351351
constwrite=newWritable({
352352
write(chunk,enc,cb){process.nextTick(cb);}
353353
});
354+
const_err=newError('asd');
354355
write.once('error',common.mustCall((err)=>{
355356
assert.strictEqual(err.message,'asd');
356357
}));
357358
write.end('asd',common.mustCall((err)=>{
358-
assert.strictEqual(err.code,'ERR_STREAM_DESTROYED');
359+
assert.strictEqual(err,_err);
359360
}));
360-
write.destroy(newError('asd'));
361+
write.destroy(_err);
361362
}
362363

363364
{
364365
// Call buffered write callback with error
365366

367+
const_err=newError('asd');
366368
constwrite=newWritable({
367369
write(chunk,enc,cb){
368-
process.nextTick(cb,newError('asd'));
370+
process.nextTick(cb,_err);
369371
},
370372
autoDestroy: false
371373
});
372374
write.cork();
373375
write.write('asd',common.mustCall((err)=>{
374-
assert.strictEqual(err.message,'asd');
376+
assert.strictEqual(err,_err);
375377
}));
376378
write.write('asd',common.mustCall((err)=>{
377-
assert.strictEqual(err.code,'ERR_STREAM_DESTROYED');
379+
assert.strictEqual(err,_err);
378380
}));
379381
write.on('error',common.mustCall((err)=>{
380-
assert.strictEqual(err.message,'asd');
382+
assert.strictEqual(err,_err);
381383
}));
382384
write.uncork();
383385
}
@@ -471,3 +473,19 @@ const assert = require('assert');
471473
write.destroy();
472474
write.destroy();
473475
}
476+
477+
{
478+
// https://github.com/nodejs/node/issues/39356
479+
consts=newWritable({
480+
final(){}
481+
});
482+
const_err=newError('oh no');
483+
// Remove `callback` and it works
484+
s.end(common.mustCall((err)=>{
485+
assert.strictEqual(err,_err);
486+
}));
487+
s.on('error',common.mustCall((err)=>{
488+
assert.strictEqual(err,_err);
489+
}));
490+
s.destroy(_err);
491+
}

‎test/parallel/test-stream-writable-end-cb-error.js‎

Lines changed: 7 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -8,19 +8,20 @@ const stream = require('stream');
88
// Invoke end callback on failure.
99
constwritable=newstream.Writable();
1010

11+
const_err=newError('kaboom');
1112
writable._write=(chunk,encoding,cb)=>{
12-
process.nextTick(cb,newError('kaboom'));
13+
process.nextTick(cb,_err);
1314
};
1415

1516
writable.on('error',common.mustCall((err)=>{
16-
assert.strictEqual(err.message,'kaboom');
17+
assert.strictEqual(err,_err);
1718
}));
1819
writable.write('asd');
1920
writable.end(common.mustCall((err)=>{
20-
assert.strictEqual(err.code,'ERR_STREAM_DESTROYED');
21+
assert.strictEqual(err,_err);
2122
}));
2223
writable.end(common.mustCall((err)=>{
23-
assert.strictEqual(err.code,'ERR_STREAM_DESTROYED');
24+
assert.strictEqual(err,_err);
2425
}));
2526
}
2627

@@ -57,18 +58,12 @@ const stream = require('stream');
5758
}
5859
});
5960
w.end('testing ended state',common.mustCall((err)=>{
60-
// This errors since .destroy(err), which is invoked by errors
61-
// in same tick below, will error all pending callbacks.
62-
// Does this make sense? Not sure.
63-
assert.strictEqual(err.code,'ERR_STREAM_DESTROYED');
61+
assert.strictEqual(err.code,'ERR_STREAM_WRITE_AFTER_END');
6462
}));
6563
assert.strictEqual(w.destroyed,false);
6664
assert.strictEqual(w.writableEnded,true);
6765
w.end(common.mustCall((err)=>{
68-
// This errors since .destroy(err), which is invoked by errors
69-
// in same tick below, will error all pending callbacks.
70-
// Does this make sense? Not sure.
71-
assert.strictEqual(err.code,'ERR_STREAM_DESTROYED');
66+
assert.strictEqual(err.code,'ERR_STREAM_WRITE_AFTER_END');
7267
}));
7368
assert.strictEqual(w.destroyed,false);
7469
assert.strictEqual(w.writableEnded,true);

‎test/parallel/test-stream-writable-end-cb-uncaught.js‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,16 @@ process.on('uncaughtException', common.mustCall((err) => {
99
}));
1010

1111
constwritable=newstream.Writable();
12+
const_err=newError('kaboom');
1213

1314
writable._write=(chunk,encoding,cb)=>{
1415
cb();
1516
};
1617
writable._final=(cb)=>{
17-
cb(newError('kaboom'));
18+
cb(_err);
1819
};
1920

2021
writable.write('asd');
2122
writable.end(common.mustCall((err)=>{
22-
assert.strictEqual(err.code,'ERR_STREAM_DESTROYED');
23+
assert.strictEqual(err,_err);
2324
}));

0 commit comments

Comments
 (0)