diff --git a/src/lib/cloud-provider.js b/src/lib/cloud-provider.js index c2b545f6e4dc10cbea9ae5c24cdd89424fd3828a..2e159054c2d01d5d04600db3d3daebba18b07619 100644 --- a/src/lib/cloud-provider.js +++ b/src/lib/cloud-provider.js @@ -28,6 +28,8 @@ class CloudProvider { * @param {string} cloudHost The cloud data server to connect to. */ openConnection () { + this.connectionAttempts += 1; + try { this.connection = new WebSocket((location.protocol === 'http:' ? 'ws://' : 'wss://') + this.cloudHost); } catch (e) { @@ -58,14 +60,15 @@ class CloudProvider { } onOpen () { - this.connectionAttempts = 0; // Reset because we successfully connected + // 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`); - this.connectionAttempts += 1; const randomizedTimeout = this.randomizeDuration(this.exponentialTimeout()); this.setTimeout(this.openConnection.bind(this), randomizedTimeout); } @@ -79,7 +82,7 @@ class CloudProvider { } setTimeout (fn, time) { - log.info(`Reconnecting in ${Math.floor(time / 1000)}s, attempt ${this.connectionAttempts}`); + log.info(`Reconnecting in ${(time / 1000).toFixed(1)}s, attempt ${this.connectionAttempts}`); this._connectionTimeout = window.setTimeout(fn, time); } @@ -167,7 +170,9 @@ class CloudProvider { this.connection.readyState !== WebSocket.CLOSING && this.connection.readyState !== WebSocket.CLOSED) { log.info('Request close cloud connection without reconnecting'); - this.connection.onclose = () => {}; // Remove close listener to prevent reconnect + // 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(); diff --git a/test/unit/util/cloud-provider.test.js b/test/unit/util/cloud-provider.test.js index b803b76d597a51bd934bfe0aed91bfa23d64cb2b..ee720044d9ce6c398a6081aa1f5de02aea534d63 100644 --- a/test/unit/util/cloud-provider.test.js +++ b/test/unit/util/cloud-provider.test.js @@ -103,45 +103,60 @@ describe('CloudProvider', () => { expect(vmIOData[1].varCreate.name).toEqual('name2'); }); - test('connecting sets connnection attempts back to 0', () => { - expect(cloudProvider.connectionAttempts).toBe(0); - cloudProvider.connectionAttempts = 10; + test('connnection attempts set back to 1 when socket is opened', () => { + cloudProvider.connectionAttempts = 100; cloudProvider.connection._open(); - expect(cloudProvider.connectionAttempts).toBe(0); + expect(cloudProvider.connectionAttempts).toBe(1); }); test('disconnect waits for a period equal to 2^k-1 before trying again', () => { - websocketConstructorCount = 0; // This is global, so set it back to 0 to start - - cloudProvider.connection.close(); - expect(timeout).toEqual(1 * 1000); // 2^1 - 1 - expect(websocketConstructorCount).toBe(1); + 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(3 * 1000); // 2^2 - 1 + expect(timeout).toEqual(1 * 1000); // 2^1 - 1 expect(websocketConstructorCount).toBe(2); expect(cloudProvider.connectionAttempts).toBe(2); cloudProvider.connection.close(); - expect(timeout).toEqual(7 * 1000); // 2^3 - 1 + expect(timeout).toEqual(3 * 1000); // 2^2 - 1 expect(websocketConstructorCount).toBe(3); expect(cloudProvider.connectionAttempts).toBe(3); cloudProvider.connection.close(); - expect(timeout).toEqual(15 * 1000); // 2^4 - 1 + expect(timeout).toEqual(7 * 1000); // 2^3 - 1 expect(websocketConstructorCount).toBe(4); expect(cloudProvider.connectionAttempts).toBe(4); cloudProvider.connection.close(); - expect(timeout).toEqual(31 * 1000); // 2^5 - 1 + 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); // maxed out at 2^5 - 1 + 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', () => {