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: #26057 Fixes: #26015 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
nodejs · Mar 12, 2019 · 82a68ce · 82a68ce
1 parent 7e40ce1
commit 82a68ce
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 5 deletions.
diff --git 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
@@ -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(); }