Skip to content

feat: Implement random port retry logic #1577

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

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 36 additions & 1 deletion bin/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
space-before-function-paren
*/
const open = require('opn');
const portfinder = require('portfinder');

const colors = {
info (useColor, msg) {
Expand Down Expand Up @@ -105,10 +106,44 @@ function bonjour (options) {
});
}

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

function findPort(server, defaultPort, defaultPortRetry, fn) {
let tryCount = 0;
Copy link
Contributor

Choose a reason for hiding this comment

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

@u9520107 Just a simple NIT here, I think the code could be more readable in the following way, but feel free to not apply the changes!

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

function findPort(server, defaultPort, defaultPortRetry, fn) {
  let tryCount = 0;

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

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

    runPortFinder(tryCount, defaultPort, fn);
  });

  runPortFinder(tryCount, defaultPort, fn);
}

server.listeningApp.on('error', (err) => {
if (err.code === 'EADDRINUSE') {
if (tryCount < defaultPortRetry) {
portfinder.basePort = defaultPort;
tryCount += 1;
portfinder.getPort((err, port) => {
fn(err, port);
});
} else {
fn(err);
}
} else {
throw err;
}
});
portfinder.basePort = defaultPort;
tryCount += 1;
portfinder.getPort((err, port) => {
fn(err, port);
});
}

module.exports = {
status,
colors,
version,
bonjour,
defaultTo
defaultTo,
tryParseInt,
findPort
};
51 changes: 28 additions & 23 deletions bin/webpack-dev-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ const fs = require('fs');
const net = require('net');
const path = require('path');

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

const yargs = require('yargs');
Expand All @@ -32,7 +31,9 @@ const {
status,
version,
bonjour,
defaultTo
defaultTo,
tryParseInt,
findPort
} = require('./utils');

const Server = require('../lib/Server');
Expand Down Expand Up @@ -97,6 +98,12 @@ 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), 3);

function processOptions (config) {
// processOptions {Promise}
if (typeof config.then === 'function') {
Expand Down Expand Up @@ -305,23 +312,7 @@ function processOptions (config) {
? defaultTo(options.port, argv.port)
: defaultTo(argv.port, options.port);

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

return;
}

portfinder.basePort = DEFAULT_PORT;

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 @@ -404,21 +395,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) {
bonjour(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);
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('../bin/utils').findPort;
const config = require('./fixtures/simple-config/webpack.config');

describe('check utility functions', () => {
Expand Down Expand Up @@ -104,3 +107,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();
});
});
});
57 changes: 57 additions & 0 deletions test/cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,4 +59,61 @@ describe('CLI', () => {
done();
});
}).timeout(18000);

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)) {
assert(cp.pid !== 0);
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)) {
assert(cp.pid !== 0);
cp2.kill('SIGINT');
}
});

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