Skip to content

Commit

Permalink
Merge pull request #1925 from ably/fix-fetch-disable-connectivity-check
Browse files Browse the repository at this point in the history
Fix web FetchRequest does not respect `disableConnectivityCheck` client option
  • Loading branch information
VeskeR authored Jan 23, 2025
2 parents 18ed0f6 + 5cdeddc commit f8047f6
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 13 deletions.
31 changes: 19 additions & 12 deletions src/platform/web/lib/http/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -117,18 +117,25 @@ const Http = class {
this.Request = async (method, uri, headers, params, body) => {
return fetchRequestImplementation(method, client ?? null, uri, headers, params, body);
};
this.checkConnectivity = async function () {
Logger.logAction(
this.logger,
Logger.LOG_MICRO,
'(Fetch)Http.checkConnectivity()',
'Sending; ' + connectivityCheckUrl,
);
const requestResult = await this.doUri(HttpMethods.Get, connectivityCheckUrl, null, null, null);
const result = !requestResult.error && (requestResult.body as string)?.replace(/\n/, '') == 'yes';
Logger.logAction(this.logger, Logger.LOG_MICRO, '(Fetch)Http.checkConnectivity()', 'Result: ' + result);
return result;
};

if (client?.options.disableConnectivityCheck) {
this.checkConnectivity = async function () {
return true;
};
} else {
this.checkConnectivity = async function () {
Logger.logAction(
this.logger,
Logger.LOG_MICRO,
'(Fetch)Http.checkConnectivity()',
'Sending; ' + connectivityCheckUrl,
);
const requestResult = await this.doUri(HttpMethods.Get, connectivityCheckUrl, null, null, null);
const result = !requestResult.error && (requestResult.body as string)?.replace(/\n/, '') == 'yes';
Logger.logAction(this.logger, Logger.LOG_MICRO, '(Fetch)Http.checkConnectivity()', 'Result: ' + result);
return result;
};
}
} else {
this.Request = async () => {
const error = hasImplementation
Expand Down
58 changes: 57 additions & 1 deletion test/browser/connection.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
'use strict';

define(['shared_helper', 'chai'], function (Helper, chai) {
define(['ably', 'shared_helper', 'chai'], function (Ably, Helper, chai) {
var { expect, assert } = chai;
var transportPreferenceName = 'ably-transport-preference';

Expand Down Expand Up @@ -453,6 +453,62 @@ define(['shared_helper', 'chai'], function (Helper, chai) {
}
this.test.helper.closeAndFinish(done, realtime);
});

// this is identical to disable_connectivity_check test in connectivity, but here we specifically test browser supported request implementations
for (const scenario of [
{
description: 'XHR request disable connectivity check',
ctx: { initialFetchSupported: Ably.Rest.Platform.Config.fetchSupported },
before: (ctx) => {
Ably.Rest.Platform.Config.fetchSupported = false;
},
after: (ctx) => {
Ably.Rest.Platform.Config.fetchSupported = ctx.initialFetchSupported;
},
},
{
description: 'Fetch request disable connectivity check',
ctx: { initialXhrSupported: Ably.Rest.Platform.Config.xhrSupported },
before: (ctx) => {
Ably.Rest.Platform.Config.xhrSupported = false;
},
after: (ctx) => {
Ably.Rest.Platform.Config.xhrSupported = ctx.initialXhrSupported;
},
},
]) {
/** @nospec */
it(scenario.description, async function () {
const helper = this.test.helper;
scenario.before(scenario.ctx);
let thrownError = null;
let res = null;

try {
helper.recordPrivateApi('pass.clientOption.connectivityCheckUrl');
helper.recordPrivateApi('pass.clientOption.disableConnectivityCheck');
const options = {
connectivityCheckUrl: helper.unroutableHost,
disableConnectivityCheck: true,
autoConnect: false,
};

helper.recordPrivateApi('call.http.checkConnectivity');
res = await helper.AblyRealtime(options).http.checkConnectivity();
} catch (error) {
thrownError = error;
} finally {
scenario.after(scenario.ctx);
}

expect(
res && !thrownError,
'Connectivity check completed ' +
(thrownError &&
(helper.recordPrivateApi('call.Utils.inspectError'), helper.Utils.inspectError(thrownError))),
).to.be.ok;
});
}
});
}
});

0 comments on commit f8047f6

Please sign in to comment.