Skip to content

Commit

Permalink
stream: ensure writable.destroy() emits error once
Browse files Browse the repository at this point in the history
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: nodejs#26057
Backport-PR-URL: nodejs#28000
Fixes: nodejs#26015
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: James M Snell <jasnell@gmail.com>
  • Loading branch information
lpinca committed Jun 1, 2019
1 parent 07458ba commit fa109c1
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 5 deletions.
17 changes: 12 additions & 5 deletions lib/internal/streams/destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand All @@ -31,9 +36,11 @@ function destroy(err, cb) {

this._destroy(err || null, (err) => {
if (!cb && err) {
process.nextTick(emitErrorNT, this, err);
if (this._writableState) {
if (!this._writableState) {
process.nextTick(emitErrorNT, this, err);
} else if (!this._writableState.errorEmitted) {
this._writableState.errorEmitted = true;
process.nextTick(emitErrorNT, this, err);
}
} else if (cb) {
cb(err);
Expand Down
26 changes: 26 additions & 0 deletions test/parallel/test-stream-writable-destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,32 @@ const { inherits } = require('util');
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.mustNotCall());
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(); }
Expand Down

0 comments on commit fa109c1

Please sign in to comment.