Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Random port retry logic #1692

Merged
merged 9 commits into from
Feb 26, 2019
55 changes: 31 additions & 24 deletions bin/webpack-dev-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,25 @@ const debug = require('debug')('webpack-dev-server');
const fs = require('fs');
const net = require('net');

const portfinder = require('portfinder');
const importLocal = require('import-local');

const yargs = require('yargs');
const webpack = require('webpack');

const options = require('./options');

const Server = require('../lib/Server');

const addEntries = require('../lib/utils/addEntries');
const colors = require('../lib/utils/colors');
const createConfig = require('../lib/utils/createConfig');
const createDomain = require('../lib/utils/createDomain');
const createLogger = require('../lib/utils/createLogger');
const defaultTo = require('../lib/utils/defaultTo');
const findPort = require('../lib/utils/findPort');
const getVersions = require('../lib/utils/getVersions');
const runBonjour = require('../lib/utils/runBonjour');
const status = require('../lib/utils/status');
const tryParseInt = require('../lib/utils/tryParseInt');

let server;

Expand Down Expand Up @@ -93,6 +94,15 @@ const config = require('webpack-cli/bin/convert-argv')(yargs, argv, {
// we should use portfinder.
const DEFAULT_PORT = 8080;

// Try to find unused port and listen on it for 3 times,
// if port is not specified in options.
// Because NaN == null is false, defaultTo fails if parseInt returns NaN
// so the tryParseInt function is introduced to handle NaN
const defaultPortRetry = defaultTo(
tryParseInt(process.env.DEFAULT_PORT_RETRY),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO: I think we should leave process.env.DEFAULT_PORT_RETRY as a document.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hiroppy can you create issue in webpack docs repo?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think DEFAULT_PORT_RETRY should be added in devServer because currently, we don' use process.env. see https://webpack.js.org/configuration/dev-server/

@evilebottnawi what do you think?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@hiroppy yep, after merge we need open issue here https://github.com/webpack/webpack.js.org 👍

3
);

function processOptions(config) {
// processOptions {Promise}
if (typeof config.then === 'function') {
Expand All @@ -106,24 +116,7 @@ function processOptions(config) {
}

const options = createConfig(config, argv, { port: DEFAULT_PORT });

portfinder.basePort = DEFAULT_PORT;

if (options.port != null) {
startDevServer(config, options);

return;
}

portfinder.getPort((err, port) => {
if (err) {
throw err;
}

options.port = port;

startDevServer(config, options);
});
startDevServer(config, options);
}

function startDevServer(config, options) {
Expand Down Expand Up @@ -209,21 +202,35 @@ function startDevServer(config, options) {
status(uri, options, log, argv.color);
});
});
} else {
return;
}

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

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

const uri = createDomain(options, server.listeningApp) + suffix;

status(uri, options, log, argv.color);
});
};

if (options.port) {
startServer();
return;
}

// only run port finder if no port as been specified
findPort(server, DEFAULT_PORT, defaultPortRetry, (err, port) => {
if (err) {
throw err;
}
options.port = port;
startServer();
});
}

processOptions(config);
35 changes: 35 additions & 0 deletions lib/utils/findPort.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
'use strict';

const portfinder = require('portfinder');

function runPortFinder(defaultPort, cb) {
portfinder.basePort = defaultPort;
portfinder.getPort((err, port) => {
cb(err, port);
});
}

function findPort(server, defaultPort, defaultPortRetry, fn) {
let tryCount = 0;
const portFinderRunCb = (err, port) => {
tryCount += 1;
fn(err, port);
};

server.listeningApp.on('error', (err) => {
if (err && err.code !== 'EADDRINUSE') {
throw err;
}

if (tryCount >= defaultPortRetry) {
fn(err);
return;
}

runPortFinder(defaultPort, portFinderRunCb);
});

runPortFinder(defaultPort, portFinderRunCb);
}

module.exports = findPort;
11 changes: 11 additions & 0 deletions lib/utils/tryParseInt.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
'use strict';

function tryParseInt(input) {
const output = parseInt(input, 10);
if (Number.isNaN(output)) {
return null;
}
return output;
}

module.exports = tryParseInt;
38 changes: 38 additions & 0 deletions test/Util.test.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
'use strict';

const EventEmitter = require('events');
const assert = require('assert');
const webpack = require('webpack');
const internalIp = require('internal-ip');
const Server = require('../lib/Server');
const createDomain = require('../lib/utils/createDomain');
const findPort = require('../lib/utils/findPort');
const config = require('./fixtures/simple-config/webpack.config');

describe('check utility functions', () => {
Expand Down Expand Up @@ -107,3 +110,38 @@ describe('check utility functions', () => {
});
});
});

describe('findPort cli utility function', () => {
let mockServer = null;
beforeEach(() => {
mockServer = {
listeningApp: new EventEmitter(),
};
});
afterEach(() => {
mockServer.listeningApp.removeAllListeners('error');
mockServer = null;
});
it('should find empty port starting from defaultPort', (done) => {
findPort(mockServer, 8180, 3, (err, port) => {
assert(err == null);
assert(port === 8180);
done();
});
});
it('should retry finding port for up to defaultPortRetry times', (done) => {
let count = 0;
const defaultPortRetry = 5;
findPort(mockServer, 8180, defaultPortRetry, (err) => {
if (err == null) {
count += 1;
const mockError = new Error('EADDRINUSE');
mockError.code = 'EADDRINUSE';
mockServer.listeningApp.emit('error', mockError);
return;
}
assert(count === defaultPortRetry);
done();
});
});
});
61 changes: 61 additions & 0 deletions test/cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,4 +95,65 @@ describe('CLI', () => {
done();
});
});

it('should use different random port when multiple instances are started on different processes', (done) => {
const cliPath = path.resolve(__dirname, '../bin/webpack-dev-server.js');
const examplePath = path.resolve(__dirname, '../examples/cli/public');

const cp = execa('node', [cliPath], { cwd: examplePath });
const cp2 = execa('node', [cliPath], { cwd: examplePath });

const runtime = {
cp: {
port: null,
done: false,
},
cp2: {
port: null,
done: false,
},
};

cp.stdout.on('data', (data) => {
const bits = data.toString();
const portMatch = /Project is running at http:\/\/localhost:(\d*)\//.exec(
bits
);
if (portMatch) {
runtime.cp.port = portMatch[1];
}
if (/Compiled successfully/.test(bits)) {
expect(cp.pid !== 0).toBe(true);
cp.kill('SIGINT');
}
});
cp2.stdout.on('data', (data) => {
const bits = data.toString();
const portMatch = /Project is running at http:\/\/localhost:(\d*)\//.exec(
bits
);
if (portMatch) {
runtime.cp2.port = portMatch[1];
}
if (/Compiled successfully/.test(bits)) {
expect(cp.pid !== 0).toBe(true);
cp2.kill('SIGINT');
}
});

cp.on('exit', () => {
runtime.cp.done = true;
if (runtime.cp2.done) {
expect(runtime.cp.port !== runtime.cp2.port).toBe(true);
done();
}
});
cp2.on('exit', () => {
runtime.cp2.done = true;
if (runtime.cp.done) {
expect(runtime.cp.port !== runtime.cp2.port).toBe(true);
done();
}
});
});
});