Skip to content

Commit 3ee076f

Browse files
lpincaBethGriggs
authored andcommitted
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 Backport-PR-URL: #28000 Fixes: #26015 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: James M Snell <[email protected]>
1 parent cc9d005 commit 3ee076f

File tree

2 files changed

+38
-5
lines changed

2 files changed

+38
-5
lines changed

lib/internal/streams/destroy.js

+12-5
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,15 @@ function destroy(err, cb) {
1010
if (readableDestroyed || writableDestroyed) {
1111
if (cb) {
1212
cb(err);
13-
} else if (err &&
14-
(!this._writableState || !this._writableState.errorEmitted)) {
15-
process.nextTick(emitErrorNT, this, err);
13+
} else if (err) {
14+
if (!this._writableState) {
15+
process.nextTick(emitErrorNT, this, err);
16+
} else if (!this._writableState.errorEmitted) {
17+
this._writableState.errorEmitted = true;
18+
process.nextTick(emitErrorNT, this, err);
19+
}
1620
}
21+
1722
return this;
1823
}
1924

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

3237
this._destroy(err || null, (err) => {
3338
if (!cb && err) {
34-
process.nextTick(emitErrorNT, this, err);
35-
if (this._writableState) {
39+
if (!this._writableState) {
40+
process.nextTick(emitErrorNT, this, err);
41+
} else if (!this._writableState.errorEmitted) {
3642
this._writableState.errorEmitted = true;
43+
process.nextTick(emitErrorNT, this, err);
3744
}
3845
} else if (cb) {
3946
cb(err);

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

+26
Original file line numberDiff line numberDiff line change
@@ -146,6 +146,32 @@ const { inherits } = require('util');
146146
assert.strictEqual(write.destroyed, true);
147147
}
148148

149+
{
150+
const writable = new Writable({
151+
destroy: common.mustCall(function(err, cb) {
152+
process.nextTick(cb, new Error('kaboom 1'));
153+
}),
154+
write(chunk, enc, cb) {
155+
cb();
156+
}
157+
});
158+
159+
writable.on('close', common.mustNotCall());
160+
writable.on('error', common.expectsError({
161+
type: Error,
162+
message: 'kaboom 2'
163+
}));
164+
165+
writable.destroy();
166+
assert.strictEqual(writable.destroyed, true);
167+
assert.strictEqual(writable._writableState.errorEmitted, false);
168+
169+
// Test case where `writable.destroy()` is called again with an error before
170+
// the `_destroy()` callback is called.
171+
writable.destroy(new Error('kaboom 2'));
172+
assert.strictEqual(writable._writableState.errorEmitted, true);
173+
}
174+
149175
{
150176
const write = new Writable({
151177
write(chunk, enc, cb) { cb(); }

0 commit comments

Comments
 (0)