End server earlier than graceful period if all requests end themselves.

Node >=10, drop sinon, more prettier.
flash-oss · Oct 25, 2021 · b77c3ef · b77c3ef
1 parent 831361d
commit b77c3ef
Show file tree

Hide file tree

Showing 7 changed files with 131 additions and 100 deletions.
diff --git a/.nvmrc b/.nvmrc
@@ -1 +1 @@
-8
+10
diff --git a/package.json b/package.json
@@ -12,8 +12,7 @@
     "got": "^11.8.1",
     "nyc": "^15.1.0",
     "pem": "^1.14.4",
-    "prettier": "^1.19.1",
-    "sinon": "^9.2.4"
+    "prettier": "^1.19.1"
   },
   "eslintConfig": {
     "parserOptions": {

diff --git a/src/delay.js b/src/delay.js
@@ -1,3 +1,3 @@
 module.exports = function delay(time) {
-  return new Promise(resolve => setTimeout(() => resolve(), time));
-}
+    return new Promise(resolve => setTimeout(() => resolve(), time));
+};
diff --git a/src/index.js b/src/index.js
@@ -128,16 +128,13 @@ module.exports = function HttpTerminator({
                     destroySocket(socket);
                 }
 
-                if (_sockets.size) {
-                    await delay(gracefulTerminationTimeout);
+                if (_sockets.size || _secureSockets.size) {
+                    const endWaitAt = Date.now() + gracefulTerminationTimeout;
+                    while ((_sockets.size || _secureSockets.size) && Date.now() < endWaitAt) await delay(1);
 
                     for (const socket of _sockets) {
                         destroySocket(socket);
                     }
-                }
-
-                if (_secureSockets.size) {
-                    await delay(gracefulTerminationTimeout);
 
                     for (const socket of _secureSockets) {
                         destroySocket(socket);

diff --git a/test/helpers/createHttpServer.js b/test/helpers/createHttpServer.js
@@ -2,38 +2,38 @@ const { createServer } = require("http");
 const { promisify } = require("util");
 
 module.exports = requestHandler => {
-  const server = createServer(requestHandler);
+    const server = createServer(requestHandler);
 
-  let serverShuttingDown;
+    let serverShuttingDown;
 
-  const stop = () => {
-    if (serverShuttingDown) {
-      return serverShuttingDown;
-    }
+    const stop = () => {
+        if (serverShuttingDown) {
+            return serverShuttingDown;
+        }
 
-    serverShuttingDown = promisify(server.close.bind(server))();
+        serverShuttingDown = promisify(server.close.bind(server))();
 
-    return serverShuttingDown;
-  };
+        return serverShuttingDown;
+    };
 
-  const getConnections = () => {
-    return promisify(server.getConnections.bind(server))();
-  };
+    const getConnections = () => {
+        return promisify(server.getConnections.bind(server))();
+    };
 
-  return new Promise((resolve, reject) => {
-    server.once("error", reject);
+    return new Promise((resolve, reject) => {
+        server.once("error", reject);
 
-    server.listen(() => {
-      const port = server.address().port;
-      const url = "http://localhost:" + port;
+        server.listen(() => {
+            const port = server.address().port;
+            const url = "http://localhost:" + port;
 
-      resolve({
-        getConnections,
-        port,
-        server,
-        stop,
-        url
-      });
+            resolve({
+                getConnections,
+                port,
+                server,
+                stop,
+                url
+            });
+        });
     });
-  });
 };
diff --git a/test/helpers/createTests.js b/test/helpers/createTests.js
@@ -2,7 +2,6 @@ const KeepAliveHttpAgent = require("agentkeepalive");
 const test = require("ava");
 const delay = require("../../src/delay");
 const safeGot = require("got");
-const sinon = require("sinon");
 const createHttpTerminator = require("../../src");
 
 const got = safeGot.extend({
@@ -31,10 +30,10 @@ module.exports = createHttpServer => {
     });
 
     test("terminates hanging sockets after gracefulTerminationTimeout", async t => {
-        const spy = sinon.spy();
+        let serverCreated = false;
 
         const httpServer = await createHttpServer(() => {
-            spy();
+            serverCreated = true;
         });
 
         t.timeout(500);
@@ -48,7 +47,7 @@ module.exports = createHttpServer => {
 
         await delay(50);
 
-        t.true(spy.called);
+        t.true(serverCreated);
 
         terminator.terminate();
 
@@ -63,19 +62,20 @@ module.exports = createHttpServer => {
     });
 
     test("server stops accepting new connections after terminator.terminate() is called", async t => {
-        const stub = sinon.stub();
-
-        stub.onCall(0).callsFake((incomingMessage, outgoingMessage) => {
-            setTimeout(() => {
-                outgoingMessage.end("foo");
-            }, 100);
-        });
-
-        stub.onCall(1).callsFake((incomingMessage, outgoingMessage) => {
-            outgoingMessage.end("bar");
-        });
+        let callCount = 0;
+
+        function requestHandler(incomingMessage, outgoingMessage) {
+            if (callCount === 0) {
+                setTimeout(() => {
+                    outgoingMessage.end("foo");
+                }, 100);
+            } else if (callCount === 1) {
+                outgoingMessage.end("bar");
+            }
+            callCount += 1;
+        }
 
-        const httpServer = await createHttpServer(stub);
+        const httpServer = await createHttpServer(requestHandler);
 
         t.timeout(500);
 
@@ -148,27 +148,28 @@ module.exports = createHttpServer => {
     });
 
     test("ongoing requests receive {connection: close} header (new request reusing an existing socket)", async t => {
-        const stub = sinon.stub();
-
-        stub.onCall(0).callsFake((incomingMessage, outgoingMessage) => {
-            outgoingMessage.write("foo");
-
-            setTimeout(() => {
-                outgoingMessage.end("bar");
-            }, 50);
-        });
-
-        stub.onCall(1).callsFake((incomingMessage, outgoingMessage) => {
-            // @todo Unable to intercept the response without the delay.
-            // When `end()` is called immediately, the `request` event
-            // already has `headersSent=true`. It is unclear how to intercept
-            // the response beforehand.
-            setTimeout(() => {
-                outgoingMessage.end("baz");
-            }, 50);
-        });
+        let callCount = 0;
+
+        function requestHandler(incomingMessage, outgoingMessage) {
+            if (callCount === 0) {
+                outgoingMessage.write("foo");
+
+                setTimeout(() => {
+                    outgoingMessage.end("bar");
+                }, 51);
+            } else if (callCount === 1) {
+                // @todo Unable to intercept the response without the delay.
+                // When `end()` is called immediately, the `request` event
+                // already has `headersSent=true`. It is unclear how to intercept
+                // the response beforehand.
+                setTimeout(() => {
+                    outgoingMessage.end("baz");
+                }, 51);
+            }
+            callCount += 1;
+        }
 
-        const httpServer = await createHttpServer(stub);
+        const httpServer = await createHttpServer(requestHandler);
 
         t.timeout(1000);
 
@@ -204,9 +205,9 @@ module.exports = createHttpServer => {
             retry: 0
         });
 
-        await delay(50);
+        await delay(75);
 
-        t.is(stub.callCount, 2);
+        t.is(callCount, 2);
 
         const response0 = await request0;
 

diff --git a/test/src/HttpTerminator.js b/test/src/HttpTerminator.js
@@ -2,7 +2,6 @@ const KeepAliveHttpAgent = require("agentkeepalive");
 const test = require("ava");
 const delay = require("../../src/delay");
 const safeGot = require("got");
-const sinon = require("sinon");
 const HttpTerminator = require("../../src");
 const createHttpServer = require("../helpers/createHttpServer");
 const createHttpsServer = require("../helpers/createHttpsServer");
@@ -34,10 +33,10 @@ test("terminates HTTP server with no connections", async t => {
 test("terminates hanging sockets after httpResponseTimeout", async t => {
     t.timeout(500);
 
-    const spy = sinon.spy();
+    let serverCreated = false;
 
     const httpServer = await createHttpServer(() => {
-        spy();
+        serverCreated = true;
     });
 
     const terminator = HttpTerminator({
@@ -49,7 +48,7 @@ test("terminates hanging sockets after httpResponseTimeout", async t => {
 
     await delay(50);
 
-    t.true(spy.called);
+    t.true(serverCreated);
 
     const terminationPromise = terminator.terminate();
 
@@ -145,27 +144,28 @@ test("ongoing requests receive {connection: close} header", async t => {
 test("ongoing requests receive {connection: close} header (new request reusing an existing socket)", async t => {
     t.timeout(1000);
 
-    const stub = sinon.stub();
-
-    stub.onCall(0).callsFake((incomingMessage, outgoingMessage) => {
-        outgoingMessage.write("foo");
-
-        setTimeout(() => {
-            outgoingMessage.end("bar");
-        }, 50);
-    });
-
-    stub.onCall(1).callsFake((incomingMessage, outgoingMessage) => {
-        // @todo Unable to intercept the response without the delay.
-        // When `end()` is called immediately, the `request` event
-        // already has `headersSent=true`. It is unclear how to intercept
-        // the response beforehand.
-        setTimeout(() => {
-            outgoingMessage.end("baz");
-        }, 50);
-    });
+    let callCount = 0;
+
+    function requestHandler(incomingMessage, outgoingMessage) {
+        if (callCount === 0) {
+            outgoingMessage.write("foo");
+
+            setTimeout(() => {
+                outgoingMessage.end("bar");
+            }, 51);
+        } else if (callCount === 1) {
+            // @todo Unable to intercept the response without the delay.
+            // When `end()` is called immediately, the `request` event
+            // already has `headersSent=true`. It is unclear how to intercept
+            // the response beforehand.
+            setTimeout(() => {
+                outgoingMessage.end("baz");
+            }, 51);
+        }
+        callCount += 1;
+    }
 
-    const httpServer = await createHttpServer(stub);
+    const httpServer = await createHttpServer(requestHandler);
 
     const terminator = HttpTerminator({
         gracefulTerminationTimeout: 150,
@@ -193,9 +193,9 @@ test("ongoing requests receive {connection: close} header (new request reusing a
         retry: 0
     });
 
-    await delay(50);
+    await delay(75);
 
-    t.is(stub.callCount, 2);
+    t.is(callCount, 2);
 
     const response0 = await request0;
 
@@ -314,3 +314,37 @@ test("returns {success: false, code: 'INTERNAL_ERROR'} if unexpected exception",
     t.false(result.success);
     t.is(result.code, "INTERNAL_ERROR");
 });
+
+test("closes immediately after in-flight connections are closed", async t => {
+    t.timeout(1000);
+
+    function requestHandler(incomingMessage, outgoingMessage) {
+        setTimeout(() => {
+            outgoingMessage.end("foo");
+        }, 100);
+    }
+
+    const httpServer = await createHttpServer(requestHandler);
+
+    t.true(httpServer.server.listening);
+
+    const terminator = HttpTerminator({
+        gracefulTerminationTimeout: 500,
+        server: httpServer.server
+    });
+
+    got(httpServer.url);
+
+    await delay(50);
+
+    t.is(await httpServer.getConnections(), 1);
+
+    terminator.terminate();
+
+    // Wait for outgoingMessage.end to be called, plus a few extra ms for the
+    // terminator to finish polling in-flight connections. (Do not, however, wait
+    // long enough to trigger graceful termination.)
+    await delay(75);
+
+    t.is(await httpServer.getConnections(), 0);
+});