stream: always emit error before close

PR-URL: #19836
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>

Author: mafintosh

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;

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();
     }));
   }));

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'));