diff --git a/src/lib/cloud-provider.js b/src/lib/cloud-provider.js index 8116813eb..2e159054c 100644 --- a/src/lib/cloud-provider.js +++ b/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; } } diff --git a/test/unit/util/cloud-provider.test.js b/test/unit/util/cloud-provider.test.js index c81588dde..ee720044d 100644 --- a/test/unit/util/cloud-provider.test.js +++ b/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 + }); });