Update cached strategy to prioritize websocket

Usually, we immediately retry cached transport, which works well in most
cases. However, if a client ends up with fallback transports due to a
temporary connection problem, they remain with fallback for the rest of
their session.

This change updates the Cached Strategy to prioritize the WebSocket
transport. Initially, we always try WebSocket first, even if the cached
transport is, for example, long-polling. Each time we override the
cached transport and connect to WebSocket, we increment the attempts
value. After a few attempts, we stop prioritizing WebSocket and switch
to the cached transport immediately.

This change allows clients to have more chances of using WebSocket. But
if we determine that fallback transports perform better in their
environment, we will continue reconnecting them as quickly as possible
to the cached transport.

spec/javascripts/unit/core/strategies/cached_strategy_spec.js → spec/javascripts/unit/core/strategies/websocket_prioritized_cached_strategy_spec.js

@@ -1,9 +1,10 @@
 var Mocks = require("mocks");
 var Runtime = require('runtime').default;
-var CachedStrategy = require('core/strategies/cached_strategy').default;
+var WebSocketPrioritizedCachedStrategy = require('core/strategies/websocket_prioritized_cached_strategy')
+  .default;
 var Util = require('core/util').default;
 
-describe("CachedStrategy", function() {
+describe("WebSocketPrioritizedCachedStrategy", function() {
   beforeEach(function() {
     jasmine.clock().uninstall();
     jasmine.clock().install();
@@ -16,13 +17,13 @@ describe("CachedStrategy", function() {
   describe("after calling isSupported", function() {
     it("should return true when the substrategy is supported", function() {
       var substrategy = Mocks.getStrategy(true);
-      var strategy = new CachedStrategy(substrategy, {}, {});
+      var strategy = new WebSocketPrioritizedCachedStrategy(substrategy, {}, {});
       expect(strategy.isSupported()).toBe(true);
     });
 
     it("should return false when the substrategy is not supported", function() {
       var substrategy = Mocks.getStrategy(false);
-      var strategy = new CachedStrategy(substrategy, {}, {});
+      var strategy = new WebSocketPrioritizedCachedStrategy(substrategy, {}, {});
       expect(strategy.isSupported()).toBe(false);
     });
   });
@@ -34,7 +35,7 @@ describe("CachedStrategy", function() {
 
     it("should try the substrategy immediately", function() {
       var substrategy = Mocks.getStrategy(false);
-      var strategy = new CachedStrategy(substrategy, {}, {});
+      var strategy = new WebSocketPrioritizedCachedStrategy(substrategy, {}, {});
       var callback = jasmine.createSpy("callback");
       strategy.connect(0, callback);
       expect(substrategy.connect).toHaveBeenCalled();
@@ -63,11 +64,12 @@ describe("CachedStrategy", function() {
       beforeEach(function() {
         substrategy = Mocks.getStrategy(true);
         transports = {
-          test: Mocks.getStrategy(true)
+          test: Mocks.getStrategy(true),
+          ws: Mocks.getStrategy(true)
         };
         timeline = Mocks.getTimeline();
 
-        strategy = new CachedStrategy(substrategy, transports, {
+        strategy = new WebSocketPrioritizedCachedStrategy(substrategy, transports, {
           useTLS: useTLS,
           timeline: timeline
         });
@@ -139,7 +141,8 @@ describe("CachedStrategy", function() {
             expect(JSON.parse(localStorage[usedKey])).toEqual({
               timestamp: Util.now(),
               transport: "test",
-              latency: 1000
+              latency: 1000,
+              cacheSkipCount: 0
             });
             expect(localStorage[unusedKey]).toEqual("mock");
           });
@@ -174,7 +177,8 @@ describe("CachedStrategy", function() {
           localStorage[usedKey] = JSON.stringify({
             timestamp: t0,
             transport: "test",
-            latency: 1000
+            latency: 1000,
+            cacheSkipCount: 4
           });
           localStorage[unusedKey] = "mock";
         });
@@ -188,7 +192,7 @@ describe("CachedStrategy", function() {
           strategy.connect(0, callback);
           expect(timeline.info).toHaveBeenCalledWith({
             cached: true,
-            transport: "test",
+            transport: 'test',
             latency: 1000
           });
         });
@@ -235,8 +239,9 @@ describe("CachedStrategy", function() {
           it("should cache the connected transport", function() {
             expect(JSON.parse(localStorage[usedKey])).toEqual({
               timestamp: Util.now(),
-              transport: "test",
-              latency: 2000
+              transport: 'test',
+              latency: 2000,
+              cacheSkipCount: 4
             });
             expect(localStorage[unusedKey]).toEqual("mock");
           });
@@ -327,8 +332,9 @@ describe("CachedStrategy", function() {
             it("should cache the connected transport", function() {
               expect(JSON.parse(localStorage[usedKey])).toEqual({
                 timestamp: Util.now(),
-                transport: "test",
-                latency: 500
+                transport: 'test',
+                latency: 500,
+                cacheSkipCount: 4
               });
               expect(localStorage[unusedKey]).toEqual("mock");
             });
@@ -350,6 +356,42 @@ describe("CachedStrategy", function() {
           });
         });
       });
+
+      describe('websocket prioritized', function() {
+        beforeEach(function() {
+          localStorage[unusedKey] = 'mock';
+          localStorage[usedKey] = JSON.stringify({
+            timestamp: Util.now(),
+            transport: 'test',
+            latency: 1000,
+            cacheSkipCount: 2
+          });
+          localStorage[usedKey] = "{}";
+        });
+
+        it('should try websocket strategy again', function() {
+          strategy.connect(0, callback);
+          expect(substrategy.connect).toHaveBeenCalled();
+        });
+
+        it('should try cached strategy when already attempted default strategy', function() {
+          localStorage[usedKey] = JSON.stringify({
+            timestamp: Util.now(),
+            transport: 'test',
+            latency: 1000,
+            cacheSkipCount: 4
+          });
+
+          strategy.connect(0, callback);
+          expect(transports.test.connect).toHaveBeenCalled();
+          expect(substrategy.connect).not.toHaveBeenCalled();
+
+          expect(JSON.parse(localStorage[usedKey])).toEqual(jasmine.objectContaining({
+            transport: 'test',
+            cacheSkipCount: 4,
+          }));
+        });
+      });
     }
 
     buildCachedTransportTests(false);

src/core/strategies/cached_strategy.ts → src/core/strategies/websocket_prioritized_cached_strategy.ts

@@ -11,13 +11,14 @@ export interface TransportStrategyDictionary {
   [key: string]: TransportStrategy;
 }
 
-/** Caches last successful transport and uses it for following attempts.
+/** Caches the last successful transport and, after the first few attempts,
+ *  uses the cached transport for subsequent attempts.
  *
  * @param {Strategy} strategy
  * @param {Object} transports
  * @param {Object} options
  */
-export default class CachedStrategy implements Strategy {
+export default class WebSocketPrioritizedCachedStrategy implements Strategy {
   strategy: Strategy;
   transports: TransportStrategyDictionary;
   ttl: number;
@@ -43,22 +44,27 @@ export default class CachedStrategy implements Strategy {
   connect(minPriority: number, callback: Function) {
     var usingTLS = this.usingTLS;
     var info = fetchTransportCache(usingTLS);
+    var cacheSkipCount = info && info.cacheSkipCount ? info.cacheSkipCount : 0;
 
     var strategies = [this.strategy];
     if (info && info.timestamp + this.ttl >= Util.now()) {
       var transport = this.transports[info.transport];
       if (transport) {
-        this.timeline.info({
-          cached: true,
-          transport: info.transport,
-          latency: info.latency
-        });
-        strategies.push(
-          new SequentialStrategy([transport], {
-            timeout: info.latency * 2 + 1000,
-            failFast: true
-          })
-        );
+        if (['ws', 'wss'].includes(info.transport) || cacheSkipCount > 3) {
+          this.timeline.info({
+            cached: true,
+            transport: info.transport,
+            latency: info.latency
+          });
+          strategies.push(
+            new SequentialStrategy([transport], {
+              timeout: info.latency * 2 + 1000,
+              failFast: true
+            })
+          );
+        } else {
+          cacheSkipCount++;
+        }
       }
     }
 
@@ -78,7 +84,8 @@ export default class CachedStrategy implements Strategy {
           storeTransportCache(
             usingTLS,
             handshake.transport.name,
-            Util.now() - startTimestamp
+            Util.now() - startTimestamp,
+            cacheSkipCount
           );
           callback(null, handshake);
         }
@@ -120,15 +127,17 @@ function fetchTransportCache(usingTLS: boolean): any {
 function storeTransportCache(
   usingTLS: boolean,
   transport: TransportStrategy,
-  latency: number
+  latency: number,
+  cacheSkipCount: number
 ) {
   var storage = Runtime.getLocalStorage();
   if (storage) {
     try {
       storage[getTransportCacheKey(usingTLS)] = Collections.safeJSONStringify({
         timestamp: Util.now(),
         transport: transport,
-        latency: latency
+        latency: latency,
+        cacheSkipCount: cacheSkipCount
       });
     } catch (e) {
       // catch over quota exceptions raised by localStorage

src/runtimes/isomorphic/default_strategy.ts

@@ -3,9 +3,9 @@ import TransportManager from 'core/transports/transport_manager';
 import Strategy from 'core/strategies/strategy';
 import SequentialStrategy from 'core/strategies/sequential_strategy';
 import BestConnectedEverStrategy from 'core/strategies/best_connected_ever_strategy';
-import CachedStrategy, {
+import WebSocketPrioritizedCachedStrategy, {
   TransportStrategyDictionary
-} from 'core/strategies/cached_strategy';
+} from 'core/strategies/websocket_prioritized_cached_strategy';
 import DelayedStrategy from 'core/strategies/delayed_strategy';
 import IfStrategy from 'core/strategies/if_strategy';
 import FirstConnectedStrategy from 'core/strategies/first_connected_strategy';
@@ -139,7 +139,7 @@ var getDefaultStrategy = function(
     ]);
   }
 
-  return new CachedStrategy(
+  return new WebSocketPrioritizedCachedStrategy(
     new FirstConnectedStrategy(
       new IfStrategy(testSupportsStrategy(ws_transport), wsStrategy, http_loop)
     ),

src/runtimes/web/default_strategy.ts

@@ -4,9 +4,9 @@ import Strategy from 'core/strategies/strategy';
 import StrategyOptions from 'core/strategies/strategy_options';
 import SequentialStrategy from 'core/strategies/sequential_strategy';
 import BestConnectedEverStrategy from 'core/strategies/best_connected_ever_strategy';
-import CachedStrategy, {
+import WebSocketPrioritizedCachedStrategy, {
   TransportStrategyDictionary
-} from 'core/strategies/cached_strategy';
+} from 'core/strategies/websocket_prioritized_cached_strategy';
 import DelayedStrategy from 'core/strategies/delayed_strategy';
 import IfStrategy from 'core/strategies/if_strategy';
 import FirstConnectedStrategy from 'core/strategies/first_connected_strategy';
@@ -181,7 +181,7 @@ var getDefaultStrategy = function(
     ]);
   }
 
-  return new CachedStrategy(
+  return new WebSocketPrioritizedCachedStrategy(
     new FirstConnectedStrategy(
       new IfStrategy(
         testSupportsStrategy(ws_transport),

types/src/core/strategies/cached_strategy.d.ts → types/src/core/strategies/websocket_prioritized_cached_strategy.d.ts

@@ -5,7 +5,7 @@ import Timeline from '../timeline/timeline';
 export interface TransportStrategyDictionary {
     [key: string]: TransportStrategy;
 }
-export default class CachedStrategy implements Strategy {
+export default class WebSocketPrioritizedCachedStrategy implements Strategy {
     strategy: Strategy;
     transports: TransportStrategyDictionary;
     ttl: number;