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 <[email protected]> Reviewed-By: Robert Nagy <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
nodejs · Jan 12, 2021 · dfc962f · dfc962f
1 parent cc28d2f
commit dfc962f
Showing 1 changed file with 119 additions and 25 deletions.
diff --git a/test/parallel/test-http-req-res-close.js b/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());
+  }));
+}