Refactor servers

lib/Server.js

@@ -495,7 +495,7 @@ class Server {
       ) {
         this.sendMessage([connection], 'error', 'Invalid Host/Origin header');
 
-        this.socketServer.close(connection);
+        this.socketServer.closeConnection(connection);
 
         return;
       }
@@ -747,12 +747,11 @@ class Server {
       });
   }
 
-  close(cb) {
-    this.sockets.forEach((socket) => {
-      this.socketServer.close(socket);
-    });
-
-    this.sockets = [];
+  close(callback) {
+    if (this.socketServer) {
+      this.socketServer.close();
+      this.sockets = [];
+    }
 
     const prom = Promise.all(
       this.staticWatchers.map((watcher) => watcher.close())
@@ -762,7 +761,7 @@ class Server {
     this.server.kill(() => {
       // watchers must be closed before closing middleware
       prom.then(() => {
-        this.middleware.close(cb);
+        this.middleware.close(callback);
       });
     });
   }

lib/servers/SockJSServer.js

@@ -34,7 +34,7 @@ module.exports = class SockJSServer extends BaseServer {
   constructor(server) {
     super(server);
 
-    this.socket = sockjs.createServer({
+    this.implementation = sockjs.createServer({
       // Use provided up-to-date sockjs-client
       sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js',
       // Default logger is very annoy. Limit useless logs.
@@ -57,7 +57,7 @@ module.exports = class SockJSServer extends BaseServer {
       return options.path;
     };
 
-    this.socket.installHandlers(this.server.server, {
+    this.implementation.installHandlers(this.server.server, {
       ...this.server.options.webSocketServer.options,
       prefix: getPrefix(this.server.options.webSocketServer.options),
     });
@@ -72,13 +72,23 @@ module.exports = class SockJSServer extends BaseServer {
     connection.write(message);
   }
 
-  close(connection) {
+  close(callback) {
+    [...this.server.sockets].forEach((socket) => {
+      this.closeConnection(socket);
+    });
+
+    if (callback) {
+      callback();
+    }
+  }
+
+  closeConnection(connection) {
     connection.close();
   }
 
   // f should be passed the resulting connection and the connection headers
   onConnection(f) {
-    this.socket.on('connection', (connection) => {
+    this.implementation.on('connection', (connection) => {
       f(connection, connection ? connection.headers : null);
     });
   }

lib/servers/WebsocketServer.js

@@ -10,29 +10,29 @@ module.exports = class WebsocketServer extends BaseServer {
   constructor(server) {
     super(server);
 
-    this.webSocketServer = new ws.Server({
+    this.implementation = new ws.Server({
       ...this.server.options.webSocketServer.options,
       noServer: true,
     });
 
     this.server.server.on('upgrade', (req, sock, head) => {
-      if (!this.webSocketServer.shouldHandle(req)) {
+      if (!this.implementation.shouldHandle(req)) {
         return;
       }
 
-      this.webSocketServer.handleUpgrade(req, sock, head, (connection) => {
-        this.webSocketServer.emit('connection', connection, req);
+      this.implementation.handleUpgrade(req, sock, head, (connection) => {
+        this.implementation.emit('connection', connection, req);
       });
     });
 
-    this.webSocketServer.on('error', (err) => {
+    this.implementation.on('error', (err) => {
       this.server.logger.error(err.message);
     });
 
     const noop = () => {};
 
-    setInterval(() => {
-      this.webSocketServer.clients.forEach((socket) => {
+    const interval = setInterval(() => {
+      this.implementation.clients.forEach((socket) => {
         if (socket.isAlive === false) {
           return socket.terminate();
         }
@@ -41,6 +41,10 @@ module.exports = class WebsocketServer extends BaseServer {
         socket.ping(noop);
       });
     }, this.server.webSocketHeartbeatInterval);
+
+    this.implementation.on('close', () => {
+      clearInterval(interval);
+    });
   }
 
   send(connection, message) {
@@ -52,13 +56,17 @@ module.exports = class WebsocketServer extends BaseServer {
     connection.send(message);
   }
 
-  close(connection) {
+  close(callback) {
+    this.implementation.close(callback);
+  }
+
+  closeConnection(connection) {
     connection.close();
   }
 
   // f should be passed the resulting connection and the connection headers
   onConnection(f) {
-    this.webSocketServer.on('connection', (connection, req) => {
+    this.implementation.on('connection', (connection, req) => {
       connection.isAlive = true;
       connection.on('pong', () => {
         connection.isAlive = true;

package.json

@@ -21,7 +21,7 @@
     "fix:js": "npm run lint:js -- --fix",
     "fix": "npm-run-all fix:js fmt",
     "commitlint": "commitlint --from=master",
-    "test:only": "jest --forceExit",
+    "test:only": "jest",
     "test:coverage": "npm run test:only -- --coverage",
     "test:watch": "npm run test:coverage --watch",
     "test": "npm run test:coverage",

test/server/Server.test.js

@@ -127,6 +127,8 @@ describe('Server', () => {
     const emitError = () => server.server.emit('error', new Error('Error !!!'));
 
     expect(emitError).toThrowError();
+
+    server.close();
   });
 
   // issue: https://github.com/webpack/webpack-dev-server/issues/1724

test/server/servers/SockJSServer.test.js

@@ -48,7 +48,7 @@ describe('SockJSServer', () => {
           socketServer.send(connection, 'hello world');
           setTimeout(() => {
             // the server closes the connection with the client
-            socketServer.close(connection);
+            socketServer.closeConnection(connection);
           }, 1000);
         });
 
@@ -97,7 +97,7 @@ describe('SockJSServer', () => {
         socketServer.onConnection(cb);
 
         expect(() => {
-          socketServer.socket.emit('connection', null);
+          socketServer.implementation.emit('connection', null);
         }).not.toThrow();
         expect(cb.mock.calls[0]).toEqual([null, null]);
       });
@@ -149,7 +149,7 @@ describe('SockJSServer', () => {
           socketServer.send(connection, 'hello world');
           setTimeout(() => {
             // the server closes the connection with the client
-            socketServer.close(connection);
+            socketServer.closeConnection(connection);
           }, 1000);
         });
 
@@ -198,7 +198,7 @@ describe('SockJSServer', () => {
         socketServer.onConnection(cb);
 
         expect(() => {
-          socketServer.socket.emit('connection', null);
+          socketServer.implementation.emit('connection', null);
         }).not.toThrow();
         expect(cb.mock.calls[0]).toEqual([null, null]);
       });

test/server/servers/WebsocketServer.test.js

@@ -41,7 +41,7 @@ describe('WebsocketServer', () => {
       socketServer.send(connection, 'hello world');
       setTimeout(() => {
         // the server closes the connection with the client
-        socketServer.close(connection);
+        socketServer.closeConnection(connection);
       }, 1000);
     });
 

test/server/webSocketServer-option.test.js

@@ -13,7 +13,7 @@ const BaseServer = require('../../lib/servers/BaseServer');
 const port = require('../ports-map')['webSocketServer-option'];
 
 describe('webSocketServer', () => {
-  describe.only('server', () => {
+  describe('server', () => {
     let mockedTestServer;
     let testServer;
     let server;
@@ -56,6 +56,7 @@ describe('webSocketServer', () => {
             class MockServer {
               // eslint-disable-next-line no-empty-function
               onConnection() {}
+              close() {}
             }
         );
       });
@@ -199,7 +200,7 @@ describe('webSocketServer', () => {
                 constructor(serv) {
                   super(serv);
 
-                  this.socket = sockjs.createServer({
+                  this.implementation = sockjs.createServer({
                     // Use provided up-to-date sockjs-client
                     sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js',
                     // Limit useless logs
@@ -214,23 +215,33 @@ describe('webSocketServer', () => {
                     },
                   });
 
-                  this.socket.installHandlers(this.server.server, {
+                  this.implementation.installHandlers(this.server.server, {
                     prefix: 'ws',
                   });
 
                   customServerUsed = true;
                 }
 
-                send(connection, message) {
-                  connection.write(message);
+                close(callback) {
+                  [...this.server.sockets].forEach((socket) => {
+                    this.closeConnection(socket);
+                  });
+
+                  if (callback) {
+                    callback();
+                  }
                 }
 
-                close(connection) {
+                closeConnection(connection) {
                   connection.close();
                 }
 
+                send(connection, message) {
+                  connection.write(message);
+                }
+
                 onConnection(f) {
-                  this.socket.on('connection', (connection) => {
+                  this.implementation.on('connection', (connection) => {
                     f(connection, connection.headers);
                   });
                 }
@@ -278,21 +289,22 @@ describe('webSocketServer', () => {
               webSocketServer: class MySockJSServer extends BaseServer {
                 constructor(serv) {
                   super(serv);
-
-                  this.socket = sockjs.createServer({
+                  this.implementation = sockjs.createServer({
                     // Use provided up-to-date sockjs-client
                     sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js',
                     // Limit useless logs
                     log: (severity, line) => {
                       if (severity === 'error') {
                         this.server.logger.error(line);
+                      } else if (severity === 'info') {
+                        this.server.logger.log(line);
                       } else {
                         this.server.logger.debug(line);
                       }
                     },
                   });
 
-                  this.socket.installHandlers(this.server.server, {
+                  this.implementation.installHandlers(this.server.server, {
                     prefix: '/ws',
                   });
                 }
@@ -301,12 +313,22 @@ describe('webSocketServer', () => {
                   connection.write(message);
                 }
 
-                close(connection) {
+                close(callback) {
+                  [...this.server.sockets].forEach((socket) => {
+                    this.closeConnection(socket);
+                  });
+
+                  if (callback) {
+                    callback();
+                  }
+                }
+
+                closeConnection(connection) {
                   connection.close();
                 }
 
                 onConnection(f) {
-                  this.socket.on('connection', (connection) => {
+                  this.implementation.on('connection', (connection) => {
                     f(connection);
                   });
                 }
@@ -377,7 +399,7 @@ describe('webSocketServer', () => {
               webSocketServer: class MySockJSServer extends BaseServer {
                 constructor(serv) {
                   super(serv);
-                  this.socket = sockjs.createServer({
+                  this.implementation = sockjs.createServer({
                     // Use provided up-to-date sockjs-client
                     sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js',
                     // Limit useless logs
@@ -392,7 +414,7 @@ describe('webSocketServer', () => {
                     },
                   });
 
-                  this.socket.installHandlers(this.server.server, {
+                  this.implementation.installHandlers(this.server.server, {
                     prefix: '/ws',
                   });
                 }
@@ -401,12 +423,22 @@ describe('webSocketServer', () => {
                   connection.write(message);
                 }
 
-                close(connection) {
+                close(callback) {
+                  [...this.server.sockets].forEach((socket) => {
+                    this.closeConnection(socket);
+                  });
+
+                  if (callback) {
+                    callback();
+                  }
+                }
+
+                closeConnection(connection) {
                   connection.close();
                 }
 
                 onConnection(f) {
-                  this.socket.on('connection', (connection) => {
+                  this.implementation.on('connection', (connection) => {
                     f(connection, {
                       host: null,
                     });
@@ -530,7 +562,7 @@ describe('webSocketServer', () => {
         expect(mockServerInstance.onConnection.mock.calls).toMatchSnapshot();
         // the only call to send() here should be an invalid header message
         expect(mockServerInstance.send.mock.calls).toMatchSnapshot();
-        expect(mockServerInstance.close.mock.calls).toMatchSnapshot();
+        expect(mockServerInstance.closeConnection.mock.calls).toMatchSnapshot();
         // onConnectionClose should never get called since the client should be closed first
         expect(mockServerInstance.onConnectionClose.mock.calls.length).toEqual(
           0