Skip to content

Commit ad4c10e

Browse files
ronagMylesBorins
authored andcommitted
stream: improve comments regarding end() errors
Cleans up comments and TODOs and tries to provide a more detail description in regards to error behavior of end(). PR-URL: #32839 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent 6e5c23b commit ad4c10e

File tree

1 file changed

+7
-10
lines changed

1 file changed

+7
-10
lines changed

lib/_stream_writable.js

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -591,20 +591,16 @@ Writable.prototype.end = function(chunk, encoding, cb) {
591591
if (typeof cb !== 'function')
592592
cb = nop;
593593

594-
// Ignore unnecessary end() calls.
595-
// TODO(ronag): Compat. Allow end() after destroy().
594+
// This is forgiving in terms of unnecessary calls to end() and can hide
595+
// logic errors. However, usually such errors are harmless and causing a
596+
// hard error can be disproportionately destructive. It is not always
597+
// trivial for the user to determine whether end() needs to be called or not.
596598
if (!state.errored && !state.ending) {
597599
endWritable(this, state, cb);
598600
} else if (state.finished) {
599-
const err = new ERR_STREAM_ALREADY_FINISHED('end');
600-
process.nextTick(cb, err);
601-
// TODO(ronag): Compat. Don't error the stream.
602-
// errorOrDestroy(this, err, true);
601+
process.nextTick(cb, new ERR_STREAM_ALREADY_FINISHED('end'));
603602
} else if (state.destroyed) {
604-
const err = new ERR_STREAM_DESTROYED('end');
605-
process.nextTick(cb, err);
606-
// TODO(ronag): Compat. Don't error the stream.
607-
// errorOrDestroy(this, err, true);
603+
process.nextTick(cb, new ERR_STREAM_DESTROYED('end'));
608604
} else if (cb !== nop) {
609605
onFinished(this, state, cb);
610606
}
@@ -713,6 +709,7 @@ function onCorkedFinish(corkReq, state, err) {
713709
state.corkedRequestsFree.next = corkReq;
714710
}
715711

712+
// TODO(ronag): Avoid using events to implement internal logic.
716713
function onFinished(stream, state, cb) {
717714
function onerror(err) {
718715
stream.removeListener('finish', onfinish);

0 commit comments

Comments
 (0)