Skip to content

Commit 99c555a

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 Fixes: #26015 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
1 parent de7db26 commit 99c555a

File tree

2 files changed

+40
-5
lines changed

2 files changed

+40
-5
lines changed

lib/internal/streams/destroy.js

+14-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,13 @@ function destroy(err, cb) {
3136

3237
this._destroy(err || null, (err) => {
3338
if (!cb && err) {
34-
process.nextTick(emitErrorAndCloseNT, this, err);
35-
if (this._writableState) {
39+
if (!this._writableState) {
40+
process.nextTick(emitErrorAndCloseNT, this, err);
41+
} else if (!this._writableState.errorEmitted) {
3642
this._writableState.errorEmitted = true;
43+
process.nextTick(emitErrorAndCloseNT, this, err);
44+
} else {
45+
process.nextTick(emitCloseNT, this);
3746
}
3847
} else if (cb) {
3948
process.nextTick(emitCloseNT, this);

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

+26
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,32 @@ const { inherits } = require('util');
153153
assert.strictEqual(write.destroyed, true);
154154
}
155155

156+
{
157+
const writable = new Writable({
158+
destroy: common.mustCall(function(err, cb) {
159+
process.nextTick(cb, new Error('kaboom 1'));
160+
}),
161+
write(chunk, enc, cb) {
162+
cb();
163+
}
164+
});
165+
166+
writable.on('close', common.mustCall());
167+
writable.on('error', common.expectsError({
168+
type: Error,
169+
message: 'kaboom 2'
170+
}));
171+
172+
writable.destroy();
173+
assert.strictEqual(writable.destroyed, true);
174+
assert.strictEqual(writable._writableState.errorEmitted, false);
175+
176+
// Test case where `writable.destroy()` is called again with an error before
177+
// the `_destroy()` callback is called.
178+
writable.destroy(new Error('kaboom 2'));
179+
assert.strictEqual(writable._writableState.errorEmitted, true);
180+
}
181+
156182
{
157183
const write = new Writable({
158184
write(chunk, enc, cb) { cb(); }

0 commit comments

Comments
 (0)