core/connection: dont cancel transient disconnect timer with connecti…

…ng status

At the moment, if a connecting status is received during a transient disconnect timer, we cancel the timer.

This isn't really ideal - as it defeats the point of a transient disconnect timeout. Equally, we suppress the error
that caused the state in the first place.

This change:

- Will only cancel the transient disconnect timeout on a state that isn't connecting.
- When the transient disconnect timeout timer expires, it will pass the *current* status, but with the error from the original state change.

src/core/connection.ts

@@ -156,13 +156,30 @@ export class DefaultConnection extends EventEmitter<ConnectionEventsMap> impleme
       // If we're in the disconnected state, assume it's transient and set a timeout to propagate the change
       if (chatState === ConnectionStatus.Disconnected && !this._transientTimeout) {
         this._transientTimeout = setTimeout(() => {
+          this._logger.debug('transient disconnect timeout reached');
           this._transientTimeout = undefined;
-          this._applyStatusChange(stateChange);
+
+          // When we apply the status change, we should apply whatever the current state is at the time
+          this._applyStatusChange({
+            ...stateChange,
+            current: this._mapAblyStatusToChat(this._connection.state),
+          });
         }, TRANSIENT_TIMEOUT);
         return;
       }
 
-      // If we're in any state other than disconnected, and we have a transient timeout, clear it
+      // If we're in the connecting state, or disconnected state, and we have a transient timeout, we should ignore it -
+      // if we can reach connected in a reasonable time, we can assume the disconnect was transient and suppress the
+      // change
+      if (
+        (chatState === ConnectionStatus.Connecting || chatState === ConnectionStatus.Disconnected) &&
+        this._transientTimeout
+      ) {
+        this._logger.debug('ignoring transient state due to transient disconnect timeout', stateChange);
+        return;
+      }
+
+      // If we're in any state other than disconnected and connecting, and we have a transient timeout, clear it
       if (this._transientTimeout) {
         clearTimeout(this._transientTimeout);
         this._transientTimeout = undefined;

test/core/connection.test.ts

@@ -2,7 +2,7 @@ import * as Ably from 'ably';
 import { beforeEach, describe, expect, it, vi } from 'vitest';
 
 import { ErrorInfo } from '../../__mocks__/ably/index.ts';
-import { ConnectionStatus, DefaultConnection } from '../../src/core/connection.ts';
+import { ConnectionStatus, ConnectionStatusChange, DefaultConnection } from '../../src/core/connection.ts';
 import { makeTestLogger } from '../helper/logger.ts';
 
 interface TestContext {
@@ -59,6 +59,9 @@ describe('connection', () => {
       context.channelLevelListeners.add(listener);
 
       context.emulateStateChange = (stateChange: Ably.ConnectionStateChange) => {
+        vi.spyOn(connection, 'state', 'get').mockReturnValue(stateChange.current);
+        vi.spyOn(connection, 'errorReason', 'get').mockReturnValue(stateChange.reason as ErrorInfo);
+
         for (const cb of context.channelLevelListeners) {
           cb(stateChange);
         }
@@ -227,9 +230,45 @@ describe('connection', () => {
       });
     }));
 
+  it<TestContext>('handles transient disconnections with intermediate state changes', (context) =>
+    new Promise<void>((done) => {
+      // Start the channel in a connected state
+      vi.spyOn(context.realtime.connection, 'state', 'get').mockReturnValue('connected');
+      vi.spyOn(context.realtime.connection, 'errorReason', 'get').mockReturnValue(
+        new Ably.ErrorInfo('error', 500, 99999),
+      );
+
+      const connection = new DefaultConnection(context.realtime, makeTestLogger());
+      expect(connection.status).toEqual(ConnectionStatus.Connected);
+
+      // Set a listener that stores the state change
+      const stateChanges: ConnectionStatus[] = [];
+      connection.onStatusChange((status) => {
+        stateChanges.push(status.current);
+      });
+
+      // Transition to a disconnected state
+      const disconnectError = new Ably.ErrorInfo('error', 500, 99999);
+      context.emulateStateChange({ current: 'disconnected', previous: 'connected', reason: disconnectError });
+
+      // Now transition to a connecting state
+      context.emulateStateChange({ current: 'connecting', previous: 'disconnected' });
+
+      // Wait for 3 seconds (well below the transient timeout)
+      void new Promise((resolve) => setTimeout(resolve, 3000)).then(() => {
+        // Transition back to a connected state
+        context.emulateStateChange({ current: 'connected', previous: 'disconnected' });
+
+        // Assert that we have only seen the connected state
+        expect(stateChanges).toEqual([]);
+
+        done();
+      });
+    }));
+
   it<TestContext>(
     'emits disconnections after a period of time',
-    (context) =>
+    async (context) =>
       new Promise<void>((done) => {
         // Start the channel in a connected state
         vi.spyOn(context.realtime.connection, 'state', 'get').mockReturnValue('connected');
@@ -241,21 +280,80 @@ describe('connection', () => {
         expect(connection.status).toEqual(ConnectionStatus.Connected);
 
         // Set a listener that stores the state change
-        const stateChanges: ConnectionStatus[] = [];
+        const stateChanges: ConnectionStatusChange[] = [];
         connection.onStatusChange((status) => {
-          stateChanges.push(status.current);
+          stateChanges.push(status);
         });
 
         // Transition to a disconnected state
-        context.emulateStateChange({ current: 'disconnected', previous: 'connected' });
+        const disconnectError = new Ably.ErrorInfo('error', 500, 99999);
+        context.emulateStateChange({ current: 'disconnected', previous: 'connected', reason: disconnectError });
 
         // Wait for longer than the transient timeout
         void new Promise((resolve) => setTimeout(resolve, 6000)).then(() => {
           // Transition back to a connected state
           context.emulateStateChange({ current: 'connected', previous: 'disconnected' });
 
           // Assert that we have only seen the connected state
-          expect(stateChanges).toEqual([ConnectionStatus.Disconnected, ConnectionStatus.Connected]);
+          expect(stateChanges.map((change) => change.current)).toEqual([
+            ConnectionStatus.Disconnected,
+            ConnectionStatus.Connected,
+          ]);
+
+          // The first change should have the disconnect error
+          expect(stateChanges[0]?.error).toEqual(disconnectError);
+
+          done();
+        });
+      }),
+    10000,
+  );
+
+  it<TestContext>(
+    'emits disconnections after a period of time to an intermediate state',
+    (context) =>
+      new Promise<void>((done) => {
+        // Start the channel in a connected state
+        vi.spyOn(context.realtime.connection, 'state', 'get').mockReturnValue('connected');
+        vi.spyOn(context.realtime.connection, 'errorReason', 'get').mockReturnValue(
+          new Ably.ErrorInfo('error', 500, 99999),
+        );
+
+        const connection = new DefaultConnection(context.realtime, makeTestLogger());
+        expect(connection.status).toEqual(ConnectionStatus.Connected);
+
+        // Set a listener that stores the state change
+        const stateChanges: ConnectionStatusChange[] = [];
+        connection.onStatusChange((status) => {
+          stateChanges.push(status);
+        });
+
+        // Transition to a disconnected state
+        const disconnectError = new Ably.ErrorInfo('error', 500, 99999);
+        context.emulateStateChange({ current: 'disconnected', previous: 'connected', reason: disconnectError });
+
+        // Now transition to a connecting state
+        context.emulateStateChange({ current: 'connecting', previous: 'disconnected' });
+
+        // Now transition to a disconnected state
+        context.emulateStateChange({ current: 'disconnected', previous: 'connecting' });
+
+        // Now transition to a connecting state
+        context.emulateStateChange({ current: 'connecting', previous: 'disconnected' });
+
+        // Wait for longer than the transient timeout
+        void new Promise((resolve) => setTimeout(resolve, 8000)).then(() => {
+          // Transition back to a connected state
+          context.emulateStateChange({ current: 'connected', previous: 'connecting' });
+
+          // Assert that we have only seen the connected state
+          expect(stateChanges.map((change) => change.current)).toEqual([
+            ConnectionStatus.Connecting,
+            ConnectionStatus.Connected,
+          ]);
+
+          // The first change should have the disconnect error
+          expect(stateChanges[0]?.error).toEqual(disconnectError);
 
           done();
         });