From 67bc5fe79780b1bfc68fc16929f908140376cfda Mon Sep 17 00:00:00 2001 From: Mathias Buus Date: Thu, 5 Apr 2018 20:52:19 +0200 Subject: [PATCH 1/2] stream: add failing event order test --- .../test-stream-destroy-event-order.js | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 test/parallel/test-stream-destroy-event-order.js diff --git a/test/parallel/test-stream-destroy-event-order.js b/test/parallel/test-stream-destroy-event-order.js new file mode 100644 index 00000000000000..a88fff820dedb9 --- /dev/null +++ b/test/parallel/test-stream-destroy-event-order.js @@ -0,0 +1,24 @@ +'use strict'; + +const common = require('../common'); +const assert = require('assert'); +const { Readable } = require('stream'); + +const rs = new Readable({ + read() {} +}); + +let closed = false; +let errored = false; + +rs.on('close', common.mustCall(() => { + closed = true; + assert(errored); +})); + +rs.on('error', common.mustCall((err) => { + errored = true; + assert(!closed); +})); + +rs.destroy(new Error('kaboom')); From dfd2f30c9d26eb43158c10b086cca534f5edb9bb Mon Sep 17 00:00:00 2001 From: Mathias Buus Date: Thu, 5 Apr 2018 13:24:13 +0200 Subject: [PATCH 2/2] stream: always emit error before close --- lib/internal/streams/destroy.js | 11 +++++++++-- .../parallel/test-http2-stream-destroy-event-order.js | 8 ++++---- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/lib/internal/streams/destroy.js b/lib/internal/streams/destroy.js index 5d29e182041cdc..3a0383cc3cea70 100644 --- a/lib/internal/streams/destroy.js +++ b/lib/internal/streams/destroy.js @@ -30,20 +30,27 @@ function destroy(err, cb) { } this._destroy(err || null, (err) => { - process.nextTick(emitCloseNT, this); if (!cb && err) { - process.nextTick(emitErrorNT, this, err); + process.nextTick(emitErrorAndCloseNT, this, err); if (this._writableState) { this._writableState.errorEmitted = true; } } else if (cb) { + process.nextTick(emitCloseNT, this); cb(err); + } else { + process.nextTick(emitCloseNT, this); } }); return this; } +function emitErrorAndCloseNT(self, err) { + emitErrorNT(self, err); + emitCloseNT(self); +} + function emitCloseNT(self) { if (self._writableState && !self._writableState.emitClose) return; diff --git a/test/parallel/test-http2-stream-destroy-event-order.js b/test/parallel/test-http2-stream-destroy-event-order.js index e2732830efbd8a..88e4a99f99eee3 100644 --- a/test/parallel/test-http2-stream-destroy-event-order.js +++ b/test/parallel/test-http2-stream-destroy-event-order.js @@ -9,8 +9,8 @@ let client; let req; const server = http2.createServer(); server.on('stream', common.mustCall((stream) => { - stream.on('close', common.mustCall(() => { - stream.on('error', common.mustCall(() => { + stream.on('error', common.mustCall(() => { + stream.on('close', common.mustCall(() => { server.close(); })); })); @@ -21,8 +21,8 @@ server.listen(0, common.mustCall(() => { client = http2.connect(`http://localhost:${server.address().port}`); req = client.request(); req.resume(); - req.on('close', common.mustCall(() => { - req.on('error', common.mustCall(() => { + req.on('error', common.mustCall(() => { + req.on('close', common.mustCall(() => { client.close(); })); }));