From ad4c10e824a853592a06a9079f140933e5db4c82 Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Tue, 14 Apr 2020 08:26:32 +0200 Subject: [PATCH] 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: https://github.com/nodejs/node/pull/32839 Reviewed-By: Matteo Collina Reviewed-By: Anna Henningsen Reviewed-By: Luigi Pinca Reviewed-By: James M Snell --- lib/_stream_writable.js | 17 +++++++---------- 1 file changed, 7 insertions(+), 10 deletions(-) diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js index c3a7a35d2b3f6f..14a7ede27611c4 100644 --- a/lib/_stream_writable.js +++ b/lib/_stream_writable.js @@ -591,20 +591,16 @@ Writable.prototype.end = function(chunk, encoding, cb) { if (typeof cb !== 'function') cb = nop; - // Ignore unnecessary end() calls. - // TODO(ronag): Compat. Allow end() after destroy(). + // This is forgiving in terms of unnecessary calls to end() and can hide + // logic errors. However, usually such errors are harmless and causing a + // hard error can be disproportionately destructive. It is not always + // trivial for the user to determine whether end() needs to be called or not. if (!state.errored && !state.ending) { endWritable(this, state, cb); } else if (state.finished) { - const err = new ERR_STREAM_ALREADY_FINISHED('end'); - process.nextTick(cb, err); - // TODO(ronag): Compat. Don't error the stream. - // errorOrDestroy(this, err, true); + process.nextTick(cb, new ERR_STREAM_ALREADY_FINISHED('end')); } else if (state.destroyed) { - const err = new ERR_STREAM_DESTROYED('end'); - process.nextTick(cb, err); - // TODO(ronag): Compat. Don't error the stream. - // errorOrDestroy(this, err, true); + process.nextTick(cb, new ERR_STREAM_DESTROYED('end')); } else if (cb !== nop) { onFinished(this, state, cb); } @@ -713,6 +709,7 @@ function onCorkedFinish(corkReq, state, err) { state.corkedRequestsFree.next = corkReq; } +// TODO(ronag): Avoid using events to implement internal logic. function onFinished(stream, state, cb) { function onerror(err) { stream.removeListener('finish', onfinish);