diff --git a/src/core/connection.ts b/src/core/connection.ts index 6688c279..7b1bebb0 100644 --- a/src/core/connection.ts +++ b/src/core/connection.ts @@ -156,16 +156,21 @@ export class DefaultConnection extends EventEmitter 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._transientTimeout = undefined; - this._applyStatusChange(stateChange); + this._onTransientDisconnectTimeout(stateChange); }, TRANSIENT_TIMEOUT); return; } - // If we're in any state other than disconnected, and we have a transient timeout, clear it if (this._transientTimeout) { - clearTimeout(this._transientTimeout); - this._transientTimeout = undefined; + // 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._logger.debug('ignoring transient state due to transient disconnect timeout', stateChange); + return; + } + + this._cancelTransientDisconnectTimeout(); } this._applyStatusChange(stateChange); @@ -224,4 +229,30 @@ export class DefaultConnection extends EventEmitter impleme } } } + + /** + * Handles a transient disconnect timeout. + * + * @param statusChange The change in status. + */ + private _onTransientDisconnectTimeout(statusChange: ConnectionStatusChange): void { + this._logger.debug('transient disconnect timeout reached'); + this._cancelTransientDisconnectTimeout(); + + // When we apply the status change, we should apply whatever the current state is at the time + this._applyStatusChange({ + ...statusChange, + current: this._mapAblyStatusToChat(this._connection.state), + }); + } + + /** + * Cancels the transient disconnect timeout. + */ + private _cancelTransientDisconnectTimeout(): void { + if (this._transientTimeout) { + clearTimeout(this._transientTimeout); + this._transientTimeout = undefined; + } + } } diff --git a/test/core/connection.test.ts b/test/core/connection.test.ts index cb3cc48d..f7367955 100644 --- a/test/core/connection.test.ts +++ b/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); } @@ -195,71 +198,146 @@ describe('connection', () => { }, ); - it('handles transient disconnections', (context) => - new Promise((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), - ); + it('handles transient disconnections', async (context) => { + // 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); + 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); - }); + // Set a listener that stores the state change + const stateChanges: ConnectionStatus[] = []; + connection.onStatusChange((status) => { + stateChanges.push(status.current); + }); - // Transition to a disconnected state - context.emulateStateChange({ current: 'disconnected', previous: 'connected' }); + // Transition to a disconnected state + context.emulateStateChange({ current: 'disconnected', previous: 'connected' }); - // 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' }); + // Wait for 3 seconds (well below the transient timeout) + await 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([]); + // Assert that we have only seen the connected state + expect(stateChanges).toEqual([]); + }); + }); - done(); - }); - })); + it('handles transient disconnections with intermediate state changes', async (context) => { + // 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), + ); - it( - 'emits disconnections after a period of time', - (context) => - new Promise((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 - context.emulateStateChange({ current: 'disconnected', previous: 'connected' }); - - // 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]); - - done(); - }); - }), - 10000, - ); + 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) + await 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([]); + }); + }); + + it('emits disconnections after a period of time', async (context) => { + // 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 }); + + // Wait for longer than the transient timeout + await 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.map((change) => change.current)).toEqual([ + ConnectionStatus.Disconnected, + ConnectionStatus.Connected, + ]); + + // The first change should have the disconnect error + expect(stateChanges[0]?.error).toEqual(disconnectError); + }); + }, 10000); + + it('emits disconnections after a period of time to an intermediate state', async (context) => { + // 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 + await 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); + }); + }, 10000); });