Skip to content

Commit bc8328a

Browse files
alexander-akaitalexander-akait
authored
refactor: servers
1 parent f7e922d commit bc8328a

8 files changed

+95
-44
lines changed

‎lib/Server.js

+7-8
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ class Server {
495495
) {
496496
this.sendMessage([connection], 'error', 'Invalid Host/Origin header');
497497

498-
this.socketServer.close(connection);
498+
this.socketServer.closeConnection(connection);
499499

500500
return;
501501
}
@@ -747,12 +747,11 @@ class Server {
747747
});
748748
}
749749

750-
close(cb) {
751-
this.sockets.forEach((socket) => {
752-
this.socketServer.close(socket);
753-
});
754-
755-
this.sockets = [];
750+
close(callback) {
751+
if (this.socketServer) {
752+
this.socketServer.close();
753+
this.sockets = [];
754+
}
756755

757756
const prom = Promise.all(
758757
this.staticWatchers.map((watcher) => watcher.close())
@@ -762,7 +761,7 @@ class Server {
762761
this.server.kill(() => {
763762
// watchers must be closed before closing middleware
764763
prom.then(() => {
765-
this.middleware.close(cb);
764+
this.middleware.close(callback);
766765
});
767766
});
768767
}

‎lib/servers/SockJSServer.js

+14-4
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ module.exports = class SockJSServer extends BaseServer {
3434
constructor(server) {
3535
super(server);
3636

37-
this.socket = sockjs.createServer({
37+
this.implementation = sockjs.createServer({
3838
// Use provided up-to-date sockjs-client
3939
sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js',
4040
// Default logger is very annoy. Limit useless logs.
@@ -57,7 +57,7 @@ module.exports = class SockJSServer extends BaseServer {
5757
return options.path;
5858
};
5959

60-
this.socket.installHandlers(this.server.server, {
60+
this.implementation.installHandlers(this.server.server, {
6161
...this.server.options.webSocketServer.options,
6262
prefix: getPrefix(this.server.options.webSocketServer.options),
6363
});
@@ -72,13 +72,23 @@ module.exports = class SockJSServer extends BaseServer {
7272
connection.write(message);
7373
}
7474

75-
close(connection) {
75+
close(callback) {
76+
[...this.server.sockets].forEach((socket) => {
77+
this.closeConnection(socket);
78+
});
79+
80+
if (callback) {
81+
callback();
82+
}
83+
}
84+
85+
closeConnection(connection) {
7686
connection.close();
7787
}
7888

7989
// f should be passed the resulting connection and the connection headers
8090
onConnection(f) {
81-
this.socket.on('connection', (connection) => {
91+
this.implementation.on('connection', (connection) => {
8292
f(connection, connection ? connection.headers : null);
8393
});
8494
}

‎lib/servers/WebsocketServer.js

+17-9
Original file line numberDiff line numberDiff line change
@@ -10,29 +10,29 @@ module.exports = class WebsocketServer extends BaseServer {
1010
constructor(server) {
1111
super(server);
1212

13-
this.webSocketServer = new ws.Server({
13+
this.implementation = new ws.Server({
1414
...this.server.options.webSocketServer.options,
1515
noServer: true,
1616
});
1717

1818
this.server.server.on('upgrade', (req, sock, head) => {
19-
if (!this.webSocketServer.shouldHandle(req)) {
19+
if (!this.implementation.shouldHandle(req)) {
2020
return;
2121
}
2222

23-
this.webSocketServer.handleUpgrade(req, sock, head, (connection) => {
24-
this.webSocketServer.emit('connection', connection, req);
23+
this.implementation.handleUpgrade(req, sock, head, (connection) => {
24+
this.implementation.emit('connection', connection, req);
2525
});
2626
});
2727

28-
this.webSocketServer.on('error', (err) => {
28+
this.implementation.on('error', (err) => {
2929
this.server.logger.error(err.message);
3030
});
3131

3232
const noop = () => {};
3333

34-
setInterval(() => {
35-
this.webSocketServer.clients.forEach((socket) => {
34+
const interval = setInterval(() => {
35+
this.implementation.clients.forEach((socket) => {
3636
if (socket.isAlive === false) {
3737
return socket.terminate();
3838
}
@@ -41,6 +41,10 @@ module.exports = class WebsocketServer extends BaseServer {
4141
socket.ping(noop);
4242
});
4343
}, this.server.webSocketHeartbeatInterval);
44+
45+
this.implementation.on('close', () => {
46+
clearInterval(interval);
47+
});
4448
}
4549

4650
send(connection, message) {
@@ -52,13 +56,17 @@ module.exports = class WebsocketServer extends BaseServer {
5256
connection.send(message);
5357
}
5458

55-
close(connection) {
59+
close(callback) {
60+
this.implementation.close(callback);
61+
}
62+
63+
closeConnection(connection) {
5664
connection.close();
5765
}
5866

5967
// f should be passed the resulting connection and the connection headers
6068
onConnection(f) {
61-
this.webSocketServer.on('connection', (connection, req) => {
69+
this.implementation.on('connection', (connection, req) => {
6270
connection.isAlive = true;
6371
connection.on('pong', () => {
6472
connection.isAlive = true;

‎package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@
2121
"fix:js": "npm run lint:js -- --fix",
2222
"fix": "npm-run-all fix:js fmt",
2323
"commitlint": "commitlint --from=master",
24-
"test:only": "jest --forceExit",
24+
"test:only": "jest",
2525
"test:coverage": "npm run test:only -- --coverage",
2626
"test:watch": "npm run test:coverage --watch",
2727
"test": "npm run test:coverage",

‎test/server/Server.test.js

+2
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,8 @@ describe('Server', () => {
127127
const emitError = () => server.server.emit('error', new Error('Error !!!'));
128128

129129
expect(emitError).toThrowError();
130+
131+
server.close();
130132
});
131133

132134
// issue: https://github.com/webpack/webpack-dev-server/issues/1724

‎test/server/servers/SockJSServer.test.js

+4-4
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe('SockJSServer', () => {
4848
socketServer.send(connection, 'hello world');
4949
setTimeout(() => {
5050
// the server closes the connection with the client
51-
socketServer.close(connection);
51+
socketServer.closeConnection(connection);
5252
}, 1000);
5353
});
5454

@@ -97,7 +97,7 @@ describe('SockJSServer', () => {
9797
socketServer.onConnection(cb);
9898

9999
expect(() => {
100-
socketServer.socket.emit('connection', null);
100+
socketServer.implementation.emit('connection', null);
101101
}).not.toThrow();
102102
expect(cb.mock.calls[0]).toEqual([null, null]);
103103
});
@@ -149,7 +149,7 @@ describe('SockJSServer', () => {
149149
socketServer.send(connection, 'hello world');
150150
setTimeout(() => {
151151
// the server closes the connection with the client
152-
socketServer.close(connection);
152+
socketServer.closeConnection(connection);
153153
}, 1000);
154154
});
155155

@@ -198,7 +198,7 @@ describe('SockJSServer', () => {
198198
socketServer.onConnection(cb);
199199

200200
expect(() => {
201-
socketServer.socket.emit('connection', null);
201+
socketServer.implementation.emit('connection', null);
202202
}).not.toThrow();
203203
expect(cb.mock.calls[0]).toEqual([null, null]);
204204
});

‎test/server/servers/WebsocketServer.test.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ describe('WebsocketServer', () => {
4141
socketServer.send(connection, 'hello world');
4242
setTimeout(() => {
4343
// the server closes the connection with the client
44-
socketServer.close(connection);
44+
socketServer.closeConnection(connection);
4545
}, 1000);
4646
});
4747

‎test/server/webSocketServer-option.test.js

+49-17
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ const BaseServer = require('../../lib/servers/BaseServer');
1313
const port = require('../ports-map')['webSocketServer-option'];
1414

1515
describe('webSocketServer', () => {
16-
describe.only('server', () => {
16+
describe('server', () => {
1717
let mockedTestServer;
1818
let testServer;
1919
let server;
@@ -56,6 +56,7 @@ describe('webSocketServer', () => {
5656
class MockServer {
5757
// eslint-disable-next-line no-empty-function
5858
onConnection() {}
59+
close() {}
5960
}
6061
);
6162
});
@@ -199,7 +200,7 @@ describe('webSocketServer', () => {
199200
constructor(serv) {
200201
super(serv);
201202

202-
this.socket = sockjs.createServer({
203+
this.implementation = sockjs.createServer({
203204
// Use provided up-to-date sockjs-client
204205
sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js',
205206
// Limit useless logs
@@ -214,23 +215,33 @@ describe('webSocketServer', () => {
214215
},
215216
});
216217

217-
this.socket.installHandlers(this.server.server, {
218+
this.implementation.installHandlers(this.server.server, {
218219
prefix: 'ws',
219220
});
220221

221222
customServerUsed = true;
222223
}
223224

224-
send(connection, message) {
225-
connection.write(message);
225+
close(callback) {
226+
[...this.server.sockets].forEach((socket) => {
227+
this.closeConnection(socket);
228+
});
229+
230+
if (callback) {
231+
callback();
232+
}
226233
}
227234

228-
close(connection) {
235+
closeConnection(connection) {
229236
connection.close();
230237
}
231238

239+
send(connection, message) {
240+
connection.write(message);
241+
}
242+
232243
onConnection(f) {
233-
this.socket.on('connection', (connection) => {
244+
this.implementation.on('connection', (connection) => {
234245
f(connection, connection.headers);
235246
});
236247
}
@@ -278,21 +289,22 @@ describe('webSocketServer', () => {
278289
webSocketServer: class MySockJSServer extends BaseServer {
279290
constructor(serv) {
280291
super(serv);
281-
282-
this.socket = sockjs.createServer({
292+
this.implementation = sockjs.createServer({
283293
// Use provided up-to-date sockjs-client
284294
sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js',
285295
// Limit useless logs
286296
log: (severity, line) => {
287297
if (severity === 'error') {
288298
this.server.logger.error(line);
299+
} else if (severity === 'info') {
300+
this.server.logger.log(line);
289301
} else {
290302
this.server.logger.debug(line);
291303
}
292304
},
293305
});
294306

295-
this.socket.installHandlers(this.server.server, {
307+
this.implementation.installHandlers(this.server.server, {
296308
prefix: '/ws',
297309
});
298310
}
@@ -301,12 +313,22 @@ describe('webSocketServer', () => {
301313
connection.write(message);
302314
}
303315

304-
close(connection) {
316+
close(callback) {
317+
[...this.server.sockets].forEach((socket) => {
318+
this.closeConnection(socket);
319+
});
320+
321+
if (callback) {
322+
callback();
323+
}
324+
}
325+
326+
closeConnection(connection) {
305327
connection.close();
306328
}
307329

308330
onConnection(f) {
309-
this.socket.on('connection', (connection) => {
331+
this.implementation.on('connection', (connection) => {
310332
f(connection);
311333
});
312334
}
@@ -377,7 +399,7 @@ describe('webSocketServer', () => {
377399
webSocketServer: class MySockJSServer extends BaseServer {
378400
constructor(serv) {
379401
super(serv);
380-
this.socket = sockjs.createServer({
402+
this.implementation = sockjs.createServer({
381403
// Use provided up-to-date sockjs-client
382404
sockjs_url: '/__webpack_dev_server__/sockjs.bundle.js',
383405
// Limit useless logs
@@ -392,7 +414,7 @@ describe('webSocketServer', () => {
392414
},
393415
});
394416

395-
this.socket.installHandlers(this.server.server, {
417+
this.implementation.installHandlers(this.server.server, {
396418
prefix: '/ws',
397419
});
398420
}
@@ -401,12 +423,22 @@ describe('webSocketServer', () => {
401423
connection.write(message);
402424
}
403425

404-
close(connection) {
426+
close(callback) {
427+
[...this.server.sockets].forEach((socket) => {
428+
this.closeConnection(socket);
429+
});
430+
431+
if (callback) {
432+
callback();
433+
}
434+
}
435+
436+
closeConnection(connection) {
405437
connection.close();
406438
}
407439

408440
onConnection(f) {
409-
this.socket.on('connection', (connection) => {
441+
this.implementation.on('connection', (connection) => {
410442
f(connection, {
411443
host: null,
412444
});
@@ -530,7 +562,7 @@ describe('webSocketServer', () => {
530562
expect(mockServerInstance.onConnection.mock.calls).toMatchSnapshot();
531563
// the only call to send() here should be an invalid header message
532564
expect(mockServerInstance.send.mock.calls).toMatchSnapshot();
533-
expect(mockServerInstance.close.mock.calls).toMatchSnapshot();
565+
expect(mockServerInstance.closeConnection.mock.calls).toMatchSnapshot();
534566
// onConnectionClose should never get called since the client should be closed first
535567
expect(mockServerInstance.onConnectionClose.mock.calls.length).toEqual(
536568
0

0 commit comments

Comments
 (0)