Skip to content

Commit

Permalink
stream: always emit error before close
Browse files Browse the repository at this point in the history
PR-URL: #19836
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
  • Loading branch information
mafintosh committed Apr 9, 2018
1 parent 0cd8359 commit a7c25b7
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 6 deletions.
11 changes: 9 additions & 2 deletions lib/internal/streams/destroy.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
8 changes: 4 additions & 4 deletions test/parallel/test-http2-stream-destroy-event-order.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();
}));
}));
Expand All @@ -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();
}));
}));
Expand Down
24 changes: 24 additions & 0 deletions test/parallel/test-stream-destroy-event-order.js
Original file line number Diff line number Diff line change
@@ -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'));

0 comments on commit a7c25b7

Please sign in to comment.