From bbef20d345b62c200e13688dc6c6b482e0348ddc Mon Sep 17 00:00:00 2001
From: Tom Auger <31711089+tom-auger@users.noreply.github.com>
Date: Wed, 12 Sep 2018 19:03:52 +0100
Subject: [PATCH] fix: live reload port fallback if port is used (#899)

* Move start reload server into separate module

* Find an unused port when starting the live reload server

* Move findUnusedPort into module

* Add tests for findUnusedPort module

* Refactor findUnusedPort

* Move starting of servers into separate module and add tests

* Remove unused constants.js

* Zap extra line breaks

* Add tests for liveReloadServer

* Rename serverController to start

* Move start into lib/server

* Add portfinder package

* Replace findUnusedPort with portfinder

* nits
---
 lib/core/Site.js                              |  13 +-
 .../__mocks__/tiny-lr.js}                     |   6 +-
 lib/server/__tests__/liveReloadServer.test.js |  26 ++++
 lib/server/__tests__/start.test.js            | 138 ++++++++++++++++++
 lib/server/liveReloadServer.js                |  38 +++++
 lib/server/server.js                          |  26 +---
 lib/server/start.js                           |  51 +++++++
 lib/start-server.js                           |  46 +-----
 package.json                                  |   1 +
 yarn.lock                                     |  30 ++--
 10 files changed, 291 insertions(+), 84 deletions(-)
 rename lib/{core/constants.js => server/__mocks__/tiny-lr.js} (69%)
 create mode 100644 lib/server/__tests__/liveReloadServer.test.js
 create mode 100644 lib/server/__tests__/start.test.js
 create mode 100644 lib/server/liveReloadServer.js
 create mode 100644 lib/server/start.js

diff --git a/lib/core/Site.js b/lib/core/Site.js
index ad45ac4647..7d83643a58 100644
--- a/lib/core/Site.js
+++ b/lib/core/Site.js
@@ -13,7 +13,7 @@ const Head = require('./Head.js');
 
 const Footer = require(`${process.cwd()}/core/Footer.js`);
 const translation = require('../server/translation.js');
-const constants = require('./constants');
+const liveReloadServer = require('../server/liveReloadServer.js');
 const {idx} = require('./utils.js');
 
 const CWD = process.cwd();
@@ -36,6 +36,8 @@ class Site extends React.Component {
       (this.props.url || 'index.html');
     let docsVersion = this.props.version;
 
+    const liveReloadScriptUrl = liveReloadServer.getReloadScriptUrl();
+
     if (!docsVersion && fs.existsSync(`${CWD}/versions.json`)) {
       const latestVersion = require(`${CWD}/versions.json`)[0];
       docsVersion = latestVersion;
@@ -147,13 +149,8 @@ class Site extends React.Component {
               />
             ))}
 
-          {process.env.NODE_ENV === 'development' && (
-            <script
-              src={`http://localhost:${
-                constants.LIVE_RELOAD_PORT
-              }/livereload.js`}
-            />
-          )}
+          {process.env.NODE_ENV === 'development' &&
+            liveReloadScriptUrl && <script src={liveReloadScriptUrl} />}
         </body>
       </html>
     );
