refactor: don't listen to http error event (#1924)

bin/webpack-dev-server.js

@@ -23,10 +23,8 @@ const Server = require('../lib/Server');
 const colors = require('../lib/utils/colors');
 const createConfig = require('../lib/utils/createConfig');
 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 tryParseInt = require('../lib/utils/tryParseInt');
 
 let server;
 
@@ -197,7 +195,7 @@ function startDevServer(config, options) {
       });
     });
   } else {
-    decidePort(server, options.port)
+    findPort(options.port)
       .then((port) => {
         options.port = port;
         server.listen(options.port, options.host, (err) => {
@@ -212,29 +210,4 @@ function startDevServer(config, options) {
   }
 }
 
-function decidePort(server, port) {
-  return new Promise((resolve, reject) => {
-    if (typeof port !== 'undefined') {
-      resolve(port);
-    } else {
-      // 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
-      );
-
-      // only run port finder if no port as been specified
-      findPort(server, DEFAULT_PORT, defaultPortRetry, (err, port) => {
-        if (err) {
-          reject(err);
-        }
-        resolve(port);
-      });
-    }
-  });
-}
-
 processOptions(config);

lib/utils/findPort.js

@@ -1,35 +1,28 @@
 '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);
+const { getPortPromise } = require('portfinder');
+const defaultTo = require('./defaultTo');
+const tryParseInt = require('./tryParseInt');
+
+const defaultPort = 8080;
+
+function findPort(port) {
+  if (typeof port !== 'undefined') {
+    return Promise.resolve(port);
+  }
+  // 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
+  );
+
+  return getPortPromise({
+    port: defaultPort,
+    stopPort: defaultPort + defaultPortRetry,
   });
-
-  runPortFinder(defaultPort, portFinderRunCb);
 }
 
 module.exports = findPort;

test/Util.test.js

@@ -1,7 +1,6 @@
 'use strict';
 
-const EventEmitter = require('events');
-const assert = require('assert');
+const http = require('http');
 const webpack = require('webpack');
 const internalIp = require('internal-ip');
 const Server = require('../lib/Server');
@@ -119,40 +118,65 @@ describe('check utility functions', () => {
 });
 
 describe('findPort cli utility function', () => {
-  let mockServer = null;
-
-  beforeEach(() => {
-    mockServer = {
-      listeningApp: new EventEmitter(),
-    };
-  });
+  let dummyServers = [];
 
   afterEach(() => {
-    mockServer.listeningApp.removeAllListeners('error');
-    mockServer = null;
+    delete process.env.DEFAULT_PORT_RETRY;
+
+    return dummyServers
+      .reduce((p, server) => {
+        return p.then(() => {
+          return new Promise((resolve) => {
+            server.close(resolve);
+          });
+        });
+      }, Promise.resolve())
+      .then(() => {
+        dummyServers = [];
+      });
   });
 
-  it('should find empty port starting from defaultPort', (done) => {
-    findPort(mockServer, 8180, 3, (err, port) => {
-      assert(err == null);
-      assert(port === 8180);
-      done();
+  function createDummyServers(n) {
+    return [...new Array(n)].reduce((p, _, i) => {
+      return p.then(() => {
+        return new Promise((resolve) => {
+          const server = http.createServer();
+          dummyServers.push(server);
+          server.listen(8080 + i, resolve);
+        });
+      });
+    }, Promise.resolve());
+  }
+
+  it('should return the port when the port is specified', () => {
+    process.env.DEFAULT_PORT_RETRY = 5;
+
+    return findPort(8082).then((port) => {
+      expect(port).toEqual(8082);
     });
   });
 
-  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();
-    });
+  it('should retry finding the port for up to defaultPortRetry times', () => {
+    const retryCount = 5;
+
+    process.env.DEFAULT_PORT_RETRY = retryCount;
+
+    return createDummyServers(retryCount)
+      .then(findPort)
+      .then((port) => {
+        expect(port).toEqual(8080 + retryCount);
+      });
+  });
+
+  it("should throw the error when the port isn't found", () => {
+    const retryCount = 5;
+
+    process.env.DEFAULT_PORT_RETRY = retryCount;
+
+    return createDummyServers(10)
+      .then(findPort)
+      .catch((err) => {
+        expect(err.message).toMatchSnapshot();
+      });
   });
 });

test/__snapshots__/Util.test.js.snap

@@ -0,0 +1,3 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`findPort cli utility function should throw the error when the port isn't found 1`] = `"No open ports found in between 8080 and 8085"`;