Skip to content

Commit d2a487e

Browse files
mcollinadanielleadams
authored andcommitted
Revert "stream: fix .end() error propagation"
This reverts commit 4c819d6. PR-URL: #37060 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Robert Nagy <ronagy@icloud.com> Reviewed-By: Beth Griggs <bgriggs@redhat.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Danielle Adams <adamzdanielle@gmail.com>
1 parent 2680979 commit d2a487e

2 files changed

Lines changed: 13 additions & 62 deletions

File tree

‎lib/internal/streams/writable.js‎

Lines changed: 13 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727

2828
const{
2929
FunctionPrototype,
30-
Error,
3130
ObjectDefineProperty,
3231
ObjectDefineProperties,
3332
ObjectSetPrototypeOf,
@@ -291,8 +290,8 @@ Writable.prototype.pipe = function() {
291290
errorOrDestroy(this,newERR_STREAM_CANNOT_PIPE());
292291
};
293292

294-
function_write(stream,chunk,encoding,cb){
295-
conststate=stream._writableState;
293+
Writable.prototype.write=function(chunk,encoding,cb){
294+
conststate=this._writableState;
296295

297296
if(typeofencoding==='function'){
298297
cb=encoding;
@@ -334,15 +333,11 @@ function _write(stream, chunk, encoding, cb) {
334333

335334
if(err){
336335
process.nextTick(cb,err);
337-
errorOrDestroy(stream,err,true);
338-
returnerr;
336+
errorOrDestroy(this,err,true);
337+
returnfalse;
339338
}
340339
state.pendingcb++;
341-
returnwriteOrBuffer(stream,state,chunk,encoding,cb);
342-
}
343-
344-
Writable.prototype.write=function(chunk,encoding,cb){
345-
return_write(this,chunk,encoding,cb)===true;
340+
returnwriteOrBuffer(this,state,chunk,encoding,cb);
346341
};
347342

348343
Writable.prototype.cork=function(){
@@ -612,30 +607,21 @@ Writable.prototype.end = function(chunk, encoding, cb) {
612607
encoding=null;
613608
}
614609

615-
leterr;
616-
617-
if(chunk!==null&&chunk!==undefined){
618-
constret=_write(this,chunk,encoding);
619-
if(retinstanceofError){
620-
err=ret;
621-
}
622-
}
610+
if(chunk!==null&&chunk!==undefined)
611+
this.write(chunk,encoding);
623612

624613
// .end() fully uncorks.
625614
if(state.corked){
626615
state.corked=1;
627616
this.uncork();
628617
}
629618

630-
if(err){
631-
// Do nothing...
632-
}elseif(!state.errored&&!state.ending){
633-
// This is forgiving in terms of unnecessary calls to end() and can hide
634-
// logic errors. However, usually such errors are harmless and causing a
635-
// hard error can be disproportionately destructive. It is not always
636-
// trivial for the user to determine whether end() needs to be called
637-
// or not.
638-
619+
// This is forgiving in terms of unnecessary calls to end() and can hide
620+
// logic errors. However, usually such errors are harmless and causing a
621+
// hard error can be disproportionately destructive. It is not always
622+
// trivial for the user to determine whether end() needs to be called or not.
623+
leterr;
624+
if(!state.errored&&!state.ending){
639625
state.ending=true;
640626
finishMaybe(this,state,true);
641627
state.ended=true;

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

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -46,38 +46,3 @@ const stream = require('stream');
4646
writable.emit('error',newError('kaboom'));
4747
}));
4848
}
49-
50-
{
51-
constw=newstream.Writable({
52-
write(chunk,encoding,callback){
53-
setImmediate(callback);
54-
},
55-
finish(callback){
56-
setImmediate(callback);
57-
}
58-
});
59-
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');
64-
}));
65-
assert.strictEqual(w.destroyed,false);
66-
assert.strictEqual(w.writableEnded,true);
67-
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');
72-
}));
73-
assert.strictEqual(w.destroyed,false);
74-
assert.strictEqual(w.writableEnded,true);
75-
w.end('end',common.mustCall((err)=>{
76-
assert.strictEqual(err.code,'ERR_STREAM_WRITE_AFTER_END');
77-
}));
78-
assert.strictEqual(w.destroyed,true);
79-
w.on('error',common.mustCall((err)=>{
80-
assert.strictEqual(err.code,'ERR_STREAM_WRITE_AFTER_END');
81-
}));
82-
w.on('finish',common.mustNotCall());
83-
}

0 commit comments

Comments
 (0)