Ensure websocket always waits before reconnecting

Currently, if the websocket connection fails, then we will always
immediately attempt a reconnect. This can easily contribute to a
thundering herd problem if all WebSocket clients (or even a large
portion of them) are disconnected simultaneously.

This commit ensures that an aborted connection always results in at
least a 1s-2s (randomized) delay.

h/static/scripts/test/websocket-test.js

+'use strict';
+
 var Socket = require('../websocket');
 
 describe('websocket wrapper', function () {
@@ -8,7 +10,7 @@ describe('websocket wrapper', function () {
     this.close = sinon.stub();
     this.send = sinon.stub();
     fakeSocket = this;
-  };
+  }
   FakeWebSocket.OPEN = 1;
 
   var WebSocket = window.WebSocket;
@@ -29,11 +31,23 @@ describe('websocket wrapper', function () {
   });
 
   it('should reconnect after an abnormal disconnection', function () {
-    var socket = new Socket('ws://test:1234');
+    new Socket('ws://test:1234');
     assert.ok(fakeSocket);
     var initialSocket = fakeSocket;
+    fakeSocket.onopen({});
+    fakeSocket.onclose({code: 1006});
+    clock.tick(2000);
+    assert.ok(fakeSocket);
+    assert.notEqual(fakeSocket, initialSocket);
+  });
+
+  it('should reconnect if initial connection fails', function () {
+    new Socket('ws://test:1234');
+    assert.ok(fakeSocket);
+    var initialSocket = fakeSocket;
+    fakeSocket.onopen({});
     fakeSocket.onclose({code: 1006});
-    clock.tick(1000);
+    clock.tick(4000);
     assert.ok(fakeSocket);
     assert.notEqual(fakeSocket, initialSocket);
   });
@@ -57,7 +71,7 @@ describe('websocket wrapper', function () {
     socket.close();
     assert.called(fakeSocket.close);
     var initialSocket = fakeSocket;
-    clock.tick(1000);
+    clock.tick(2000);
     assert.equal(fakeSocket, initialSocket);
   });
 

h/static/scripts/websocket.js

@@ -7,6 +7,9 @@ var inherits = require('inherits');
 // see https://developer.mozilla.org/en-US/docs/Web/API/CloseEvent
 var CLOSE_NORMAL = 1000;
 
+// Minimum delay, in ms, before reconnecting after an abnormal connection close.
+var RECONNECT_MIN_DELAY = 1000;
+
 /**
  * Socket is a minimal wrapper around WebSocket which provides:
  *
@@ -25,6 +28,9 @@ function Socket(url) {
   // the current WebSocket instance
   var socket;
 
+  // a pending operation to connect a WebSocket
+  var operation;
+
   function sendMessages() {
     while (messageQueue.length > 0) {
       var messageString = JSON.stringify(messageQueue.shift());
@@ -32,51 +38,70 @@ function Socket(url) {
     }
   }
 
-  function reconnect() {
-    var didConnect = false;
-    var connectOperation = retry.operation({
-      // Wait 2s before attempting to reconnect
-      minTimeout: 2000,
-      // Don't retry forever
+  // Connect the websocket immediately. If a connection attempt is already in
+  // progress, do nothing.
+  function connect() {
+    if (operation) {
+      return;
+    }
+
+    operation = retry.operation({
+      minTimeout: RECONNECT_MIN_DELAY * 2,
+      // Don't retry forever -- fail permanently after 10 retries
       retries: 10,
       // Randomize retry times to minimise the thundering herd effect
       randomize: true
     });
-    connectOperation.attempt(function (currentAttempt) {
+
+    operation.attempt(function () {
       socket = new WebSocket(url);
       socket.onopen = function (event) {
-        // signal successful connection
-        connectOperation.retry();
-        didConnect = true;
-        sendMessages();
-
+        onOpen();
         self.emit('open', event);
       };
-
       socket.onclose = function (event) {
-        if (event.code !== CLOSE_NORMAL) {
-          if (didConnect) {
-            console.warn('The WebSocket connection closed abnormally ' +
-              '(code: %d, reason: %s). Reconnecting automatically.',
-              event.code, event.reason);
-            reconnect();
-          } else {
-            console.warn('Retrying connection (attempt %d)', currentAttempt);
-            connectOperation.retry(new Error(event.reason));
-          }
-        }
+        if (event.code === CLOSE_NORMAL) {
           self.emit('close', event);
+          return;
+        }
+        var err = new Error('WebSocket closed abnormally, code: ' + event.code);
+        console.warn(err);
+        onAbnormalClose(err);
       };
-
       socket.onerror = function (event) {
         self.emit('error', event);
       };
-
       socket.onmessage = function (event) {
         self.emit('message', event);
       };
     });
-  };
+  }
+
+  // onOpen is called when a websocket connection is successfully established.
+  function onOpen() {
+    operation = null;
+    sendMessages();
+  }
+
+  // onAbnormalClose is called when a websocket connection closes abnormally.
+  // This may be the result of a failure to connect, or an abnormal close after
+  // a previous successful connection.
+  function onAbnormalClose(error) {
+    // If we're already in a reconnection loop, trigger a retry...
+    if (operation) {
+      if (!operation.retry(error)) {
+        console.error('reached max retries attempting to reconnect websocket');
+      }
+      return;
+    }
+    // ...otherwise reconnect the websocket after a short delay.
+    var delay = RECONNECT_MIN_DELAY;
+    delay += Math.floor(Math.random() * delay);
+    operation = setTimeout(function () {
+      operation = null;
+      connect();
+    }, delay);
+  }
 
   /** Close the underlying WebSocket connection */
   this.close = function () {
@@ -99,8 +124,7 @@ function Socket(url) {
     return socket.readyState === WebSocket.OPEN;
   };
 
-  // establish the initial connection
-  reconnect();
+  connect();
 }
 
 inherits(Socket, EventEmitter);