From 22b14142123fa1e7422043a0483e3423a785a7db Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Tue, 17 Aug 2021 21:34:56 +0530 Subject: [PATCH] refactor: remove `killable` (#3657) --- lib/Server.js | 53 ++++++++++++++------- lib/servers/BaseServer.js | 2 +- lib/servers/SockJSServer.js | 10 +--- lib/servers/WebsocketServer.js | 10 +--- package-lock.json | 11 ----- package.json | 1 - test/e2e/web-socket-communication.test.js | 2 +- test/server/setupExitSignals-option.test.js | 6 +-- 8 files changed, 44 insertions(+), 51 deletions(-) diff --git a/lib/Server.js b/lib/Server.js index 59141a4319..c92bc8218a 100644 --- a/lib/Server.js +++ b/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) => { diff --git a/lib/servers/BaseServer.js b/lib/servers/BaseServer.js index d4812954bb..f512776cd2 100644 --- a/lib/servers/BaseServer.js +++ b/lib/servers/BaseServer.js @@ -5,6 +5,6 @@ module.exports = class BaseServer { constructor(server) { this.server = server; - this.clients = new Set(); + this.clients = []; } }; diff --git a/lib/servers/SockJSServer.js b/lib/servers/SockJSServer.js index 66c30daf83..4ee3110ee0 100644 --- a/lib/servers/SockJSServer.js +++ b/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(); }; } diff --git a/lib/servers/WebsocketServer.js b/lib/servers/WebsocketServer.js index d2639ff9f7..f2d334cabc 100644 --- a/lib/servers/WebsocketServer.js +++ b/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(); }); } }; diff --git a/package-lock.json b/package-lock.json index c330dc8061..1b71f79d81 100644 --- a/package-lock.json +++ b/package-lock.json @@ -21,7 +21,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", @@ -11768,11 +11767,6 @@ "node": "*" } }, - "node_modules/killable": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/killable/-/killable-1.0.1.tgz", - "integrity": "sha512-LzqtLKlUwirEUyl/nicirVmNiPvYs7l5n8wOPP7fyJVpUPkvCnW/vuiXGpylGUlnPDnB7311rARzAt3Mhswpjg==" - }, "node_modules/kind-of": { "version": "3.2.2", "resolved": "https://registry.npmjs.org/kind-of/-/kind-of-3.2.2.tgz", @@ -26705,11 +26699,6 @@ "through": ">=2.2.7 <3" } }, - "killable": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/killable/-/killable-1.0.1.tgz", - "integrity": "sha512-LzqtLKlUwirEUyl/nicirVmNiPvYs7l5n8wOPP7fyJVpUPkvCnW/vuiXGpylGUlnPDnB7311rARzAt3Mhswpjg==" - }, "kind-of": { "version": "3.2.2", "resolved": "https://registry.npmjs.org/kind-of/-/kind-of-3.2.2.tgz", diff --git a/package.json b/package.json index a1ff24714c..d5246ba390 100644 --- a/package.json +++ b/package.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", diff --git a/test/e2e/web-socket-communication.test.js b/test/e2e/web-socket-communication.test.js index 7c1cca180e..80e759a53f 100644 --- a/test/e2e/web-socket-communication.test.js +++ b/test/e2e/web-socket-communication.test.js @@ -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" ); diff --git a/test/server/setupExitSignals-option.test.js b/test/server/setupExitSignals-option.test.js index 6fda5ef375..999a14f319 100644 --- a/test/server/setupExitSignals-option.test.js +++ b/test/server/setupExitSignals-option.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();