diff --git a/lib/core/constants.js b/lib/server/__mocks__/tiny-lr.js
similarity index 69%
rename from lib/core/constants.js
rename to lib/server/__mocks__/tiny-lr.js
index a99db66323..53fab4ccc1 100644
--- a/lib/core/constants.js
+++ b/lib/server/__mocks__/tiny-lr.js
@@ -5,6 +5,8 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-module.exports = {
-  LIVE_RELOAD_PORT: 35729,
+const tinylrServer = {
+  listen: jest.fn(),
 };
+
+module.exports = () => tinylrServer;
diff --git a/lib/server/__tests__/liveReloadServer.test.js b/lib/server/__tests__/liveReloadServer.test.js
new file mode 100644
index 0000000000..bbdf47e3b6
--- /dev/null
+++ b/lib/server/__tests__/liveReloadServer.test.js
@@ -0,0 +1,26 @@
+/**
+ * Copyright (c) 2017-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+jest.mock('gaze');
+jest.mock('../readMetadata.js');
+jest.mock('tiny-lr');
+
+// When running Jest the siteConfig import fails because siteConfig doesn't exist
+// relative to the cwd of the tests. Rather than mocking out cwd just mock
+// siteConfig virtually.
+jest.mock(`${process.cwd()}/siteConfig.js`, () => jest.fn(), {virtual: true});
+
+const liveReloadServer = require('../liveReloadServer.js');
+
+describe('get reload script', () => {
+  test('when server started, returns url with correct port', () => {
+    const port = 1234;
+    liveReloadServer.start(port);
+    const expectedUrl = `http://localhost:${port}/livereload.js`;
+    expect(liveReloadServer.getReloadScriptUrl()).toBe(expectedUrl);
+  });
+});
diff --git a/lib/server/__tests__/start.test.js b/lib/server/__tests__/start.test.js
new file mode 100644
index 0000000000..e175cc4a2f
--- /dev/null
+++ b/lib/server/__tests__/start.test.js
@@ -0,0 +1,138 @@
+/**
+ * Copyright (c) 2017-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+const program = require('commander');
+const openBrowser = require('react-dev-utils/openBrowser');
+const portFinder = require('portfinder');
+const liveReloadServer = require('../liveReloadServer.js');
+const server = require('../server.js');
+
+const siteConfig = require(`${process.cwd()}/siteConfig.js`);
+
+// When running Jest the siteConfig import fails because siteConfig doesn't exist
+// relative to the cwd of the tests. Rather than mocking out cwd just mock
+// siteConfig virtually.
+jest.mock(`${process.cwd()}/siteConfig.js`, () => jest.fn(), {virtual: true});
+
+jest.mock('commander');
+jest.mock('react-dev-utils/openBrowser');
+jest.mock('portfinder');
+jest.mock('../liveReloadServer.js');
+jest.mock('../server.js');
+jest.mock('process');
+
+console.log = jest.fn();
+
+const start = require('../start.js');
+
+beforeEach(() => jest.resetAllMocks());
+
+describe('start live reload', () => {
+  test('uses inital port 35729', () => {
+    portFinder.getPortPromise.mockResolvedValue();
+    start.startLiveReloadServer();
+    expect(portFinder.getPortPromise).toHaveBeenCalledWith({port: 35729});
+  });
+
+  test('when an unused port is found, starts the live reload server on that port', () => {
+    expect.assertions(1);
+    const unusedPort = 1234;
+    portFinder.getPortPromise.mockResolvedValue(unusedPort);
+    return start.startLiveReloadServer().then(() => {
+      expect(liveReloadServer.start).toHaveBeenCalledWith(unusedPort);
+    });
+  });
+
+  test('when no unused port found, returns error', () => {
+    expect.assertions(1);
+    const unusedPortError = new Error('no unused port');
+    portFinder.getPortPromise.mockRejectedValue(unusedPortError);
+    return expect(start.startLiveReloadServer()).rejects.toEqual(
+      unusedPortError
+    );
+  });
+});
+
+describe('start server', () => {
+  test('when custom port provided as parameter, uses as inital port', () => {
+    const customPort = 1234;
+    program.port = customPort;
+    portFinder.getPortPromise.mockResolvedValue();
+    start.startServer();
+    expect(portFinder.getPortPromise).toBeCalledWith({port: customPort});
+    delete program.port;
+  });
+
+  test('when port environment variable set and no custom port, used as inital port', () => {
+    const customPort = '4321';
+    process.env.PORT = customPort;
+    portFinder.getPortPromise.mockResolvedValue();
+    start.startServer();
+    expect(portFinder.getPortPromise).toBeCalledWith({port: customPort});
+    delete process.env.PORT;
+  });
+
+  test('when no custom port specified, uses port 3000', () => {
+    portFinder.getPortPromise.mockResolvedValue();
+    start.startServer();
+    expect(portFinder.getPortPromise).toBeCalledWith({port: 3000});
+  });
+
+  test('when unused port found, starts server on that port', () => {
+    expect.assertions(1);
+    const port = 1357;
+    portFinder.getPortPromise.mockResolvedValue(port);
+    return start.startServer().then(() => {
+      expect(server).toHaveBeenCalledWith(port);
+    });
+  });
+
+  test('when unused port found, opens browser to server address', () => {
+    expect.assertions(1);
+    const baseUrl = '/base_url';
+    siteConfig.baseUrl = baseUrl;
+    const port = 2468;
+    portFinder.getPortPromise.mockResolvedValue(port);
+    const expectedServerAddress = `http://localhost:${port}${baseUrl}`;
+    return start.startServer().then(() => {
+      expect(openBrowser).toHaveBeenCalledWith(expectedServerAddress);
+    });
+  });
+});
+
+describe('start docusaurus', () => {
+  test('when watch enabled, starts live reload server', () => {
+    expect.assertions(1);
+    program.watch = true;
+    portFinder.getPortPromise.mockResolvedValue();
+    return start.startDocusaurus().then(() => {
+      expect(liveReloadServer.start).toBeCalled();
+    });
+  });
+
+  test('when live reload fails to start, server still started', () => {
+    expect.assertions(1);
+    program.watch = true;
+    console.warn = jest.fn();
+    portFinder.getPortPromise
+      .mockRejectedValueOnce('could not find live reload port')
+      .mockResolvedValueOnce();
+    return start.startDocusaurus().then(() => {
+      expect(server).toBeCalled();
+    });
+  });
+
+  test('live reload disabled, only starts docusarus server', () => {
+    expect.assertions(2);
+    program.watch = false;
+    portFinder.getPortPromise.mockResolvedValue();
+    return start.startDocusaurus().then(() => {
+      expect(liveReloadServer.start).not.toBeCalled();
+      expect(server).toBeCalled();
+    });
+  });
+});
diff --git a/lib/server/liveReloadServer.js b/lib/server/liveReloadServer.js
new file mode 100644
index 0000000000..754f4b301d
--- /dev/null
+++ b/lib/server/liveReloadServer.js
@@ -0,0 +1,38 @@
+/**
+ * Copyright (c) 2017-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+const gaze = require('gaze');
+const tinylr = require('tiny-lr');
+const readMetadata = require('./readMetadata.js');
+
+let reloadScriptUrl;
+
+function start(port) {
+  process.env.NODE_ENV = 'development';
+  const server = tinylr();
+  server.listen(port, () => {
+    console.log('LiveReload server started on port %d', port);
+  });
+
+  gaze(
+    [`../${readMetadata.getDocsPath()}/**/*`, '**/*', '!node_modules/**/*'],
+    function() {
+      this.on('all', () => {
+        server.notifyClients(['/']);
+      });
+    }
+  );
+
+  reloadScriptUrl = `http://localhost:${port}/livereload.js`;
+}
+
+const getReloadScriptUrl = () => reloadScriptUrl;
+
+module.exports = {
+  start,
+  getReloadScriptUrl,
+};
diff --git a/lib/server/server.js b/lib/server/server.js
index 87a59d9dc2..b09df60294 100644
--- a/lib/server/server.js
+++ b/lib/server/server.js
@@ -7,7 +7,7 @@
 
 /* eslint-disable no-cond-assign */
 
