Revert "stream: fix .end() error propagation"

This reverts commit 4c819d6.

PR-URL: #37060
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Robert Nagy <[email protected]>
Reviewed-By: Beth Griggs <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Reviewed-By: Danielle Adams <[email protected]>

Author: mcollina

lib/internal/streams/writable.js

@@ -27,7 +27,6 @@
 
 const {
   FunctionPrototype,
-  Error,
   ObjectDefineProperty,
   ObjectDefineProperties,
   ObjectSetPrototypeOf,
@@ -291,8 +290,8 @@ Writable.prototype.pipe = function() {
   errorOrDestroy(this, new ERR_STREAM_CANNOT_PIPE());
 };
 
-function _write(stream, chunk, encoding, cb) {
-  const state = stream._writableState;
+Writable.prototype.write = function(chunk, encoding, cb) {
+  const state = this._writableState;
 
   if (typeof encoding === 'function') {
     cb = encoding;
@@ -334,15 +333,11 @@ function _write(stream, chunk, encoding, cb) {
 
   if (err) {
     process.nextTick(cb, err);
-    errorOrDestroy(stream, err, true);
-    return err;
+    errorOrDestroy(this, err, true);
+    return false;
   }
   state.pendingcb++;
-  return writeOrBuffer(stream, state, chunk, encoding, cb);
-}
-
-Writable.prototype.write = function(chunk, encoding, cb) {
-  return _write(this, chunk, encoding, cb) === true;
+  return writeOrBuffer(this, state, chunk, encoding, cb);
 };
 
 Writable.prototype.cork = function() {
@@ -612,30 +607,21 @@ Writable.prototype.end = function(chunk, encoding, cb) {
     encoding = null;
   }
 
-  let err;
-
-  if (chunk !== null && chunk !== undefined) {
-    const ret = _write(this, chunk, encoding);
-    if (ret instanceof Error) {
-      err = ret;
-    }
-  }
+  if (chunk !== null && chunk !== undefined)
+    this.write(chunk, encoding);
 
   // .end() fully uncorks.
   if (state.corked) {
     state.corked = 1;
     this.uncork();
   }
 
-  if (err) {
-    // Do nothing...
-  } else if (!state.errored && !state.ending) {
-    // 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.
-
+  // 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.
+  let err;
+  if (!state.errored && !state.ending) {
     state.ending = true;
     finishMaybe(this, state, true);
     state.ended = true;

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

@@ -46,38 +46,3 @@ const stream = require('stream');
     writable.emit('error', new Error('kaboom'));
   }));
 }
-
-{
-  const w = new stream.Writable({
-    write(chunk, encoding, callback) {
-      setImmediate(callback);
-    },
-    finish(callback) {
-      setImmediate(callback);
-    }
-  });
-  w.end('testing ended state', common.mustCall((err) => {
-    // This errors since .destroy(err), which is invoked by errors
-    // in same tick below, will error all pending callbacks.
-    // Does this make sense? Not sure.
-    assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED');
-  }));
-  assert.strictEqual(w.destroyed, false);
-  assert.strictEqual(w.writableEnded, true);
-  w.end(common.mustCall((err) => {
-    // This errors since .destroy(err), which is invoked by errors
-    // in same tick below, will error all pending callbacks.
-    // Does this make sense? Not sure.
-    assert.strictEqual(err.code, 'ERR_STREAM_DESTROYED');
-  }));
-  assert.strictEqual(w.destroyed, false);
-  assert.strictEqual(w.writableEnded, true);
-  w.end('end', common.mustCall((err) => {
-    assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END');
-  }));
-  assert.strictEqual(w.destroyed, true);
-  w.on('error', common.mustCall((err) => {
-    assert.strictEqual(err.code, 'ERR_STREAM_WRITE_AFTER_END');
-  }));
-  w.on('finish', common.mustNotCall());
-}