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 <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 files changed, 7 insertions, 10 deletions

diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js
index 5368bbef35..25235166d9 100644
--- a/lib/_stream_writable.js
+++ b/lib/_stream_writable.js
@@ -598,20 +598,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);
   }
@@ -720,6 +716,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);