From 60288b3f31008faa5627088ebe856ac1b5bc6230 Mon Sep 17 00:00:00 2001
From: Andrew Bulat <andrii.bulat@gmail.com>
Date: Fri, 6 Sep 2024 17:19:53 +0100
Subject: [PATCH] Fix websocket reconnection can get stuck in
 disconnected/connecting cycle

Fixes a regression introduced in ably-js v2 in the "no upgrade" PR [1].
Under certain network conditions (internet down) a race condition could
occur in the connection manager. Specifically, websocket connectivity
check in the `startWebSocketSlowTimer` function needed to set the
`wsCheckResult` flag to `false` before the timer was cleared by other
code branches. If that happened, it then caused an endless loop of
websocket connection retries, even after the network conditions
stabilized. A websocket transport would successfully open but then be
immediately disposed due to the `wsCheckResult` flag still being set to
`false`, without any mechanism to reset it in this scenario.

Upon closer look, it makes sense that the websocket connection health
flags (`wsCheckResult` and `abandonedWebSocket`) should be reset when
attempting a new websocket connection. The previous solution did not
explicitly reset these flags in the `connectWs` function, leading to the
described race condition and endless loop. Thus, `wsCheckResult` and
`abandonedWebSocket` are now reset to their neutral values upon entering
the `connectWs` function.

Resolves #1844

[1] https://github.com/ably/ably-js/pull/1645
---
 src/common/lib/transport/connectionmanager.ts | 44 +++++++++----------
 1 file changed, 21 insertions(+), 23 deletions(-)

diff --git a/src/common/lib/transport/connectionmanager.ts b/src/common/lib/transport/connectionmanager.ts
index 1e5316fdb..ac7d7dd3b 100644
--- a/src/common/lib/transport/connectionmanager.ts
+++ b/src/common/lib/transport/connectionmanager.ts
@@ -1101,27 +1101,25 @@ class ConnectionManager extends EventEmitter {
         'ConnectionManager WebSocket slow timer',
         'checking connectivity',
       );
-      if (this.wsCheckResult === null) {
-        this.checkWsConnectivity()
-          .then(() => {
-            Logger.logAction(
-              this.logger,
-              Logger.LOG_MINOR,
-              'ConnectionManager WebSocket slow timer',
-              'ws connectivity check succeeded',
-            );
-            this.wsCheckResult = true;
-          })
-          .catch(() => {
-            Logger.logAction(
-              this.logger,
-              Logger.LOG_MAJOR,
-              'ConnectionManager WebSocket slow timer',
-              'ws connectivity check failed',
-            );
-            this.wsCheckResult = false;
-          });
-      }
+      this.checkWsConnectivity()
+        .then(() => {
+          Logger.logAction(
+            this.logger,
+            Logger.LOG_MINOR,
+            'ConnectionManager WebSocket slow timer',
+            'ws connectivity check succeeded',
+          );
+          this.wsCheckResult = true;
+        })
+        .catch(() => {
+          Logger.logAction(
+            this.logger,
+            Logger.LOG_MAJOR,
+            'ConnectionManager WebSocket slow timer',
+            'ws connectivity check failed',
+          );
+          this.wsCheckResult = false;
+        });
       if (this.realtime.http.checkConnectivity) {
         Utils.whenPromiseSettles(this.realtime.http.checkConnectivity(), (err, connectivity) => {
           if (err || !connectivity) {
@@ -1450,8 +1448,6 @@ class ConnectionManager extends EventEmitter {
     if (transportPreference && transportPreference === this.baseTransport && this.webSocketTransportAvailable) {
       this.checkWsConnectivity()
         .then(() => {
-          this.wsCheckResult = true;
-          this.abandonedWebSocket = false;
           this.unpersistTransportPreference();
           if (this.state === this.states.connecting) {
             Logger.logAction(
@@ -1493,6 +1489,8 @@ class ConnectionManager extends EventEmitter {
    */
   connectWs(transportParams: TransportParams, connectCount: number) {
     Logger.logAction(this.logger, Logger.LOG_MICRO, 'ConnectionManager.connectWs()');
+    this.wsCheckResult = null;
+    this.abandonedWebSocket = false;
     this.startWebSocketSlowTimer();
     this.startWebSocketGiveUpTimer(transportParams);