Merge pull request #3818 from paulkaplan/reconnect-cloud

Add exponential backoff for cloud provider.

src/lib/cloud-provider.js

@@ -16,24 +16,28 @@ class CloudProvider {
         this.vm = vm;
         this.username = username;
         this.projectId = projectId;
+        this.cloudHost = cloudHost;
 
-        // Open a websocket connection to the clouddata server
-        this.openConnection(cloudHost);
+        this.connectionAttempts = 0;
+
+        this.openConnection();
     }
 
     /**
      * Open a new websocket connection to the clouddata server.
      * @param {string} cloudHost The cloud data server to connect to.
      */
-    openConnection (cloudHost) {
-        if (window.WebSocket === null) {
-            log.warn('Websocket support is not available in this browser');
+    openConnection () {
+        this.connectionAttempts += 1;
+
+        try {
+            this.connection = new WebSocket((location.protocol === 'http:' ? 'ws://' : 'wss://') + this.cloudHost);
+        } catch (e) {
+            log.warn('Websocket support is not available in this browser', e);
             this.connection = null;
             return;
         }
 
-        this.connection = new WebSocket((location.protocol === 'http:' ? 'ws://' : 'wss://') + cloudHost);
-
         this.connection.onerror = this.onError.bind(this);
         this.connection.onmessage = this.onMessage.bind(this);
         this.connection.onopen = this.onOpen.bind(this);
@@ -42,8 +46,7 @@ class CloudProvider {
 
     onError (event) {
         log.error(`Websocket connection error: ${JSON.stringify(event)}`);
-        // TODO Add re-connection attempt logic here
-        this.clear();
+        // Error is always followed by close, which handles reconnect logic.
     }
 
     onMessage (event) {
@@ -57,12 +60,30 @@ class CloudProvider {
     }
 
     onOpen () {
+        // Reset connection attempts to 1 to make sure any subsequent reconnects
+        // use connectionAttempts=1 to calculate timeout
+        this.connectionAttempts = 1;
         this.writeToServer('handshake');
         log.info(`Successfully connected to clouddata server.`);
     }
 
     onClose () {
         log.info(`Closed connection to websocket`);
+        const randomizedTimeout = this.randomizeDuration(this.exponentialTimeout());
+        this.setTimeout(this.openConnection.bind(this), randomizedTimeout);
+    }
+
+    exponentialTimeout () {
+        return (Math.pow(2, Math.min(this.connectionAttempts, 5)) - 1) * 1000;
+    }
+
+    randomizeDuration (t) {
+        return Math.random() * t;
+    }
+
+    setTimeout (fn, time) {
+        log.info(`Reconnecting in ${(time / 1000).toFixed(1)}s, attempt ${this.connectionAttempts}`);
+        this._connectionTimeout = window.setTimeout(fn, time);
     }
 
     parseMessage (message) {
@@ -148,7 +169,10 @@ class CloudProvider {
         if (this.connection &&
             this.connection.readyState !== WebSocket.CLOSING &&
             this.connection.readyState !== WebSocket.CLOSED) {
-
+            log.info('Request close cloud connection without reconnecting');
+            // Remove listeners, after this point we do not want to react to connection updates
+            this.connection.onclose = () => {};
+            this.connection.onerror = () => {};
             this.connection.close();
         }
         this.clear();
@@ -163,6 +187,11 @@ class CloudProvider {
         this.vm = null;
         this.username = null;
         this.projectId = null;
+        if (this._connectionTimeout) {
+            clearTimeout(this._connectionTimeout);
+            this._connectionTimeout = null;
+        }
+        this.connectionAttempts = 0;
     }
 
 }

test/unit/util/cloud-provider.test.js

@@ -1,33 +1,51 @@
 import CloudProvider from '../../../src/lib/cloud-provider';
 
-// Disable window.WebSocket
-global.WebSocket = null;
+let websocketConstructorCount = 0;
+
+// Stub the global websocket so we can call open/close/error/send on it
+global.WebSocket = function (url) {
+    this._url = url;
+    this._sentMessages = [];
+
+    // These are not real websocket methods, but used to trigger callbacks
+    this._open = () => this.onopen();
+    this._error = e => this.onerror(e);
+    this._receive = msg => this.onmessage(msg);
+
+    // Stub the real websocket.send to store sent messages
+    this.send = msg => this._sentMessages.push(msg);
+    this.close = () => this.onclose();
+
+    websocketConstructorCount++;
+};
+global.WebSocket.CLOSING = 'CLOSING';
+global.WebSocket.CLOSED = 'CLOSED';
 
 describe('CloudProvider', () => {
     let cloudProvider = null;
-    let sentMessage = null;
     let vmIOData = [];
-
+    let timeout = 0;
     beforeEach(() => {
         vmIOData = [];
         cloudProvider = new CloudProvider();
-        // Stub connection
-        cloudProvider.connection = {
-            send: msg => {
-                sentMessage = msg;
-            }
-        };
         // Stub vm
         cloudProvider.vm = {
             postIOData: (_namespace, data) => {
                 vmIOData.push(data);
             }
         };
+        // Stub setTimeout so this can run instantly.
+        cloudProvider.setTimeout = (fn, after) => {
+            timeout = after;
+            fn();
+        };
+        // Stub randomize to make it consistent for testing.
+        cloudProvider.randomizeDuration = t => t;
     });
 
     test('updateVariable', () => {
         cloudProvider.updateVariable('hello', 1);
-        const obj = JSON.parse(sentMessage);
+        const obj = JSON.parse(cloudProvider.connection._sentMessages[0]);
         expect(obj.method).toEqual('set');
         expect(obj.name).toEqual('hello');
         expect(obj.value).toEqual(1);
@@ -35,7 +53,7 @@ describe('CloudProvider', () => {
 
     test('updateVariable with falsey value', () => {
         cloudProvider.updateVariable('hello', 0);
-        const obj = JSON.parse(sentMessage);
+        const obj = JSON.parse(cloudProvider.connection._sentMessages[0]);
         expect(obj.method).toEqual('set');
         expect(obj.name).toEqual('hello');
         expect(obj.value).toEqual(0);
@@ -43,7 +61,7 @@ describe('CloudProvider', () => {
 
     test('writeToServer with falsey index value', () => {
         cloudProvider.writeToServer('method', 'name', 5, 0);
-        const obj = JSON.parse(sentMessage);
+        const obj = JSON.parse(cloudProvider.connection._sentMessages[0]);
         expect(obj.method).toEqual('method');
         expect(obj.name).toEqual('name');
         expect(obj.value).toEqual(5);
@@ -55,7 +73,7 @@ describe('CloudProvider', () => {
             method: 'ack',
             name: 'name'
         });
-        cloudProvider.onMessage({data: msg});
+        cloudProvider.connection._receive({data: msg});
         expect(vmIOData[0].varCreate.name).toEqual('name');
     });
 
@@ -65,7 +83,7 @@ describe('CloudProvider', () => {
             name: 'name',
             value: 'value'
         });
-        cloudProvider.onMessage({data: msg});
+        cloudProvider.connection._receive({data: msg});
         expect(vmIOData[0].varUpdate.name).toEqual('name');
         expect(vmIOData[0].varUpdate.value).toEqual('value');
     });
@@ -80,8 +98,75 @@ describe('CloudProvider', () => {
             method: 'ack',
             name: 'name2'
         });
-        cloudProvider.onMessage({data: `${msg1}\n${msg2}`});
+        cloudProvider.connection._receive({data: `${msg1}\n${msg2}`});
         expect(vmIOData[0].varUpdate.name).toEqual('name1');
         expect(vmIOData[1].varCreate.name).toEqual('name2');
     });
+
+    test('connnection attempts set back to 1 when socket is opened', () => {
+        cloudProvider.connectionAttempts = 100;
+        cloudProvider.connection._open();
+        expect(cloudProvider.connectionAttempts).toBe(1);
+    });
+
+    test('disconnect waits for a period equal to 2^k-1 before trying again', () => {
+        websocketConstructorCount = 1; // This is global, so set it back to 1 to start
+        // Constructor attempts to open connection, so attempts is initially 1
+        expect(cloudProvider.connectionAttempts).toBe(1);
+
+        // Make sure a close without a previous OPEN still waits 1s before reconnecting
+        cloudProvider.connection.close();
+        expect(timeout).toEqual(1 * 1000); // 2^1 - 1
+        expect(websocketConstructorCount).toBe(2);
+        expect(cloudProvider.connectionAttempts).toBe(2);
+
+        cloudProvider.connection.close();
+        expect(timeout).toEqual(3 * 1000); // 2^2 - 1
+        expect(websocketConstructorCount).toBe(3);
+        expect(cloudProvider.connectionAttempts).toBe(3);
+
+        cloudProvider.connection.close();
+        expect(timeout).toEqual(7 * 1000); // 2^3 - 1
+        expect(websocketConstructorCount).toBe(4);
+        expect(cloudProvider.connectionAttempts).toBe(4);
+
+        cloudProvider.connection.close();
+        expect(timeout).toEqual(15 * 1000); // 2^4 - 1
+        expect(websocketConstructorCount).toBe(5);
+        expect(cloudProvider.connectionAttempts).toBe(5);
+
+        cloudProvider.connection.close();
+        expect(timeout).toEqual(31 * 1000); // 2^5 - 1
+        expect(websocketConstructorCount).toBe(6);
+        expect(cloudProvider.connectionAttempts).toBe(6);
+
+        cloudProvider.connection.close();
+        expect(timeout).toEqual(31 * 1000); // maxed out at 2^5 - 1
+        expect(websocketConstructorCount).toBe(7);
+        expect(cloudProvider.connectionAttempts).toBe(7);
+    });
+
+    test('close after connection is opened waits 1s before reconnecting', () => {
+        // This test is basically to check that opening the connection does not impact
+        // the time until reconnection for the first reconnect.
+        // It is easy to introduce a bug that causes reconnection time to be different
+        // based on whether an initial connection was made.
+        websocketConstructorCount = 1;
+        cloudProvider.connection._open();
+        cloudProvider.connection.close();
+        expect(timeout).toEqual(1 * 1000); // 2^1 - 1
+        expect(websocketConstructorCount).toBe(2);
+        expect(cloudProvider.connectionAttempts).toBe(2);
+    });
+
+    test('exponentialTimeout caps connection attempt number', () => {
+        cloudProvider.connectionAttempts = 1000;
+        expect(cloudProvider.exponentialTimeout()).toEqual(31 * 1000);
+    });
+
+    test('requestCloseConnection does not try to reconnect', () => {
+        websocketConstructorCount = 1; // This is global, so set it back to 1 to start
+        cloudProvider.requestCloseConnection();
+        expect(websocketConstructorCount).toBe(1); // No reconnection attempts
+    });
 });