http: add test case for req-res close ordering

Since 55e83cb
has changed the ordering of the close event, add a test case.
IncomingMessage will emit close before the response is sent in case the
server is consuming data from it.

Refs: #33035 (comment)

PR-URL: #36645
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Robert Nagy <ronagy@icloud.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

test/parallel/test-http-req-res-close.js

@@ -4,34 +4,128 @@ const common = require('../common');
 const http = require('http');
 const assert = require('assert');
 
-const server = http.Server(common.mustCall((req, res) => {
-  let resClosed = false;
-
-  res.end();
-  let resFinished = false;
-  res.on('finish', common.mustCall(() => {
-    resFinished = true;
-    assert.strictEqual(resClosed, false);
-    assert.strictEqual(res.destroyed, false);
-    assert.strictEqual(resClosed, false);
+// When the response is ended immediately, `req` should emit `close`
+// after `res`
+{
+  const server = http.Server(common.mustCall((req, res) => {
+    let resClosed = false;
+    let reqClosed = false;
+
+    res.end();
+    let resFinished = false;
+    res.on('finish', common.mustCall(() => {
+      resFinished = true;
+      assert.strictEqual(resClosed, false);
+      assert.strictEqual(res.destroyed, false);
+      assert.strictEqual(resClosed, false);
+    }));
+    assert.strictEqual(req.destroyed, false);
+    res.on('close', common.mustCall(() => {
+      resClosed = true;
+      assert.strictEqual(resFinished, true);
+      assert.strictEqual(reqClosed, false);
+      assert.strictEqual(res.destroyed, true);
+    }));
+    assert.strictEqual(req.destroyed, false);
+    req.on('end', common.mustCall(() => {
+      assert.strictEqual(req.destroyed, false);
+    }));
+    req.on('close', common.mustCall(() => {
+      reqClosed = true;
+      assert.strictEqual(resClosed, true);
+      assert.strictEqual(req.destroyed, true);
+      assert.strictEqual(req._readableState.ended, true);
+    }));
+    res.socket.on('close', () => server.close());
   }));
-  assert.strictEqual(req.destroyed, false);
-  res.on('close', common.mustCall(() => {
-    resClosed = true;
-    assert.strictEqual(resFinished, true);
-    assert.strictEqual(res.destroyed, true);
+
+  server.listen(0, common.mustCall(() => {
+    http.get({ port: server.address().port }, common.mustCall());
   }));
-  assert.strictEqual(req.destroyed, false);
-  req.on('end', common.mustCall(() => {
+}
+
+// When there's no `data` handler attached to `req`,
+// `req` should emit `close` after `res`.
+{
+  const server = http.Server(common.mustCall((req, res) => {
+    let resClosed = false;
+    let reqClosed = false;
+
+    // This time, don't end the response immediately
+    setTimeout(() => res.end(), 100);
+    let resFinished = false;
+    res.on('finish', common.mustCall(() => {
+      resFinished = true;
+      assert.strictEqual(resClosed, false);
+      assert.strictEqual(res.destroyed, false);
+      assert.strictEqual(resClosed, false);
+    }));
+    assert.strictEqual(req.destroyed, false);
+    res.on('close', common.mustCall(() => {
+      resClosed = true;
+      assert.strictEqual(resFinished, true);
+      assert.strictEqual(reqClosed, false);
+      assert.strictEqual(res.destroyed, true);
+    }));
     assert.strictEqual(req.destroyed, false);
+    req.on('end', common.mustCall(() => {
+      assert.strictEqual(req.destroyed, false);
+    }));
+    req.on('close', common.mustCall(() => {
+      reqClosed = true;
+      assert.strictEqual(resClosed, true);
+      assert.strictEqual(req.destroyed, true);
+      assert.strictEqual(req._readableState.ended, true);
+    }));
+    res.socket.on('close', () => server.close());
   }));
-  req.on('close', common.mustCall(() => {
-    assert.strictEqual(req.destroyed, true);
-    assert.strictEqual(req._readableState.ended, true);
+
+  server.listen(0, common.mustCall(() => {
+    http.get({ port: server.address().port }, common.mustCall());
   }));
-  res.socket.on('close', () => server.close());
-}));
+}
+
+// When a `data` handler is `attached` to `req`
+// (i.e. the server is consuming  data from it), `req` should emit `close`
+// before `res`.
+// https://github.com/nodejs/node/pull/33035 introduced this change in behavior.
+// See https://github.com/nodejs/node/pull/33035#issuecomment-751482764
+{
+  const server = http.Server(common.mustCall((req, res) => {
+    let resClosed = false;
+    let reqClosed = false;
 
-server.listen(0, common.mustCall(() => {
-  http.get({ port: server.address().port }, common.mustCall());
-}));
+    // Don't end the response immediately
+    setTimeout(() => res.end(), 100);
+    let resFinished = false;
+    req.on('data', () => {});
+    res.on('finish', common.mustCall(() => {
+      resFinished = true;
+      assert.strictEqual(resClosed, false);
+      assert.strictEqual(res.destroyed, false);
+      assert.strictEqual(resClosed, false);
+    }));
+    assert.strictEqual(req.destroyed, false);
+    res.on('close', common.mustCall(() => {
+      resClosed = true;
+      assert.strictEqual(resFinished, true);
+      assert.strictEqual(reqClosed, true);
+      assert.strictEqual(res.destroyed, true);
+    }));
+    assert.strictEqual(req.destroyed, false);
+    req.on('end', common.mustCall(() => {
+      assert.strictEqual(req.destroyed, false);
+    }));
+    req.on('close', common.mustCall(() => {
+      reqClosed = true;
+      assert.strictEqual(resClosed, false);
+      assert.strictEqual(req.destroyed, true);
+      assert.strictEqual(req._readableState.ended, true);
+    }));
+    res.socket.on('close', () => server.close());
+  }));
+
+  server.listen(0, common.mustCall(() => {
+    http.get({ port: server.address().port }, common.mustCall());
+  }));
+}