feat: random port retry logic (#1692)

Author: mistic

Diff for: bin/webpack-dev-server.js

@@ -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;
 
@@ -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),
+  3
+);
+
 function processOptions(config) {
   // processOptions {Promise}
   if (typeof config.then === 'function') {
@@ -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) {
@@ -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);

Diff for: lib/utils/findPort.js

@@ -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;

Diff for: lib/utils/tryParseInt.js

@@ -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;

Diff for: test/Util.test.js

@@ -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', () => {
@@ -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();
+    });
+  });
+});

Diff for: test/cli.test.js

@@ -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();
+      }
+    });
+  });
 });