feat: web-ext run now tries in a loop to discover an open remote de…

…bugger port (#472) * Feat: Discover an open remote debugger port * feat: discover an open remote debugger port * feat: discover an open remote debugger port * feat: discover an open remote debugger port * feat: discover an open remote debugger port * feat: discover an open remote debugger port * feat: discover an open remote debugger port
mozilla · Sep 14, 2016 · 0d1b824 · 0d1b824
1 parent a6c5004
commit 0d1b824
Show file tree

Hide file tree

Showing 2 changed files with 62 additions and 35 deletions.
diff --git a/src/firefox/index.js b/src/firefox/index.js
@@ -40,6 +40,7 @@ import type {ExtensionManifest} from '../util/manifest';
 
 export type RemotePortFinderParams = {
   portToTry?: number,
+  retriesLeft?: number,
   connectToFirefox?: FirefoxConnectorFn,
 };
 
@@ -49,28 +50,33 @@ export type RemotePortFinderFn =
 export function defaultRemotePortFinder(
   {
     portToTry = REMOTE_PORT,
+    retriesLeft = 10,
     connectToFirefox = defaultFirefoxConnector,
   }: RemotePortFinderParams = {}
 ): Promise<number> {
   log.debug(`Checking if remote Firefox port ${portToTry} is available`);
-
-  return connectToFirefox(portToTry)
-    .then((client) => {
-      log.debug(`Remote Firefox port ${portToTry} is in use`);
-      client.disconnect();
-      // TODO: instead of throw an error, pick a new random port until
-      // one of them is available.
-      // https://github.com/mozilla/web-ext/issues/283
-      throw new WebExtError(
-        `Cannot listen on port ${portToTry} because it's in use`);
-    })
-    .catch(onlyErrorsWithCode('ECONNREFUSED', () => {
+  function tryToFindAnOpenPort() {
+    return connectToFirefox(portToTry)
+      .then((client) => {
+        log.debug(`Remote Firefox port ${portToTry} is in use ` +
+          `(retries remaining: ${retriesLeft} )`);
+        client.disconnect();
+        portToTry++;
+        if (retriesLeft > 0) {
+          retriesLeft--;
+          return tryToFindAnOpenPort();
+        }
+        else {
+          throw new WebExtError('Too many retries on port search');
+        }
+      })
       // The connection was refused so this port is good to use.
-      return portToTry;
-    }));
+     .catch(onlyErrorsWithCode('ECONNREFUSED', () => {
+       return portToTry;
+     }));
+  }
+  return tryToFindAnOpenPort();
 }
-
-
 // Declare the needed 'fx-runner' module flow types.
 
 export type FirefoxRunnerParams = {

diff --git a/tests/unit/test-firefox/test.firefox.js b/tests/unit/test-firefox/test.firefox.js
@@ -5,7 +5,6 @@ import {assert} from 'chai';
 import deepcopy from 'deepcopy';
 import sinon from 'sinon';
 import FirefoxProfile from 'firefox-profile';
-
 import * as firefox from '../../../src/firefox';
 import {onlyInstancesOf, WebExtError} from '../../../src/errors';
 import {fs} from 'mz';
@@ -480,29 +479,51 @@ describe('firefox', () => {
         });
     });
 
-    it('throws an error when the port is occupied', () => {
-      // TODO: add a retry for occupied ports.
-      // https://github.com/mozilla/web-ext/issues/283
-      const client = fake(RemoteFirefox.prototype);
-      const connectToFirefox = sinon.spy(() => Promise.resolve(client));
-      return findRemotePort({connectToFirefox})
-        .then(makeSureItFails())
-        .catch(onlyInstancesOf(WebExtError, (error) => {
-          assert.match(error.message, /Cannot listen on port/);
-          assert.equal(client.disconnect.called, true);
+    it('returns a port on first try', () => {
+      const connectToFirefox = sinon.spy(() => new Promise(
+        (resolve, reject) => {
+          reject(
+            new TCPConnectError('first call connection fails - port is free')
+          );
         }));
+      return findRemotePort({connectToFirefox, retriesLeft: 2})
+        .then((port) => {
+          assert.equal(connectToFirefox.callCount, 1);
+          assert.isNumber(port);
+        });
     });
 
-    it('re-throws unexpected connection errors', () => {
-      const connectToFirefox = sinon.spy(
-        () => Promise.reject(new Error('not a connection error')));
-      return findRemotePort({connectToFirefox})
-        .then(makeSureItFails())
-        .catch((error) => {
-          assert.match(error.message, /not a connection error/);
+    it('cancels search after too many fails', () => {
+      const client = fake(RemoteFirefox.prototype);
+      const connectToFirefox = sinon.spy(() => new Promise(
+        (resolve) => resolve(client)));
+      return findRemotePort({connectToFirefox, retriesLeft: 2})
+        .catch((err) => {
+          assert.equal(err, 'WebExtError: Too many retries on port search');
+          assert.equal(connectToFirefox.callCount, 3);
         });
     });
 
-  });
+    it('retries port discovery after first failure', () => {
+      const client = fake(RemoteFirefox.prototype);
+      let callCount = 0;
+      const connectToFirefox = sinon.spy(() => {
+        callCount++;
+        return new Promise((resolve, reject) => {
+          if (callCount === 2) {
+            reject(new TCPConnectError('port is free'));
+          }
+          else {
+            resolve(client);
+          }
+        });
+      });
+      return findRemotePort({connectToFirefox, retriesLeft: 2})
+        .then((port) => {
+          assert.isNumber(port);
+          assert.equal(connectToFirefox.callCount, 2);
+        });
+    });
 
+  });
 });