From fddead38be71c1baff0fd3d6be6d9856e02e7baa Mon Sep 17 00:00:00 2001 From: Kirill Nagaitsev Date: Tue, 28 Jan 2020 23:00:12 -0600 Subject: [PATCH 1/2] feat(server): added ws heartbeat ping interval --- lib/Server.js | 1 + lib/servers/WebsocketServer.js | 15 +++++++++++++++ test/server/servers/WebsocketServer.test.js | 7 +++++++ .../__snapshots__/WebsocketServer.test.js.snap | 1 + 4 files changed, 24 insertions(+) diff --git a/lib/Server.js b/lib/Server.js index dff653aceb..1e8cf2d1b4 100644 --- a/lib/Server.js +++ b/lib/Server.js @@ -71,6 +71,7 @@ class Server { updateCompiler(this.compiler, this.options); + this.heartbeatInterval = 30000; // this.SocketServerImplementation is a class, so it must be instantiated before use this.socketServerImplementation = getSocketServerImplementation( this.options diff --git a/lib/servers/WebsocketServer.js b/lib/servers/WebsocketServer.js index 8424b6d382..34002c675a 100644 --- a/lib/servers/WebsocketServer.js +++ b/lib/servers/WebsocketServer.js @@ -27,6 +27,17 @@ module.exports = class WebsocketServer extends BaseServer { this.wsServer.on('error', (err) => { this.server.log.error(err.message); }); + + const noop = () => {}; + + setInterval(() => { + this.wsServer.clients.forEach((ws) => { + if (ws.isAlive === false) return ws.terminate(); + + ws.isAlive = false; + ws.ping(noop); + }); + }, this.server.heartbeatInterval); } send(connection, message) { @@ -45,6 +56,10 @@ module.exports = class WebsocketServer extends BaseServer { // f should be passed the resulting connection and the connection headers onConnection(f) { this.wsServer.on('connection', (connection, req) => { + connection.isAlive = true; + connection.on('pong', () => { + connection.isAlive = true; + }); f(connection, req.headers); }); } diff --git a/test/server/servers/WebsocketServer.test.js b/test/server/servers/WebsocketServer.test.js index e8e59a786b..80f899386d 100644 --- a/test/server/servers/WebsocketServer.test.js +++ b/test/server/servers/WebsocketServer.test.js @@ -23,6 +23,7 @@ describe('WebsocketServer', () => { }, sockPath: '/ws-server', listeningApp, + heartbeatInterval: 800, }; socketServer = new WebsocketServer(server); @@ -57,6 +58,12 @@ describe('WebsocketServer', () => { data.push('close'); }; + // the heartbeat interval was shortened greatly above + // so that the client is quickly pinged + client.on('ping', () => { + data.push('ping'); + }); + setTimeout(() => { expect(headers.host).toMatchSnapshot(); expect(data).toMatchSnapshot(); diff --git a/test/server/servers/__snapshots__/WebsocketServer.test.js.snap b/test/server/servers/__snapshots__/WebsocketServer.test.js.snap index 2722e7f517..24a5d9a2fc 100644 --- a/test/server/servers/__snapshots__/WebsocketServer.test.js.snap +++ b/test/server/servers/__snapshots__/WebsocketServer.test.js.snap @@ -6,6 +6,7 @@ exports[`WebsocketServer server should recieve connection, send message, and clo Array [ "open", "hello world", + "ping", "close", ] `; From e74d5b4fcb8cae51382165a642c421d270f8ec5c Mon Sep 17 00:00:00 2001 From: Kirill Nagaitsev Date: Tue, 28 Jan 2020 23:32:03 -0600 Subject: [PATCH 2/2] test(server): check that connection is terminated when client not alive --- test/server/servers/WebsocketServer.test.js | 123 ++++++++++-------- .../WebsocketServer.test.js.snap | 4 +- 2 files changed, 72 insertions(+), 55 deletions(-) diff --git a/test/server/servers/WebsocketServer.test.js b/test/server/servers/WebsocketServer.test.js index 80f899386d..252a031f67 100644 --- a/test/server/servers/WebsocketServer.test.js +++ b/test/server/servers/WebsocketServer.test.js @@ -7,16 +7,17 @@ const WebsocketServer = require('../../../lib/servers/WebsocketServer'); const port = require('../../ports-map').WebsocketServer; describe('WebsocketServer', () => { + let server; let socketServer; let listeningApp; - beforeAll((done) => { + beforeEach((done) => { // eslint-disable-next-line new-cap const app = new express(); listeningApp = http.createServer(app); listeningApp.listen(port, 'localhost', () => { - const server = { + server = { log: { error: () => {}, debug: () => {}, @@ -25,76 +26,92 @@ describe('WebsocketServer', () => { listeningApp, heartbeatInterval: 800, }; - socketServer = new WebsocketServer(server); - done(); }); }); - describe('server', () => { - it('should recieve connection, send message, and close client', (done) => { - const data = []; - - let headers; - socketServer.onConnection((connection, h) => { - headers = h; - data.push('open'); - socketServer.send(connection, 'hello world'); - setTimeout(() => { - // the server closes the connection with the client - socketServer.close(connection); - }, 1000); - }); + it('should recieve connection, send message, and close client', (done) => { + const data = []; - // eslint-disable-next-line new-cap - const client = new ws(`http://localhost:${port}/ws-server`); + let headers; + socketServer.onConnection((connection, h) => { + headers = h; + data.push('open'); + socketServer.send(connection, 'hello world'); + setTimeout(() => { + // the server closes the connection with the client + socketServer.close(connection); + }, 1000); + }); - client.onmessage = (e) => { - data.push(e.data); - }; + // eslint-disable-next-line new-cap + const client = new ws(`http://localhost:${port}/ws-server`); - client.onclose = () => { - data.push('close'); - }; + client.onmessage = (e) => { + data.push(e.data); + }; - // the heartbeat interval was shortened greatly above - // so that the client is quickly pinged - client.on('ping', () => { - data.push('ping'); - }); + client.onclose = () => { + data.push('close'); + }; - setTimeout(() => { - expect(headers.host).toMatchSnapshot(); - expect(data).toMatchSnapshot(); - done(); - }, 3000); + // the heartbeat interval was shortened greatly above + // so that the client is quickly pinged + client.on('ping', () => { + data.push('ping'); }); - it('should receive client close event', (done) => { - let receivedClientClose = false; - socketServer.onConnection((connection) => { - socketServer.onConnectionClose(connection, () => { - receivedClientClose = true; - }); + setTimeout(() => { + expect(headers.host).toMatchSnapshot(); + expect(data).toMatchSnapshot(); + done(); + }, 3000); + }); + + it('should receive client close event', (done) => { + let receivedClientClose = false; + socketServer.onConnection((connection) => { + socketServer.onConnectionClose(connection, () => { + receivedClientClose = true; }); + }); - // eslint-disable-next-line new-cap - const client = new ws(`http://localhost:${port}/ws-server`); + // eslint-disable-next-line new-cap + const client = new ws(`http://localhost:${port}/ws-server`); - setTimeout(() => { - // the client closes itself, the server does not close it - client.close(); - }, 1000); + setTimeout(() => { + // the client closes itself, the server does not close it + client.close(); + }, 1000); - setTimeout(() => { - expect(receivedClientClose).toBeTruthy(); - done(); - }, 3000); + setTimeout(() => { + expect(receivedClientClose).toBeTruthy(); + done(); + }, 3000); + }); + + it('should terminate a client that is not alive', (done) => { + let receivedClientClose = false; + socketServer.onConnection((connection) => { + // this makes the server think the client did not respond + // to a ping in time, so the server will terminate it + connection.isAlive = false; + socketServer.onConnectionClose(connection, () => { + receivedClientClose = true; + }); }); + + // eslint-disable-next-line new-cap, no-unused-vars + const client = new ws(`http://localhost:${port}/ws-server`); + + setTimeout(() => { + expect(receivedClientClose).toBeTruthy(); + done(); + }, 3000); }); - afterAll((done) => { + afterEach((done) => { listeningApp.close(done); }); }); diff --git a/test/server/servers/__snapshots__/WebsocketServer.test.js.snap b/test/server/servers/__snapshots__/WebsocketServer.test.js.snap index 24a5d9a2fc..6e3c1a0e4f 100644 --- a/test/server/servers/__snapshots__/WebsocketServer.test.js.snap +++ b/test/server/servers/__snapshots__/WebsocketServer.test.js.snap @@ -1,8 +1,8 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`WebsocketServer server should recieve connection, send message, and close client 1`] = `"localhost:8131"`; +exports[`WebsocketServer should recieve connection, send message, and close client 1`] = `"localhost:8131"`; -exports[`WebsocketServer server should recieve connection, send message, and close client 2`] = ` +exports[`WebsocketServer should recieve connection, send message, and close client 2`] = ` Array [ "open", "hello world",