-function execute(port, options) {
+function execute(port) {
   const extractTranslations = require('../write-translations');
   const metadataUtils = require('./metadataUtils');
   const blog = require('./blog');
@@ -22,9 +22,6 @@ function execute(port, options) {
   const mkdirp = require('mkdirp');
   const glob = require('glob');
   const chalk = require('chalk');
-  const gaze = require('gaze');
-  const tinylr = require('tiny-lr');
-  const constants = require('../core/constants');
   const translate = require('./translate');
   const {renderToStaticMarkupWithDoctype} = require('./renderUtils');
   const feed = require('./feed');
@@ -105,26 +102,6 @@ function execute(port, options) {
     });
   }
 
-  function startLiveReload() {
-    process.env.NODE_ENV = 'development';
-    const server = tinylr();
-    server.listen(constants.LIVE_RELOAD_PORT, () => {
-      console.log(
-        'LiveReload server started on port %d',
-        constants.LIVE_RELOAD_PORT
-      );
-    });
-
-    gaze(
-      [`../${readMetadata.getDocsPath()}/**/*`, '**/*', '!node_modules/**/*'],
-      function() {
-        this.on('all', () => {
-          server.notifyClients(['/']);
-        });
-      }
-    );
-  }
-
   reloadMetadata();
   reloadMetadataBlog();
   extractTranslations();
@@ -398,7 +375,6 @@ function execute(port, options) {
     requestFile(`http://localhost:${port}${req.path}.html`, res, next);
   });
 
