From 82a68cebe35fca37f4d61bad4c8ed82b361e5815 Mon Sep 17 00:00:00 2001
From: Luigi Pinca <luigipinca@gmail.com>
Date: Tue, 12 Feb 2019 19:24:39 +0100
Subject: [PATCH] stream: ensure writable.destroy() emits error once

Prevent the `'error'` event from being emitted multiple times if
`writable.destroy()` is called with an error before the `_destroy()`
callback is called.

Emit the first error, discard all others.

PR-URL: https://github.com/nodejs/node/pull/26057
Fixes: https://github.com/nodejs/node/issues/26015
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
---
 lib/internal/streams/destroy.js               | 19 ++++++++++----
 test/parallel/test-stream-writable-destroy.js | 26 +++++++++++++++++++
 2 files changed, 40 insertions(+), 5 deletions(-)

diff --git a/lib/internal/streams/destroy.js b/lib/internal/streams/destroy.js
index 0c652be9dd0b4d..200c75459ad56d 100644
--- a/lib/internal/streams/destroy.js
+++ b/lib/internal/streams/destroy.js
@@ -10,10 +10,15 @@ function destroy(err, cb) {
   if (readableDestroyed || writableDestroyed) {
     if (cb) {
       cb(err);
-    } else if (err &&
-               (!this._writableState || !this._writableState.errorEmitted)) {
-      process.nextTick(emitErrorNT, this, err);
+    } else if (err) {
+      if (!this._writableState) {
+        process.nextTick(emitErrorNT, this, err);
+      } else if (!this._writableState.errorEmitted) {
+        this._writableState.errorEmitted = true;
+        process.nextTick(emitErrorNT, this, err);
+      }
     }
+
     return this;
   }
 
@@ -31,9 +36,13 @@ function destroy(err, cb) {
 
   this._destroy(err || null, (err) => {
     if (!cb && err) {
-      process.nextTick(emitErrorAndCloseNT, this, err);
-      if (this._writableState) {
+      if (!this._writableState) {
+        process.nextTick(emitErrorAndCloseNT, this, err);
+      } else if (!this._writableState.errorEmitted) {
         this._writableState.errorEmitted = true;
+        process.nextTick(emitErrorAndCloseNT, this, err);
+      } else {
+        process.nextTick(emitCloseNT, this);
       }
     } else if (cb) {
       process.nextTick(emitCloseNT, this);
diff --git a/test/parallel/test-stream-writable-destroy.js b/test/parallel/test-stream-writable-destroy.js
index 867571ef377a93..56e17990794a79 100644
--- a/test/parallel/test-stream-writable-destroy.js
+++ b/test/parallel/test-stream-writable-destroy.js
@@ -152,6 +152,32 @@ const assert = require('assert');
   assert.strictEqual(write.destroyed, true);
 }
 
+{
+  const writable = new Writable({
+    destroy: common.mustCall(function(err, cb) {
+      process.nextTick(cb, new Error('kaboom 1'));
+    }),
+    write(chunk, enc, cb) {
+      cb();
+    }
+  });
+
+  writable.on('close', common.mustCall());
+  writable.on('error', common.expectsError({
+    type: Error,
+    message: 'kaboom 2'
+  }));
+
+  writable.destroy();
+  assert.strictEqual(writable.destroyed, true);
+  assert.strictEqual(writable._writableState.errorEmitted, false);
+
+  // 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);
+}
+
 {
   const write = new Writable({
     write(chunk, enc, cb) { cb(); }