Skip to content

Commit

Permalink
refactor(server): move socket handling into server.listen (#2061)
Browse files Browse the repository at this point in the history
  • Loading branch information
knagaitsev authored and alexander-akait committed Jan 29, 2020
1 parent ff45f5e commit 445f8d5
Show file tree
Hide file tree
Showing 9 changed files with 484 additions and 68 deletions.
34 changes: 0 additions & 34 deletions bin/webpack-dev-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@

/* eslint-disable no-shadow, no-console */

const fs = require('fs');
const net = require('net');
const debug = require('debug')('webpack-dev-server');
const importLocal = require('import-local');
const yargs = require('yargs');
Expand Down Expand Up @@ -116,42 +114,10 @@ function startDevServer(config, options) {
}

if (options.socket) {
server.listeningApp.on('error', (e) => {
if (e.code === 'EADDRINUSE') {
const clientSocket = new net.Socket();

clientSocket.on('error', (err) => {
if (err.code === 'ECONNREFUSED') {
// No other server listening on this socket so it can be safely removed
fs.unlinkSync(options.socket);

server.listen(options.socket, options.host, (error) => {
if (error) {
throw error;
}
});
}
});

clientSocket.connect({ path: options.socket }, () => {
throw new Error('This socket is already used');
});
}
});

server.listen(options.socket, options.host, (err) => {
if (err) {
throw err;
}

// chmod 666 (rw rw rw)
const READ_WRITE = 438;

fs.chmod(options.socket, READ_WRITE, (err) => {
if (err) {
throw err;
}
});
});
} else {
server.listen(options.port, options.host, (err) => {
Expand Down
39 changes: 36 additions & 3 deletions lib/Server.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@ const runBonjour = require('./utils/runBonjour');
const routes = require('./utils/routes');
const getSocketServerImplementation = require('./utils/getSocketServerImplementation');
const handleStdin = require('./utils/handleStdin');
const tryParseInt = require('./utils/tryParseInt');
const startUnixSocket = require('./utils/startUnixSocket');
const schema = require('./options.json');

// Workaround for node ^8.6.0, ^9.0.0
Expand Down Expand Up @@ -742,23 +744,54 @@ class Server {
listen(port, hostname, fn) {
this.hostname = hostname;

return this.listeningApp.listen(port, hostname, (err) => {
const setupCallback = () => {
this.createSocketServer();

if (this.options.bonjour) {
runBonjour(this.options);
}

this.showStatus();
};

if (fn) {
// between setupCallback and userCallback should be any other needed handling,
// specifically so that things are done in the right order to prevent
// backwards compatability issues
let userCallbackCalled = false;
const userCallback = (err) => {
if (fn && !userCallbackCalled) {
userCallbackCalled = true;
fn.call(this.listeningApp, err);
}
};

const onListeningCallback = () => {
if (typeof this.options.onListening === 'function') {
this.options.onListening(this);
}
});
};

const fullCallback = (err) => {
setupCallback();
userCallback(err);
onListeningCallback();
};

// try to follow the Node standard in terms of deciding
// whether this is a socket or a port that we will listen on:
// https://github.com/nodejs/node/blob/64219741218aa87e259cf8257596073b8e747f0a/lib/net.js#L196
if (typeof port === 'string' && tryParseInt(port) === null) {
// in this case the "port" argument is actually a socket path
const socket = port;
// set this so that status helper can identify how the project is being run correctly
this.options.socket = socket;

startUnixSocket(this.listeningApp, socket, fullCallback);
} else {
this.listeningApp.listen(port, hostname, fullCallback);
}

return this.listeningApp;
}

close(cb) {
Expand Down
62 changes: 62 additions & 0 deletions lib/utils/startUnixSocket.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
'use strict';

const fs = require('fs');
const net = require('net');
const { promisify } = require('util');

const accessAsync = promisify(fs.access);

async function startUnixSocket(listeningApp, socket, cb) {
const chmodSocket = (done) => {
// chmod 666 (rw rw rw) - octal
const READ_WRITE = 438;
fs.chmod(socket, READ_WRITE, done);
};

const startSocket = () => {
listeningApp.on('error', (err) => {
cb(err);
});

// 511 is the default value for the server.listen backlog parameter
// https://nodejs.org/api/net.html#net_server_listen
listeningApp.listen(socket, 511, (err) => {
if (err) {
cb(err);
} else {
chmodSocket(cb);
}
});
};

try {
await accessAsync(socket, fs.constants.F_OK);
} catch (e) {
// file does not exist
startSocket();
return;
}

// file exists

const clientSocket = new net.Socket();

clientSocket.on('error', (err) => {
if (err.code === 'ECONNREFUSED' || err.code === 'ENOTSOCK') {
// No other server listening on this socket so it can be safely removed
fs.unlinkSync(socket);

startSocket();
}
});

clientSocket.connect({ path: socket }, () => {
// if a client socket successfully connects to the given socket path,
// it means that the socket is in use
const err = new Error('This socket is already used');
clientSocket.destroy();
cb(err);
});
}

module.exports = startUnixSocket;
15 changes: 8 additions & 7 deletions test/cli/cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ const { unlinkAsync } = require('../helpers/fs');
const testBin = require('../helpers/test-bin');
const port1 = require('../ports-map').cli[0];
const timer = require('../helpers/timer');
const { skipTestOnWindows } = require('../helpers/conditional-test');

const httpsCertificateDirectory = resolve(
__dirname,
Expand Down Expand Up @@ -94,14 +95,14 @@ describe('CLI', () => {
.catch(done);
});

// The Unix socket to listen to (instead of a host).
it('--socket', async () => {
const socketPath = join('.', 'webpack.sock');
const { exitCode, stdout } = await testBin(`--socket ${socketPath}`);
expect(exitCode).toEqual(0);
describe('Unix socket', () => {
if (skipTestOnWindows('Unix sockets are not supported on Windows')) {
return;
}

if (process.platform !== 'win32') {
expect(stdout.includes(socketPath)).toBe(true);
// The Unix socket to listen to (instead of a host).
it('--socket', async () => {
const socketPath = join('.', 'webpack.sock');

testBin(`--socket ${socketPath}`)
.then((output) => {
Expand Down
22 changes: 17 additions & 5 deletions test/helpers/test-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,19 @@ function startFullSetup(config, options, done) {

server = new Server(compiler, options);

const port = Object.prototype.hasOwnProperty.call(options, 'port')
? options.port
: 8080;
// originally the fallback default was 8080, but it should be
// undefined so that the server.listen method can choose it for us,
// and thus prevent port mapping collision between tests
let port;
if (Object.prototype.hasOwnProperty.call(options, 'port')) {
port = options.port;
} else if (Object.prototype.hasOwnProperty.call(options, 'socket')) {
port = options.socket;
} else {
// TODO: remove this when findPort is implemented in the server.listen method
port = 8080;
}

const host = Object.prototype.hasOwnProperty.call(options, 'host')
? options.host
: 'localhost';
Expand All @@ -65,10 +75,12 @@ function startFullSetup(config, options, done) {

function startAwaitingCompilationFullSetup(config, options, done) {
let readyCount = 0;
const ready = () => {
let err;
const ready = (e) => {
err = e instanceof Error || (typeof e === 'object' && e.code) ? e : err;
readyCount += 1;
if (readyCount === 2) {
done();
done(err);
}
};

Expand Down
30 changes: 30 additions & 0 deletions test/helpers/test-unix-socket.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
'use strict';

const http = require('http');

const TestUnixSocket = class TestUnixSocket {
constructor() {
this.server = http.createServer();
this.sockets = new Set();
this.server.on('connection', (socket) => {
this.sockets.add(socket);
socket.on('close', () => {
this.sockets.delete(socket);
});
});
}

close(done) {
if (this.server.listening) {
// get rid of connected sockets
for (const socket of this.sockets.values()) {
socket.destroy();
}
this.server.close(done);
} else {
done();
}
}
};

module.exports = TestUnixSocket;
77 changes: 58 additions & 19 deletions test/server/onListening-option.test.js
Original file line number Diff line number Diff line change
@@ -1,32 +1,71 @@
'use strict';

const { unlink } = require('fs');
const { join } = require('path');
const testServer = require('../helpers/test-server');
const config = require('../fixtures/simple-config/webpack.config');
const port = require('../ports-map')['onListening-option'];
const { skipTestOnWindows } = require('../helpers/conditional-test');

describe('onListening option', () => {
let onListeningIsRunning = false;

beforeAll((done) => {
testServer.start(
config,
{
onListening: (devServer) => {
if (!devServer) {
throw new Error('webpack-dev-server is not defined');
}

onListeningIsRunning = true;
describe('with host and port', () => {
let onListeningIsRunning = false;

beforeAll((done) => {
testServer.start(
config,
{
onListening: (devServer) => {
if (!devServer) {
throw new Error('webpack-dev-server is not defined');
}

onListeningIsRunning = true;
},
port,
},
port,
},
done
);
done
);
});

afterAll(testServer.close);

it('should run onListening callback', () => {
expect(onListeningIsRunning).toBe(true);
});
});

afterAll(testServer.close);
describe('with Unix socket', () => {
if (skipTestOnWindows('Unix sockets are not supported on Windows')) {
return;
}

const socketPath = join('.', 'onListening.webpack.sock');
let onListeningIsRunning = false;

beforeAll((done) => {
testServer.start(
config,
{
onListening: (devServer) => {
if (!devServer) {
throw new Error('webpack-dev-server is not defined');
}

onListeningIsRunning = true;
},
socket: socketPath,
},
done
);
});

afterAll(testServer.close);

it('should run onListening callback', (done) => {
expect(onListeningIsRunning).toBe(true);

it('should runs onListening callback', () => {
expect(onListeningIsRunning).toBe(true);
unlink(socketPath, done);
});
});
});
Loading

0 comments on commit 445f8d5

Please sign in to comment.