From 59551f5e7a7ea3c32b09fdbb505b622753c57ba3 Mon Sep 17 00:00:00 2001 From: Andrew Bulat Date: Wed, 11 Sep 2024 17:21:42 +0100 Subject: [PATCH] Add `wsConnectivityCheckUrl` client option Resolves #1850 --- ably.d.ts | 9 +++++++++ src/common/lib/transport/connectionmanager.ts | 3 ++- src/common/lib/util/defaults.ts | 6 ++++++ src/common/types/IDefaults.d.ts | 2 +- src/platform/nodejs/lib/util/defaults.ts | 2 +- src/platform/web/lib/util/defaults.ts | 2 +- test/common/modules/private_api_recorder.js | 4 +++- test/common/modules/shared_helper.js | 2 ++ test/realtime/transports.test.js | 20 +++++++++---------- 9 files changed, 35 insertions(+), 15 deletions(-) diff --git a/ably.d.ts b/ably.d.ts index 72f58ab63b..b8e85c6a4b 100644 --- a/ably.d.ts +++ b/ably.d.ts @@ -506,6 +506,15 @@ export interface ClientOptions extends AuthOptions { */ connectivityCheckUrl?: string; + /** + * Override the URL used by the realtime client to check if WebSocket connections are available. + * + * If the client suspects that WebSocket connections are unavailable on the current network, + * it will attempt to open a WebSocket connection to this URL to check WebSocket connectivity. + * If this fails, the client will attempt to connect to Ably systems using fallback transports, if available. + */ + wsConnectivityCheckUrl?: string; + /** * Disable the check used by the realtime client to check if the internet * is available before connecting to a fallback host. diff --git a/src/common/lib/transport/connectionmanager.ts b/src/common/lib/transport/connectionmanager.ts index ac7d7dd3bc..8ef56dcc2e 100644 --- a/src/common/lib/transport/connectionmanager.ts +++ b/src/common/lib/transport/connectionmanager.ts @@ -2023,7 +2023,8 @@ class ConnectionManager extends EventEmitter { } checkWsConnectivity() { - const ws = new Platform.Config.WebSocket(Defaults.wsConnectivityUrl); + const wsConnectivityCheckUrl = this.options.wsConnectivityCheckUrl || Defaults.wsConnectivityCheckUrl; + const ws = new Platform.Config.WebSocket(wsConnectivityCheckUrl); return new Promise((resolve, reject) => { let finished = false; ws.onopen = () => { diff --git a/src/common/lib/util/defaults.ts b/src/common/lib/util/defaults.ts index ceb90d6ffb..41292df675 100644 --- a/src/common/lib/util/defaults.ts +++ b/src/common/lib/util/defaults.ts @@ -311,6 +311,11 @@ export function normaliseOptions( connectivityCheckUrl = uri; } + let wsConnectivityCheckUrl = options.wsConnectivityCheckUrl; + if (wsConnectivityCheckUrl && wsConnectivityCheckUrl.indexOf('://') === -1) { + wsConnectivityCheckUrl = 'wss://' + wsConnectivityCheckUrl; + } + return { ...options, realtimeHost, @@ -319,6 +324,7 @@ export function normaliseOptions( timeouts, connectivityCheckParams, connectivityCheckUrl, + wsConnectivityCheckUrl, headers, }; } diff --git a/src/common/types/IDefaults.d.ts b/src/common/types/IDefaults.d.ts index 5e2e12d37d..37b6b03659 100644 --- a/src/common/types/IDefaults.d.ts +++ b/src/common/types/IDefaults.d.ts @@ -3,7 +3,7 @@ import { RestAgentOptions } from './ClientOptions'; export default interface IDefaults { connectivityCheckUrl: string; - wsConnectivityUrl: string; + wsConnectivityCheckUrl: string; defaultTransports: TransportName[]; restAgentOptions?: RestAgentOptions; } diff --git a/src/platform/nodejs/lib/util/defaults.ts b/src/platform/nodejs/lib/util/defaults.ts index 200d9e8fd6..7cd6f94c36 100644 --- a/src/platform/nodejs/lib/util/defaults.ts +++ b/src/platform/nodejs/lib/util/defaults.ts @@ -3,7 +3,7 @@ import { TransportNames } from '../../../../common/constants/TransportName'; const Defaults: IDefaults = { connectivityCheckUrl: 'https://internet-up.ably-realtime.com/is-the-internet-up.txt', - wsConnectivityUrl: 'wss://ws-up.ably-realtime.com', + wsConnectivityCheckUrl: 'wss://ws-up.ably-realtime.com', /* Note: order matters here: the base transport is the leftmost one in the * intersection of baseTransportOrder and the transports clientOption that's supported. */ defaultTransports: [TransportNames.WebSocket], diff --git a/src/platform/web/lib/util/defaults.ts b/src/platform/web/lib/util/defaults.ts index c2bfd52ecc..0d3d4aed04 100644 --- a/src/platform/web/lib/util/defaults.ts +++ b/src/platform/web/lib/util/defaults.ts @@ -3,7 +3,7 @@ import { TransportNames } from 'common/constants/TransportName'; const Defaults: IDefaults = { connectivityCheckUrl: 'https://internet-up.ably-realtime.com/is-the-internet-up.txt', - wsConnectivityUrl: 'wss://ws-up.ably-realtime.com', + wsConnectivityCheckUrl: 'wss://ws-up.ably-realtime.com', /* Order matters here: the base transport is the leftmost one in the * intersection of baseTransportOrder and the transports clientOption that's * supported. */ diff --git a/test/common/modules/private_api_recorder.js b/test/common/modules/private_api_recorder.js index 1c1013bebd..57cc6c55d3 100644 --- a/test/common/modules/private_api_recorder.js +++ b/test/common/modules/private_api_recorder.js @@ -69,6 +69,7 @@ define(['test/support/output_directory_paths'], function (outputDirectoryPaths) 'pass.clientOption.pushRecipientChannel', 'pass.clientOption.webSocketConnectTimeout', 'pass.clientOption.webSocketSlowTimeout', + 'pass.clientOption.wsConnectivityCheckUrl', // actually ably-js public API (i.e. it’s in the TypeScript typings) but no other SDK has it. At the same time it's not entirely clear if websocket connectivity check should be considered an ably-js-specific functionality (as for other params above), so for the time being we consider it as private API 'read.Defaults.version', 'read.EventEmitter.events', 'read.Platform.Config.push', @@ -125,7 +126,7 @@ define(['test/support/output_directory_paths'], function (outputDirectoryPaths) 'replace.transport.send', 'serialize.recoveryKey', 'write.Defaults.ENVIRONMENT', - 'write.Defaults.wsConnectivityUrl', + 'write.Defaults.wsConnectivityCheckUrl', 'write.Platform.Config.push', // This implies using a mock implementation of the internal IPlatformPushConfig interface. Our mock (in push_channel_transport.js) then interacts with internal objects and private APIs of public objects to implement this interface; I haven’t added annotations for that private API usage, since there wasn’t an easy way to pass test context information into the mock. I think that for now we can just say that if we wanted to get rid of this private API usage, then we’d need to remove this mock entirely. 'write.auth.authOptions.requestHeaders', 'write.auth.key', @@ -139,6 +140,7 @@ define(['test/support/output_directory_paths'], function (outputDirectoryPaths) 'write.connectionManager.msgSerial', 'write.connectionManager.wsHosts', 'write.realtime.options.realtimeHost', + 'write.realtime.options.wsConnectivityCheckUrl', 'write.realtime.options.timeouts.realtimeRequestTimeout', 'write.rest._currentFallback.validUntil', ]; diff --git a/test/common/modules/shared_helper.js b/test/common/modules/shared_helper.js index e13ec6bcdb..4ce973bb45 100644 --- a/test/common/modules/shared_helper.js +++ b/test/common/modules/shared_helper.js @@ -19,6 +19,7 @@ define([ /* IANA reserved; requests to it will hang forever */ var unroutableHost = '10.255.255.1'; var unroutableAddress = 'http://' + unroutableHost + '/'; + var unroutableWssAddress = 'wss://' + unroutableHost + '/'; class SharedHelper { getTestApp = testAppModule.getTestApp; @@ -31,6 +32,7 @@ define([ unroutableHost = unroutableHost; unroutableAddress = unroutableAddress; + unroutableWssAddress = unroutableWssAddress; flushTestLogs = globals.flushLogs; constructor(context) { diff --git a/test/realtime/transports.test.js b/test/realtime/transports.test.js index 6c508a7a63..3c6b760037 100644 --- a/test/realtime/transports.test.js +++ b/test/realtime/transports.test.js @@ -3,7 +3,7 @@ define(['shared_helper', 'async', 'chai', 'ably'], function (Helper, async, chai, Ably) { const expect = chai.expect; const Defaults = Ably.Rest.Platform.Defaults; - const originialWsCheckUrl = Defaults.wsConnectivityUrl; + const originialWsCheckUrl = Defaults.wsConnectivityCheckUrl; const transportPreferenceName = 'ably-transport-preference'; const localStorageSupported = globalThis.localStorage; const urlScheme = 'https://'; @@ -20,8 +20,9 @@ define(['shared_helper', 'async', 'chai', 'ably'], function (Helper, async, chai }).connection.connectionManager.baseTransport; } - function restoreWsConnectivityUrl() { - Defaults.wsConnectivityUrl = originialWsCheckUrl; + function restoreWsConnectivityCheckUrl() { + Helper.forHook(this).recordPrivateApi('write.Defaults.wsConnectivityCheckUrl'); + Defaults.wsConnectivityCheckUrl = originialWsCheckUrl; } const Config = Ably.Rest.Platform.Config; @@ -50,7 +51,7 @@ define(['shared_helper', 'async', 'chai', 'ably'], function (Helper, async, chai }); }); - afterEach(restoreWsConnectivityUrl); + afterEach(restoreWsConnectivityCheckUrl); afterEach(restoreWebSocketConstructor); if ( @@ -163,14 +164,13 @@ define(['shared_helper', 'async', 'chai', 'ably'], function (Helper, async, chai helper.recordPrivateApi('read.realtime.options.realtimeHost'); const goodHost = helper.AblyRest().options.realtimeHost; - // use unroutable host ws connectivity check to simulate no internet - helper.recordPrivateApi('write.Defaults.wsConnectivityUrl'); - Defaults.wsConnectivityUrl = `wss://${helper.unroutableAddress}`; - helper.recordPrivateApi('pass.clientOption.webSocketSlowTimeout'); + helper.recordPrivateApi('pass.clientOption.wsConnectivityCheckUrl'); const realtime = helper.AblyRealtime( options(helper, { realtimeHost: helper.unroutableAddress, + // use unroutable host ws connectivity check to simulate no internet + wsConnectivityCheckUrl: helper.unroutableWssAddress, // ensure ws slow timeout procs and performs ws connectivity check, which would fail due to unroutable host webSocketSlowTimeout: 1, // give up trying to connect fairly quickly @@ -198,8 +198,8 @@ define(['shared_helper', 'async', 'chai', 'ably'], function (Helper, async, chai // restore original settings helper.recordPrivateApi('replace.connectionManager.tryATransport'); connection.connectionManager.tryATransport = tryATransportOriginal; - helper.recordPrivateApi('write.Defaults.wsConnectivityUrl'); - Defaults.wsConnectivityUrl = originialWsCheckUrl; + helper.recordPrivateApi('write.realtime.options.wsConnectivityCheckUrl'); + realtime.options.wsConnectivityCheckUrl = originialWsCheckUrl; helper.recordPrivateApi('write.realtime.options.realtimeHost'); realtime.options.realtimeHost = goodHost; helper.recordPrivateApi('write.connectionManager.wsHosts');