Make sure to try to reconnect when request to dispatcher fails

src/peer/socket.js

@@ -85,15 +85,12 @@ class Socket extends EventEmitter {
     }
 
     if (this._dispatcherUrl) {
-      let serverInfo;
       try {
-        serverInfo = await this._getSignalingServer();
+        this.signalingServerUrl = await this._getSignalingServerUrlWithRetry();
       } catch (err) {
         this.emit('error', err);
         return;
       }
-      const httpProtocol = serverInfo.secure ? 'https://' : 'http://';
-      this.signalingServerUrl = `${httpProtocol}${serverInfo.host}:${serverInfo.port}`;
     }
 
     this._io = io(this.signalingServerUrl, {
@@ -115,41 +112,48 @@ class Socket extends EventEmitter {
   }
 
   /**
-   * Connect to "new" signaling server. Attempts up to 10 times before giving up and emitting an error on the socket.
-   * @param {number} [numAttempts=0] - Current number of attempts.
+   * Connect to "new" signaling server.
    * @return {Promise<void>} A promise that resolves with new connection has done.
    * @private
    */
-  async _connectToNewServer(numAttempts = 0) {
-    // max number of attempts to get a new server from the dispatcher.
-    const maxNumberOfAttempts = 10;
-    if (
-      numAttempts >= maxNumberOfAttempts ||
-      this._reconnectAttempts >= config.numberServersToTry
-    ) {
+  async _connectToNewServer() {
+    if (this._reconnectAttempts >= config.numberServersToTry) {
       this.emit('error', 'Could not connect to server.');
       return;
     }
 
-    // Keep trying until we connect to a new server because consul can take some time to remove from the active list.
-    let serverInfo;
     try {
-      serverInfo = await this._getSignalingServer();
+      this.signalingServerUrl = await this._getSignalingServerUrlWithRetry();
     } catch (err) {
       this.emit('error', err);
       return;
     }
+    this._io.io.uri = this.signalingServerUrl;
+    this._io.connect();
+    this._reconnectAttempts++;
+  }
 
-    if (this.signalingServerUrl.indexOf(serverInfo.host) === -1) {
-      const httpProtocol = serverInfo.secure ? 'https://' : 'http://';
-      this.signalingServerUrl = `${httpProtocol}${serverInfo.host}:${serverInfo.port}`;
-
-      this._io.io.uri = this.signalingServerUrl;
-      this._io.connect();
-      this._reconnectAttempts++;
-    } else {
-      this._connectToNewServer(++numAttempts);
+  /**
+   * Return signaling server url. This attempts trying up to maxNumberOfAttempts times before giving up then throw error.
+   * @return {String} A string of signaling server url.
+   */
+  async _getSignalingServerUrlWithRetry() {
+    for (let attempts = 0; attempts < config.maxNumberOfAttempts; attempts++) {
+      const serverInfo = await this._getSignalingServer().catch(err => {
+        logger.warn(err);
+      });
+      if (
+        serverInfo &&
+        serverInfo.port &&
+        serverInfo.host &&
+        (!this.signalingServerUrl ||
+          this.signalingServerUrl.indexOf(serverInfo.host) === -1)
+      ) {
+        const httpProtocol = serverInfo.secure ? 'https://' : 'http://';
+        return `${httpProtocol}${serverInfo.host}:${serverInfo.port}`;
+      }
     }
+    throw new Error('Could not get signaling server url.');
   }
 
   /**

src/shared/config.js

@@ -55,6 +55,9 @@ const maxDataSize = 20 * 1024 * 1024;
 // The minimum interval of using Room.send() is 100 ms
 const minBroadcastIntervalMs = 100;
 
+// max number of attempts to get a new server from the dispatcher.
+const maxNumberOfAttempts = 10;
+
 // Number of reconnection attempts to the same server before giving up
 const reconnectionAttempts = 2;
 
@@ -88,6 +91,7 @@ export default {
   maxChunkSize,
   maxDataSize,
   minBroadcastIntervalMs,
+  maxNumberOfAttempts,
   reconnectionAttempts,
   numberServersToTry,
   sendInterval,

tests/peer/socket.js

@@ -589,4 +589,90 @@ describe('Socket', () => {
       });
     });
   });
+
+  describe('_getSignalingServerUrlWithRetry', () => {
+    const signalingHost = 'signaling.io';
+    const signalingPort = 443;
+    const signalingSecure = true;
+    const httpProtocol = 'https://';
+    const signalingServerUrl = `${httpProtocol}${signalingHost}:${signalingPort}`;
+    let emitStub;
+    let getSignalingServerStub;
+
+    beforeEach(() => {
+      emitStub = sinon.stub(socket, 'emit');
+      getSignalingServerStub = sinon.stub(socket, '_getSignalingServer');
+      getSignalingServerStub.returns(
+        Promise.resolve({
+          host: signalingHost,
+          port: signalingPort,
+          secure: signalingSecure,
+        })
+      );
+    });
+
+    afterEach(() => {
+      emitStub.restore();
+      getSignalingServerStub.restore();
+    });
+
+    it('should return signalingServerUrl', async () => {
+      const url = await socket._getSignalingServerUrlWithRetry();
+
+      assert.equal(url, signalingServerUrl);
+    });
+
+    it('should attempt 10 times before giving up and throw error', async () => {
+      socket.signalingServerUrl = signalingServerUrl;
+
+      await socket._getSignalingServerUrlWithRetry().catch(err => {
+        assert.equal(getSignalingServerStub.callCount, 10);
+        assert.equal(err.message, 'Could not get signaling server url.');
+      });
+
+      // assert.throws(await socket._getSignalingServerUrlWithRetry(), 'Could not get signaling server url.');
+    });
+  });
+
+  describe('_connectToNewServer', () => {
+    let connectToNewServerSpy;
+    let emitStub;
+    let getSignalingServerUrlWithRetryStub;
+
+    beforeEach(() => {
+      connectToNewServerSpy = sinon.spy(socket, '_connectToNewServer');
+      emitStub = sinon.stub(socket, 'emit');
+      getSignalingServerUrlWithRetryStub = sinon.stub(
+        socket,
+        '_getSignalingServerUrlWithRetry'
+      );
+    });
+
+    afterEach(() => {
+      connectToNewServerSpy.restore();
+      emitStub.restore();
+      getSignalingServerUrlWithRetryStub.restore();
+    });
+
+    it('should set _io.io.uri', () => {
+      const signalingServerUrl = 'https:signaling.io:443';
+      getSignalingServerUrlWithRetryStub.returns(signalingServerUrl);
+
+      socket.start(undefined, token).then(async () => {
+        await socket._connectToNewServer();
+
+        assert.equal(socket._io.io.uri, signalingServerUrl);
+      });
+    });
+
+    describe('when response from dispatcher is empty', () => {
+      it('should emit an error on the socket', async () => {
+        getSignalingServerUrlWithRetryStub.throws();
+
+        await socket._connectToNewServer();
+
+        assert.deepEqual(emitStub.args[0], ['error', new Error()]);
+      });
+    });
+  });
 });