refactor: remove killable (#3657)

Author: snitin315

lib/Server.js

@@ -7,7 +7,6 @@ const util = require("util");
 const fs = require("graceful-fs");
 const ipaddr = require("ipaddr.js");
 const internalIp = require("internal-ip");
-const killable = require("killable");
 const express = require("express");
 const { validate } = require("schema-utils");
 const schema = require("./options.json");
@@ -35,6 +34,7 @@ class Server {
     this.staticWatchers = [];
     // Keep track of websocket proxies for external websocket upgrade.
     this.webSocketProxies = [];
+    this.sockets = [];
     this.compiler = compiler;
   }
 
@@ -682,8 +682,6 @@ class Server {
     this.setupFeatures();
     this.createServer();
 
-    killable(this.server);
-
     if (this.options.setupExitSignals) {
       const signals = ["SIGINT", "SIGTERM"];
 
@@ -1144,9 +1142,6 @@ class Server {
   }
 
   createServer() {
-    const https = require("https");
-    const http = require("http");
-
     if (this.options.https) {
       if (this.options.http2) {
         // TODO: we need to replace spdy with http2 which is an internal module
@@ -1160,12 +1155,26 @@ class Server {
           this.app
         );
       } else {
+        const https = require("https");
+
         this.server = https.createServer(this.options.https, this.app);
       }
     } else {
+      const http = require("http");
+
       this.server = http.createServer(this.app);
     }
 
+    this.server.on("connection", (socket) => {
+      // Add socket to list
+      this.sockets.push(socket);
+
+      socket.once("close", () => {
+        // Remove socket from list
+        this.sockets.splice(this.sockets.indexOf(socket), 1);
+      });
+    });
+
     this.server.on("error", (error) => {
       throw error;
     });
@@ -1775,34 +1784,42 @@ class Server {
   }
 
   async stop() {
-    if (this.webSocketProxies.length > 0) {
-      this.webSocketProxies = [];
-    }
+    this.webSocketProxies = [];
 
-    if (this.staticWatchers.length > 0) {
-      await Promise.all(this.staticWatchers.map((watcher) => watcher.close()));
+    await Promise.all(this.staticWatchers.map((watcher) => watcher.close()));
 
-      this.staticWatchers = [];
-    }
+    this.staticWatchers = [];
 
     if (this.webSocketServer) {
       await new Promise((resolve) => {
         this.webSocketServer.implementation.close(() => {
+          this.webSocketServer = null;
+
           resolve();
         });
-      });
 
-      this.webSocketServer = null;
+        for (const client of this.webSocketServer.clients) {
+          client.terminate();
+        }
+
+        this.webSocketServer.clients = [];
+      });
     }
 
     if (this.server) {
       await new Promise((resolve) => {
-        this.server.kill(() => {
+        this.server.close(() => {
+          this.server = null;
+
           resolve();
         });
-      });
 
-      this.server = null;
+        for (const socket of this.sockets) {
+          socket.destroy();
+        }
+
+        this.sockets = [];
+      });
 
       if (this.middleware) {
         await new Promise((resolve, reject) => {

lib/servers/BaseServer.js

@@ -5,6 +5,6 @@
 module.exports = class BaseServer {
   constructor(server) {
     this.server = server;
-    this.clients = new Set();
+    this.clients = [];
   }
 };

lib/servers/SockJSServer.js

@@ -67,20 +67,14 @@ module.exports = class SockJSServer extends BaseServer {
       client.send = client.write;
       client.terminate = client.close;
 
-      this.clients.add(client);
+      this.clients.push(client);
 
       client.on("close", () => {
-        this.clients.delete(client);
+        this.clients.splice(this.clients.indexOf(client), 1);
       });
     });
 
     this.implementation.close = (callback) => {
-      for (const client of this.clients) {
-        client.close();
-      }
-
-      this.clients.clear();
-
       callback();
     };
   }

lib/servers/WebsocketServer.js

@@ -54,7 +54,7 @@ module.exports = class WebsocketServer extends BaseServer {
     }, WebsocketServer.heartbeatInterval);
 
     this.implementation.on("connection", (client) => {
-      this.clients.add(client);
+      this.clients.push(client);
 
       client.isAlive = true;
 
@@ -63,18 +63,12 @@ module.exports = class WebsocketServer extends BaseServer {
       });
 
       client.on("close", () => {
-        this.clients.delete(client);
+        this.clients.splice(this.clients.indexOf(client), 1);
       });
     });
 
     this.implementation.on("close", () => {
       clearInterval(interval);
-
-      for (const ws of this.clients) {
-        ws.terminate();
-      }
-
-      this.clients.clear();
     });
   }
 };

package-lock.json

@@ -45,7 +45,6 @@
     "http-proxy-middleware": "^2.0.0",
     "internal-ip": "^6.2.0",
     "ipaddr.js": "^2.0.1",
-    "killable": "^1.0.1",
     "open": "^8.0.9",
     "p-retry": "^4.5.0",
     "portfinder": "^1.0.28",

package.json

@@ -96,7 +96,7 @@ describe("web socket communication", () => {
         }, 200);
       });
 
-      expect(server.webSocketServer.clients.size).toBe(0);
+      expect(server.webSocketServer.clients.length).toBe(0);
       expect(consoleMessages.map((message) => message.text())).toMatchSnapshot(
         "console messages"
       );

test/e2e/web-socket-communication.test.js

@@ -8,7 +8,7 @@ const port = require("../ports-map")["setup-exit-signals-option"];
 describe("setupExitSignals option", () => {
   let server;
   let exitSpy;
-  let killSpy;
+  let stopCallbackSpy;
   let stdinResumeSpy;
 
   const signals = ["SIGINT", "SIGTERM"];
@@ -30,7 +30,7 @@ describe("setupExitSignals option", () => {
     stdinResumeSpy = jest
       .spyOn(process.stdin, "resume")
       .mockImplementation(() => {});
-    killSpy = jest.spyOn(server.server, "kill");
+    stopCallbackSpy = jest.spyOn(server, "stopCallback");
   });
 
   afterEach(async () => {
@@ -48,7 +48,7 @@ describe("setupExitSignals option", () => {
     process.emit(signal);
 
     setTimeout(() => {
-      expect(killSpy.mock.calls.length).toEqual(1);
+      expect(stopCallbackSpy.mock.calls.length).toEqual(1);
       expect(exitSpy.mock.calls.length).toEqual(1);
 
       done();