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,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>
     );

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;

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

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

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,
+};

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);
 }
 

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,
+};

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