stream: don't emit errors after destroy

- Don't emit any error on destroy().
- Emit error on destroy(err).
- Always pass any error to callbak in destroy(err, cb).

Author: ronag

lib/internal/streams/destroy.js

@@ -29,11 +29,8 @@ function destroy(err, cb) {
 
   if ((w && w.destroyed) || (r && r.destroyed)) {
     if (cb) {
-      cb(err);
-    } else if (needError(this, err)) {
-      process.nextTick(emitErrorNT, this, err);
+      cb(err || null);
     }
-
     return this;
   }
 
@@ -47,11 +44,11 @@ function destroy(err, cb) {
     r.destroyed = true;
   }
 
-  this._destroy(err || null, (err) => {
+  this._destroy(err || null, (er) => {
     if (cb) {
-      process.nextTick(emitCloseNT, this);
-      cb(err);
-    } else if (needError(this, err)) {
+      cb(er || null);
+    }
+    if (err && needError(this, er || null)) {
       process.nextTick(emitErrorAndCloseNT, this, err);
     } else {
       process.nextTick(emitCloseNT, this);
@@ -114,6 +111,11 @@ function errorOrDestroy(stream, err) {
   const r = stream._readableState;
   const w = stream._writableState;
 
+  // Don't emit errors if already destroyed.
+  if ((w && w.destroyed) || (r && r.destroyed)) {
+    return;
+  }
+
   if ((r && r.autoDestroy) || (w && w.autoDestroy))
     stream.destroy(err);
   else if (needError(stream, err))

test/parallel/test-stream-duplex-destroy.js

@@ -144,9 +144,7 @@ const assert = require('assert');
 
   duplex.on('finish', common.mustNotCall('no finish event'));
   duplex.on('end', common.mustNotCall('no end event'));
-  duplex.on('error', common.mustCall((err) => {
-    assert.strictEqual(err, expected);
-  }));
+  duplex.on('error', common.mustNotCall());
 
   duplex.destroy();
   assert.strictEqual(duplex.destroyed, true);

test/parallel/test-stream-readable-destroy.js

@@ -130,9 +130,7 @@ const assert = require('assert');
   });
 
   read.on('end', common.mustNotCall('no end event'));
-  read.on('error', common.mustCall((err) => {
-    assert.strictEqual(err, expected);
-  }));
+  read.on('error', common.mustNotCall());
 
   read.destroy();
   assert.strictEqual(read.destroyed, true);
@@ -175,6 +173,7 @@ const assert = require('assert');
   const expected = new Error('kaboom');
 
   read.on('close', common.mustCall());
+  read.on('error', common.mustCall());
   read.destroy(expected, common.mustCall(function(err) {
     assert.strictEqual(err, expected);
   }));
@@ -189,3 +188,29 @@ const assert = require('assert');
   read.push('hi');
   read.on('data', common.mustNotCall());
 }
+
+{
+  const read = new Readable({
+    read() {},
+    destroy: common.mustCall((err, cb) => {
+      cb(new Error('test'));
+    })
+  });
+
+  read.on('error', common.mustNotCall());
+  read.destroy();
+}
+
+{
+  const read = new Readable({
+    read() {},
+    destroy: common.mustCall((err, cb) => {
+      cb(new Error('test'))
+    })
+  });
+
+  read.on('error', common.mustCall(err => {
+    assert.strictEqual(err.message, 'destroyed');
+  }));
+  read.destroy(new Error('destroyed'));
+}

test/parallel/test-stream-transform-destroy.js

@@ -135,9 +135,7 @@ const assert = require('assert');
   transform.on('close', common.mustCall());
   transform.on('finish', common.mustNotCall('no finish event'));
   transform.on('end', common.mustNotCall('no end event'));
-  transform.on('error', common.mustCall((err) => {
-    assert.strictEqual(err, expected);
-  }));
+  transform.on('error', common.mustNotCall());
 
   transform.destroy();
 }

test/parallel/test-stream-writable-destroy.js

@@ -129,9 +129,7 @@ const assert = require('assert');
 
   write.on('close', common.mustCall());
   write.on('finish', common.mustNotCall('no finish event'));
-  write.on('error', common.mustCall((err) => {
-    assert.strictEqual(err, expected);
-  }));
+  write.on('error', common.mustNotCall());
 
   write.destroy();
   assert.strictEqual(write.destroyed, true);
@@ -163,10 +161,7 @@ const assert = require('assert');
   });
 
   writable.on('close', common.mustCall());
-  writable.on('error', common.expectsError({
-    type: Error,
-    message: 'kaboom 2'
-  }));
+  writable.on('error', common.mustNotCall());
 
   writable.destroy();
   assert.strictEqual(writable.destroyed, true);
@@ -175,7 +170,7 @@ const assert = require('assert');
   // Test case where `writable.destroy()` is called again with an error before
   // the `_destroy()` callback is called.
   writable.destroy(new Error('kaboom 2'));
-  assert.strictEqual(writable._writableState.errorEmitted, true);
+  assert.strictEqual(writable._writableState.errorEmitted, false);
 }
 
 {