From 49f1bb9b9366c928b82046695c288699fce012ad Mon Sep 17 00:00:00 2001 From: Luigi Pinca 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 Reviewed-By: Ruben Bridgewater Reviewed-By: James M Snell --- 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(); }