From 70c9e5cb41c8e08b28dcb6b4290dba06548fc389 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Tue, 17 Aug 2021 17:39:34 +0530 Subject: [PATCH 1/4] refactor: remove `killable` in favor of mehtod --- lib/Server.js | 28 ++++++++++++++++++++++++++-- package-lock.json | 11 ----------- package.json | 1 - 3 files changed, 26 insertions(+), 14 deletions(-) diff --git a/lib/Server.js b/lib/Server.js index 59141a4319..34262f72cd 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"); @@ -61,6 +60,31 @@ class Server { return /^[a-zA-Z][a-zA-Z\d+\-.]*:/.test(URL); } + static killable(server) { + let sockets = []; + + server.on("connection", (socket) => { + // add socket to list + sockets.push(socket); + + socket.once("close", () => { + // remove socket from list + sockets.splice(sockets.indexOf(socket), 1); + }); + }); + + server.kill = (cb) => { + server.close(cb); + sockets.forEach((socket) => { + socket.destroy(); + }); + // reset so the server can be restarted + sockets = []; + }; + + return server; + } + static async getHostname(hostname) { if (hostname === "local-ip") { return (await internalIp.v4()) || (await internalIp.v6()) || "0.0.0.0"; @@ -682,7 +706,7 @@ class Server { this.setupFeatures(); this.createServer(); - killable(this.server); + Server.killable(this.server); if (this.options.setupExitSignals) { const signals = ["SIGINT", "SIGTERM"]; 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", From b28bae27238d6624ef3d32b443b1cc718a571f85 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Tue, 17 Aug 2021 18:13:47 +0530 Subject: [PATCH 2/4] refactor: code --- lib/Server.js | 46 ++++++++++++++++++---------------------------- 1 file changed, 18 insertions(+), 28 deletions(-) diff --git a/lib/Server.js b/lib/Server.js index 34262f72cd..c72505831b 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -34,6 +34,7 @@ class Server { this.staticWatchers = []; // Keep track of websocket proxies for external websocket upgrade. this.webSocketProxies = []; + this.sockets = []; this.compiler = compiler; } @@ -60,31 +61,6 @@ class Server { return /^[a-zA-Z][a-zA-Z\d+\-.]*:/.test(URL); } - static killable(server) { - let sockets = []; - - server.on("connection", (socket) => { - // add socket to list - sockets.push(socket); - - socket.once("close", () => { - // remove socket from list - sockets.splice(sockets.indexOf(socket), 1); - }); - }); - - server.kill = (cb) => { - server.close(cb); - sockets.forEach((socket) => { - socket.destroy(); - }); - // reset so the server can be restarted - sockets = []; - }; - - return server; - } - static async getHostname(hostname) { if (hostname === "local-ip") { return (await internalIp.v4()) || (await internalIp.v6()) || "0.0.0.0"; @@ -706,8 +682,6 @@ class Server { this.setupFeatures(); this.createServer(); - Server.killable(this.server); - if (this.options.setupExitSignals) { const signals = ["SIGINT", "SIGTERM"]; @@ -1190,6 +1164,16 @@ class Server { 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; }); @@ -1821,7 +1805,13 @@ class Server { if (this.server) { await new Promise((resolve) => { - this.server.kill(() => { + this.server.kill((cb) => { + this.server.stopCallback(cb); + this.sockets.forEach((socket) => { + socket.destroy(); + }); + // reset so the server can be restarted + this.sockets = []; resolve(); }); }); From dd91f76a65d650128526ece0ac6a61b9c47a49c4 Mon Sep 17 00:00:00 2001 From: evilebottnawi Date: Tue, 17 Aug 2021 18:33:05 +0300 Subject: [PATCH 3/4] refactor: code --- lib/Server.js | 49 ++++++++++++----------- lib/servers/BaseServer.js | 2 +- lib/servers/SockJSServer.js | 10 +---- lib/servers/WebsocketServer.js | 10 +---- test/e2e/web-socket-communication.test.js | 2 +- 5 files changed, 32 insertions(+), 41 deletions(-) diff --git a/lib/Server.js b/lib/Server.js index c72505831b..c92bc8218a 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -1142,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 @@ -1158,18 +1155,22 @@ 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 + // Add socket to list this.sockets.push(socket); socket.once("close", () => { - // remove socket from list + // Remove socket from list this.sockets.splice(this.sockets.indexOf(socket), 1); }); }); @@ -1783,40 +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((cb) => { - this.server.stopCallback(cb); - this.sockets.forEach((socket) => { - socket.destroy(); - }); - // reset so the server can be restarted - this.sockets = []; + 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/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" ); From 6f43ab64fcda2338df0a0fb87623311e097f6611 Mon Sep 17 00:00:00 2001 From: evilebottnawi Date: Tue, 17 Aug 2021 18:50:52 +0300 Subject: [PATCH 4/4] test: fix --- test/server/setupExitSignals-option.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) 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();