-  if (options.watch) startLiveReload();
   app.listen(port);
 }
 
diff --git a/lib/server/start.js b/lib/server/start.js
new file mode 100644
index 0000000000..626b03e4a1
--- /dev/null
+++ b/lib/server/start.js
@@ -0,0 +1,51 @@
+/**
+ * Copyright (c) 2017-present, Facebook, Inc.
+ *
+ * This source code is licensed under the MIT license found in the
+ * LICENSE file in the root directory of this source tree.
+ */
+
+const program = require('commander');
+const openBrowser = require('react-dev-utils/openBrowser');
+const portFinder = require('portfinder');
+const liveReloadServer = require('./liveReloadServer.js');
+const server = require('./server.js');
+
+const CWD = process.cwd();
+
+function startLiveReloadServer() {
+  const promise = portFinder.getPortPromise({port: 35729}).then(port => {
+    liveReloadServer.start(port);
+  });
+  return promise;
+}
+
+function startServer() {
+  const initialServerPort =
+    parseInt(program.port, 10) || process.env.PORT || 3000;
+  const promise = portFinder
+    .getPortPromise({port: initialServerPort})
+    .then(port => {
+      server(port);
+      const {baseUrl} = require(`${CWD}/siteConfig.js`);
+      const serverAddress = `http://localhost:${port}${baseUrl}`;
+      console.log('Docusaurus server started on port %d', port);
+      openBrowser(serverAddress);
+    });
+  return promise;
+}
+
+function startDocusaurus() {
+  if (program.watch) {
+    return startLiveReloadServer()
+      .catch(ex => console.warn(`Failed to start live reload server: ${ex}`))
+      .then(() => startServer());
+  }
+  return startServer();
+}
+
+module.exports = {
+  startDocusaurus,
+  startServer,
+  startLiveReloadServer,
+};
diff --git a/lib/start-server.js b/lib/start-server.js
index e5df435113..c31f9c324e 100755
--- a/lib/start-server.js
+++ b/lib/start-server.js
@@ -21,12 +21,12 @@ require('babel-register')({
 const chalk = require('chalk');
 const fs = require('fs');
 const program = require('commander');
-const openBrowser = require('react-dev-utils/openBrowser');
-const tcpPortUsed = require('tcp-port-used');
 
 const CWD = process.cwd();
 const env = require('./server/env.js');
 
+const {startDocusaurus} = require('./server/start.js');
+
 if (!fs.existsSync(`${CWD}/siteConfig.js`)) {
   console.error(
     chalk.red('Error: No siteConfig.js file found in website folder!')
@@ -44,41 +44,7 @@ program
   .option('--no-watch', 'Toggle live reload file watching')
   .parse(process.argv);
 
-let port = parseInt(program.port, 10) || process.env.PORT || 3000;
-let numAttempts = 0;
-const MAX_ATTEMPTS = 10;
-
-function checkPort() {
-  tcpPortUsed
-    .check(port, 'localhost')
-    .then(inUse => {
-      if (inUse && numAttempts >= MAX_ATTEMPTS) {
-        console.log(
-          'Reached max attempts, exiting. Please open up some ports or ' +
-            'increase the number of attempts and try again.'
-        );
-        process.exit(1);
-      } else if (inUse) {
-        console.error(chalk.red(`Port ${port} is in use`));
-        // Try again but with port + 1
-        port += 1;
-        numAttempts += 1;
-        checkPort();
-      } else {
-        // start local server on specified port
-        const server = require('./server/server.js');
-        server(port, program.opts());
-        const {baseUrl} = require(`${CWD}/siteConfig.js`);
-        const host = `http://localhost:${port}${baseUrl}`;
-        console.log('Docusaurus server started on port %d', port);
-        openBrowser(host);
-      }
-    })
-    .catch(ex => {
-      setTimeout(() => {
-        throw ex;
-      }, 0);
-    });
-}
-
-checkPort();
+startDocusaurus().catch(ex => {
+  console.error(chalk.red(`Failed to start Docusaurus server: ${ex}`));
+  process.exit(1);
+});
diff --git a/package.json b/package.json
index e0ce0c4baa..071f76246b 100644
--- a/package.json
+++ b/package.json
@@ -87,6 +87,7 @@
     "markdown-toc": "^1.2.0",
     "mkdirp": "^0.5.1",
     "opencollective": "^1.0.3",
+    "portfinder": "^1.0.17",
     "postcss": "^7.0.1",
     "prismjs": "^1.15.0",
     "react": "^16.5.0",
diff --git a/yarn.lock b/yarn.lock
index c45f5555c4..6e65129836 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -379,6 +379,10 @@ async-limiter@~1.0.0:
   version "1.0.0"
   resolved "https://registry.yarnpkg.com/async-limiter/-/async-limiter-1.0.0.tgz#78faed8c3d074ab81f22b4e985d79e8738f720f8"
 
+async@^1.5.2:
+  version "1.5.2"
+  resolved "https://registry.yarnpkg.com/async/-/async-1.5.2.tgz#ec6a61ae56480c0c3cb241c95618e20892f9672a"
+
 async@^2.1.4, async@^2.5.0:
   version "2.6.1"
   resolved "https://registry.yarnpkg.com/async/-/async-2.6.1.tgz#b245a23ca71930044ec53fa46aa00a3e87c6a610"
@@ -408,7 +412,7 @@ autoprefixer@^6.3.1:
     postcss "^5.2.16"
     postcss-value-parser "^3.2.3"
 
-autoprefixer@^9.1.3:
+autoprefixer@^9.1.5:
   version "9.1.5"
   resolved "https://registry.yarnpkg.com/autoprefixer/-/autoprefixer-9.1.5.tgz#8675fd8d1c0d43069f3b19a2c316f3524e4f6671"
   dependencies:
@@ -1573,7 +1577,7 @@ combined-stream@1.0.6, combined-stream@~1.0.6:
   dependencies:
     delayed-stream "~1.0.0"
 
-commander@^2.11.0, commander@^2.14.1, commander@^2.15.1, commander@^2.16.0, commander@^2.9.0:
+commander@^2.11.0, commander@^2.14.1, commander@^2.15.1, commander@^2.18.0, commander@^2.9.0:
   version "2.18.0"
   resolved "https://registry.yarnpkg.com/commander/-/commander-2.18.0.tgz#2bf063ddee7c7891176981a2cc798e5754bc6970"
 
@@ -2990,7 +2994,7 @@ glob@^5.0.3:
     once "^1.3.0"
     path-is-absolute "^1.0.0"
 
-glob@^7.0.0, glob@^7.0.3, glob@^7.0.5, glob@^7.1.1, glob@^7.1.2, glob@~7.1.1:
+glob@^7.0.0, glob@^7.0.3, glob@^7.0.5, glob@^7.1.1, glob@^7.1.2, glob@^7.1.3, glob@~7.1.1:
   version "7.1.3"
   resolved "https://registry.yarnpkg.com/glob/-/glob-7.1.3.tgz#3960832d3f1574108342dafd3a67b332c0969df1"
   dependencies:
@@ -4192,7 +4196,7 @@ jest-worker@^23.2.0:
   dependencies:
     merge-stream "^1.0.1"
 
-jest@^23.4.2:
+jest@^23.6.0:
   version "23.6.0"
   resolved "https://registry.yarnpkg.com/jest/-/jest-23.6.0.tgz#ad5835e923ebf6e19e7a1d7529a432edfee7813d"
   dependencies:
@@ -4931,7 +4935,7 @@ mixin-deep@^1.1.3, mixin-deep@^1.2.0:
     for-in "^1.0.2"
     is-extendable "^1.0.1"
 
-mkdirp@^0.5.0, mkdirp@^0.5.1, mkdirp@~0.5.1:
+mkdirp@0.5.x, mkdirp@^0.5.0, mkdirp@^0.5.1, mkdirp@~0.5.1:
   version "0.5.1"
   resolved "http://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz#30057438eac6cf7f8c4767f38648d6697d75c903"
   dependencies:
@@ -5496,6 +5500,14 @@ pn@^1.1.0:
   version "1.1.0"
   resolved "https://registry.yarnpkg.com/pn/-/pn-1.1.0.tgz#e2f4cef0e219f463c179ab37463e4e1ecdccbafb"
 
+portfinder@^1.0.17:
+  version "1.0.17"
+  resolved "https://registry.yarnpkg.com/portfinder/-/portfinder-1.0.17.tgz#a8a1691143e46c4735edefcf4fbcccedad26456a"
+  dependencies:
+    async "^1.5.2"
+    debug "^2.2.0"
+    mkdirp "0.5.x"
+
 posix-character-classes@^0.1.0:
   version "0.1.1"
   resolved "https://registry.yarnpkg.com/posix-character-classes/-/posix-character-classes-0.1.1.tgz#01eac0fe3b5af71a2a6c02feabb8c1fef7e00eab"
@@ -5860,7 +5872,7 @@ rc@^1.1.2, rc@^1.2.7:
     minimist "^1.2.0"
     strip-json-comments "~2.0.1"
 
-react-dev-utils@^5.0.1:
+react-dev-utils@^5.0.2:
   version "5.0.2"
   resolved "https://registry.yarnpkg.com/react-dev-utils/-/react-dev-utils-5.0.2.tgz#7bb68d2c4f6ffe7ed1184c5b0124fcad692774d2"
   dependencies:
@@ -5883,7 +5895,7 @@ react-dev-utils@^5.0.1:
     strip-ansi "3.0.1"
     text-table "0.2.0"
 
-react-dom@^16.4.1:
+react-dom@^16.5.0:
   version "16.5.0"
   resolved "https://registry.yarnpkg.com/react-dom/-/react-dom-16.5.0.tgz#57704e5718669374b182a17ea79a6d24922cb27d"
   dependencies:
@@ -5896,7 +5908,7 @@ react-error-overlay@^4.0.1:
   version "4.0.1"
   resolved "https://registry.yarnpkg.com/react-error-overlay/-/react-error-overlay-4.0.1.tgz#417addb0814a90f3a7082eacba7cee588d00da89"
 
-react@^16.4.1:
+react@^16.5.0:
   version "16.5.0"
   resolved "https://registry.yarnpkg.com/react/-/react-16.5.0.tgz#f2c1e754bf9751a549d9c6d9aca41905beb56575"
   dependencies:
@@ -7037,7 +7049,7 @@ trim-right@^1.0.1:
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/trim-right/-/trim-right-1.0.1.tgz#cb2e1203067e0c8de1f614094b9fe45704ea6003"
 
-truncate-html@^1.0.0:
+truncate-html@^1.0.1:
   version "1.0.1"
   resolved "https://registry.yarnpkg.com/truncate-html/-/truncate-html-1.0.1.tgz#6f1d03cbb2308bfda266f9ce8f25e62c66919d4f"
   dependencies: