diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index 6ae8e76670..5363dbe9e6 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -529,7 +529,6 @@ class Auth { const body = Utils.toQueryString(authParams).slice(1); /* slice is to remove the initial '?' */ this.client.http.doUri( HttpMethods.Post, - client, authOptions.authUrl, headers, body, @@ -539,7 +538,6 @@ class Auth { } else { this.client.http.doUri( HttpMethods.Get, - client, authOptions.authUrl, authHeaders || {}, null, @@ -585,7 +583,6 @@ class Auth { ); this.client.http.do( HttpMethods.Post, - client, tokenUri, requestHeaders, JSON.stringify(signedTokenParams), diff --git a/src/common/lib/client/baseclient.ts b/src/common/lib/client/baseclient.ts index 65d10d4194..72a61f7ff2 100644 --- a/src/common/lib/client/baseclient.ts +++ b/src/common/lib/client/baseclient.ts @@ -78,7 +78,7 @@ class BaseClient { this._currentFallback = null; this.serverTimeOffset = null; - this.http = new Platform.Http(normalOptions); + this.http = new Platform.Http(this); this.auth = new Auth(this, normalOptions); this._rest = modules.Rest ? new modules.Rest(this) : null; diff --git a/src/common/lib/client/resource.ts b/src/common/lib/client/resource.ts index 2f737066e8..f3b3e8b322 100644 --- a/src/common/lib/client/resource.ts +++ b/src/common/lib/client/resource.ts @@ -244,7 +244,7 @@ class Resource { ); } - client.http.do(method, client, path, headers, body, params, function (err, res, headers, unpacked, statusCode) { + client.http.do(method, path, headers, body, params, function (err, res, headers, unpacked, statusCode) { if (err && Auth.isTokenErr(err as ErrorInfo)) { /* token has expired, so get a new one */ client.auth.authorize(null, null, function (err: ErrorInfo) { diff --git a/src/common/lib/client/rest.ts b/src/common/lib/client/rest.ts index 525dda6281..063aff2a31 100644 --- a/src/common/lib/client/rest.ts +++ b/src/common/lib/client/rest.ts @@ -72,7 +72,6 @@ export class Rest { }; this.client.http.do( HttpMethods.Get, - this.client, timeUri, headers, null, diff --git a/src/common/types/http.ts b/src/common/types/http.ts index 8b4ee56995..2b2e71f273 100644 --- a/src/common/types/http.ts +++ b/src/common/types/http.ts @@ -2,7 +2,6 @@ import HttpMethods from '../constants/HttpMethods'; import BaseClient from '../lib/client/baseclient'; import ErrorInfo, { IPartialErrorInfo } from '../lib/types/errorinfo'; import { Agents } from 'got'; -import { NormalisedClientOptions } from './ClientOptions'; export type PathParameter = string | ((host: string) => string); export type RequestCallback = ( @@ -15,7 +14,7 @@ export type RequestCallback = ( export type RequestParams = Record | null; export interface IHttpStatic { - new (options?: NormalisedClientOptions): IHttp; + new (client?: BaseClient): IHttp; methods: Array; methodsWithBody: Array; methodsWithoutBody: Array; @@ -29,7 +28,6 @@ export interface IHttp { _getHosts: (client: BaseClient) => string[]; do( method: HttpMethods, - client: BaseClient | null, path: PathParameter, headers: Record | null, body: unknown, @@ -38,7 +36,6 @@ export interface IHttp { ): void; doUri( method: HttpMethods, - client: BaseClient | null, uri: string, headers: Record | null, body: unknown, diff --git a/src/platform/nodejs/lib/util/http.ts b/src/platform/nodejs/lib/util/http.ts index 68d63d4402..67b436d070 100644 --- a/src/platform/nodejs/lib/util/http.ts +++ b/src/platform/nodejs/lib/util/http.ts @@ -14,7 +14,7 @@ import http from 'http'; import https from 'https'; import BaseClient from 'common/lib/client/baseclient'; import BaseRealtime from 'common/lib/client/baserealtime'; -import { NormalisedClientOptions, RestAgentOptions } from 'common/types/ClientOptions'; +import { RestAgentOptions } from 'common/types/ClientOptions'; import { isSuccessCode } from 'common/constants/HttpStatusCodes'; import { shallowEquals, throwMissingModuleError } from 'common/lib/util/utils'; @@ -105,22 +105,26 @@ const Http: IHttpStatic = class { _getHosts = getHosts; supportsAuthHeaders = true; supportsLinkHeaders = true; - private options: NormalisedClientOptions | null; + private client: BaseClient | null; - constructor(options?: NormalisedClientOptions) { - this.options = options ?? null; + constructor(client?: BaseClient) { + this.client = client ?? null; } - /* Unlike for doUri, the 'client' param here is mandatory, as it's used to generate the hosts */ do( method: HttpMethods, - client: BaseClient, path: PathParameter, headers: Record | null, body: unknown, params: RequestParams, callback: RequestCallback ): void { + /* Unlike for doUri, the presence of `this.client` here is mandatory, as it's used to generate the hosts */ + const client = this.client; + if (!client) { + throw new Error('http.do called without client'); + } + const uriFromHost = typeof path === 'function' ? path @@ -132,11 +136,11 @@ const Http: IHttpStatic = class { if (currentFallback) { if (currentFallback.validUntil > Date.now()) { /* Use stored fallback */ - this.doUri(method, client, uriFromHost(currentFallback.host), headers, body, params, (err, ...args) => { + this.doUri(method, uriFromHost(currentFallback.host), headers, body, params, (err, ...args) => { if (err && shouldFallback(err as ErrnoException)) { /* unstore the fallback and start from the top with the default sequence */ client._currentFallback = null; - this.do(method, client, path, headers, body, params, callback); + this.do(method, path, headers, body, params, callback); return; } callback(err, ...args); @@ -152,13 +156,13 @@ const Http: IHttpStatic = class { /* see if we have one or more than one host */ if (hosts.length === 1) { - this.doUri(method, client, uriFromHost(hosts[0]), headers, body, params, callback); + this.doUri(method, uriFromHost(hosts[0]), headers, body, params, callback); return; } const tryAHost = (candidateHosts: Array, persistOnSuccess?: boolean) => { const host = candidateHosts.shift(); - this.doUri(method, client, uriFromHost(host as string), headers, body, params, function (err, ...args) { + this.doUri(method, uriFromHost(host as string), headers, body, params, function (err, ...args) { if (err && shouldFallback(err as ErrnoException) && candidateHosts.length) { tryAHost(candidateHosts, true); return; @@ -178,7 +182,6 @@ const Http: IHttpStatic = class { doUri( method: HttpMethods, - client: BaseClient, uri: string, headers: Record | null, body: unknown, @@ -188,7 +191,8 @@ const Http: IHttpStatic = class { /* Will generally be making requests to one or two servers exclusively * (Ably and perhaps an auth server), so for efficiency, use the * foreverAgent to keep the TCP stream alive between requests where possible */ - const agentOptions = (client && client.options.restAgentOptions) || (Defaults.restAgentOptions as RestAgentOptions); + const agentOptions = + (this.client && this.client.options.restAgentOptions) || (Defaults.restAgentOptions as RestAgentOptions); const doOptions: Options = { headers: headers || undefined, responseType: 'buffer' }; if (!this.agent) { @@ -215,7 +219,9 @@ const Http: IHttpStatic = class { doOptions.agent = this.agent; doOptions.url = uri; - doOptions.timeout = { request: ((client && client.options.timeouts) || Defaults.TIMEOUTS).httpRequestTimeout }; + doOptions.timeout = { + request: ((this.client && this.client.options.timeouts) || Defaults.TIMEOUTS).httpRequestTimeout, + }; // We have our own logic that retries appropriate statuscodes to fallback endpoints, // with timeouts constructed appropriately. Don't want `got` doing its own retries to // the same endpoint, inappropriately retrying 429s, etc @@ -223,29 +229,28 @@ const Http: IHttpStatic = class { (got[method](doOptions) as CancelableRequest) .then((res: Response) => { - handler(uri, params, client, callback)(null, res, res.body); + handler(uri, params, this.client, callback)(null, res, res.body); }) .catch((err: ErrnoException) => { if (err instanceof got.HTTPError) { - handler(uri, params, client, callback)(null, err.response, err.response.body); + handler(uri, params, this.client, callback)(null, err.response, err.response.body); return; } - handler(uri, params, client, callback)(err); + handler(uri, params, this.client, callback)(err); }); } checkConnectivity = (callback: (errorInfo: ErrorInfo | null, connected?: boolean) => void): void => { - if (this.options?.disableConnectivityCheck) { + if (this.client?.options.disableConnectivityCheck) { callback(null, true); return; } - const connectivityCheckUrl = this.options?.connectivityCheckUrl || Defaults.connectivityCheckUrl; - const connectivityCheckParams = this.options?.connectivityCheckParams ?? null; - const connectivityUrlIsDefault = !this.options?.connectivityCheckUrl; + const connectivityCheckUrl = this.client?.options.connectivityCheckUrl || Defaults.connectivityCheckUrl; + const connectivityCheckParams = this.client?.options.connectivityCheckParams ?? null; + const connectivityUrlIsDefault = !this.client?.options.connectivityCheckUrl; this.doUri( HttpMethods.Get, - null as any, connectivityCheckUrl, null, null, diff --git a/src/platform/web/lib/http/http.ts b/src/platform/web/lib/http/http.ts index 1714cd3b25..0b594b1dc9 100644 --- a/src/platform/web/lib/http/http.ts +++ b/src/platform/web/lib/http/http.ts @@ -11,7 +11,6 @@ import XHRStates from 'common/constants/XHRStates'; import Logger from 'common/lib/util/logger'; import { StandardCallback } from 'common/types/utils'; import fetchRequest from './request/fetchrequest'; -import { NormalisedClientOptions } from 'common/types/ClientOptions'; import { isSuccessCode } from 'common/constants/HttpStatusCodes'; function shouldFallback(errorInfo: ErrorInfo) { @@ -45,16 +44,17 @@ const Http: IHttpStatic = class { static methodsWithoutBody = [HttpMethods.Get, HttpMethods.Delete]; static methodsWithBody = [HttpMethods.Post, HttpMethods.Put, HttpMethods.Patch]; checksInProgress: Array> | null = null; + private client: BaseClient | null; - constructor(options?: NormalisedClientOptions) { - const connectivityCheckUrl = options?.connectivityCheckUrl || Defaults.connectivityCheckUrl; - const connectivityCheckParams = options?.connectivityCheckParams ?? null; - const connectivityUrlIsDefault = !options?.connectivityCheckUrl; + constructor(client?: BaseClient) { + this.client = client ?? null; + const connectivityCheckUrl = client?.options.connectivityCheckUrl || Defaults.connectivityCheckUrl; + const connectivityCheckParams = client?.options.connectivityCheckParams ?? null; + const connectivityUrlIsDefault = !client?.options.connectivityCheckUrl; if (Platform.Config.xhrSupported) { this.supportsAuthHeaders = true; this.Request = function ( method: HttpMethods, - client: BaseClient | null, uri: string, headers: Record | null, params: RequestParams, @@ -67,14 +67,14 @@ const Http: IHttpStatic = class { params, body, XHRStates.REQ_SEND, - client && client.options.timeouts, + (client && client.options.timeouts) ?? null, method ); req.once('complete', callback); req.exec(); return req; }; - if (options?.disableConnectivityCheck) { + if (client?.options.disableConnectivityCheck) { this.checkConnectivity = function (callback: (err: null, connectivity: true) => void) { callback(null, true); }; @@ -87,7 +87,6 @@ const Http: IHttpStatic = class { ); this.doUri( HttpMethods.Get, - null as any, connectivityCheckUrl, null, null, @@ -107,17 +106,19 @@ const Http: IHttpStatic = class { } } else if (Platform.Config.fetchSupported) { this.supportsAuthHeaders = true; - this.Request = fetchRequest; + this.Request = (method, uri, headers, params, body, callback) => { + fetchRequest(method, client ?? null, uri, headers, params, body, callback); + }; this.checkConnectivity = function (callback: (err: ErrorInfo | null, connectivity: boolean) => void) { Logger.logAction(Logger.LOG_MICRO, '(Fetch)Http.checkConnectivity()', 'Sending; ' + connectivityCheckUrl); - this.doUri(HttpMethods.Get, null as any, connectivityCheckUrl, null, null, null, function (err, responseText) { + this.doUri(HttpMethods.Get, connectivityCheckUrl, null, null, null, function (err, responseText) { const result = !err && (responseText as string)?.replace(/\n/, '') == 'yes'; Logger.logAction(Logger.LOG_MICRO, '(Fetch)Http.checkConnectivity()', 'Result: ' + result); callback(null, result); }); }; } else { - this.Request = (method, client, uri, headers, params, body, callback) => { + this.Request = (method, uri, headers, params, body, callback) => { callback(new PartialErrorInfo('no supported HTTP transports available', null, 400), null); }; } @@ -126,13 +127,18 @@ const Http: IHttpStatic = class { /* Unlike for doUri, the 'client' param here is mandatory, as it's used to generate the hosts */ do( method: HttpMethods, - client: BaseClient, path: string, headers: Record | null, body: unknown, params: RequestParams, callback?: RequestCallback ): void { + /* Unlike for doUri, the presence of `this.client` here is mandatory, as it's used to generate the hosts */ + const client = this.client; + if (!client) { + throw new Error('http.do called without client'); + } + const uriFromHost = typeof path == 'function' ? path @@ -148,12 +154,12 @@ const Http: IHttpStatic = class { callback?.(new PartialErrorInfo('Request invoked before assigned to', null, 500)); return; } - this.Request(method, client, uriFromHost(currentFallback.host), headers, params, body, (err?, ...args) => { + this.Request(method, uriFromHost(currentFallback.host), headers, params, body, (err?, ...args) => { // This typecast is safe because ErrnoExceptions are only thrown in NodeJS if (err && shouldFallback(err as ErrorInfo)) { /* unstore the fallback and start from the top with the default sequence */ client._currentFallback = null; - this.do(method, client, path, headers, body, params, callback); + this.do(method, path, headers, body, params, callback); return; } callback?.(err, ...args); @@ -169,14 +175,14 @@ const Http: IHttpStatic = class { /* if there is only one host do it */ if (hosts.length === 1) { - this.doUri(method, client, uriFromHost(hosts[0]), headers, body, params, callback as RequestCallback); + this.doUri(method, uriFromHost(hosts[0]), headers, body, params, callback as RequestCallback); return; } /* hosts is an array with preferred host plus at least one fallback */ const tryAHost = (candidateHosts: Array, persistOnSuccess?: boolean) => { const host = candidateHosts.shift(); - this.doUri(method, client, uriFromHost(host as string), headers, body, params, function (err, ...args) { + this.doUri(method, uriFromHost(host as string), headers, body, params, function (err, ...args) { // This typecast is safe because ErrnoExceptions are only thrown in NodeJS if (err && shouldFallback(err as ErrorInfo) && candidateHosts.length) { tryAHost(candidateHosts, true); @@ -197,7 +203,6 @@ const Http: IHttpStatic = class { doUri( method: HttpMethods, - client: BaseClient | null, uri: string, headers: Record | null, body: unknown, @@ -208,12 +213,11 @@ const Http: IHttpStatic = class { callback(new PartialErrorInfo('Request invoked before assigned to', null, 500)); return; } - this.Request(method, client, uri, headers, params, body, callback); + this.Request(method, uri, headers, params, body, callback); } Request?: ( method: HttpMethods, - client: BaseClient | null, uri: string, headers: Record | null, params: RequestParams, diff --git a/test/browser/modules.test.js b/test/browser/modules.test.js index e8f4c2320e..201767c3c6 100644 --- a/test/browser/modules.test.js +++ b/test/browser/modules.test.js @@ -285,7 +285,7 @@ describe('browser/modules', function () { const channelName = 'channel'; const channel = rest.channels.get(channelName); const contentTypeUsedForPublishPromise = new Promise((resolve, reject) => { - rest.http.do = (method, client, path, headers, body, params, callback) => { + rest.http.do = (method, path, headers, body, params, callback) => { if (!(method == 'post' && path == `/channels/${channelName}/messages`)) { return; } diff --git a/test/realtime/auth.test.js b/test/realtime/auth.test.js index 92dde3019a..affb68bae6 100644 --- a/test/realtime/auth.test.js +++ b/test/realtime/auth.test.js @@ -22,7 +22,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async */ function getJWT(params, callback) { var authUrl = echoServer + '/createJWT'; - http.doUri('get', null, authUrl, null, null, params, function (err, body) { + http.doUri('get', authUrl, null, null, params, function (err, body) { if (err) { callback(err, null); } diff --git a/test/rest/http.test.js b/test/rest/http.test.js index 315f751551..dcd7ee8262 100644 --- a/test/rest/http.test.js +++ b/test/rest/http.test.js @@ -25,7 +25,7 @@ define(['ably', 'shared_helper', 'chai'], function (Ably, helper, chai) { var originalDo = rest.http.do; // Intercept Http.do with test - function testRequestHandler(method, rest, path, headers, body, params, callback) { + function testRequestHandler(method, path, headers, body, params, callback) { expect('X-Ably-Version' in headers, 'Verify version header exists').to.be.ok; expect('Ably-Agent' in headers, 'Verify agent header exists').to.be.ok; @@ -47,7 +47,7 @@ define(['ably', 'shared_helper', 'chai'], function (Ably, helper, chai) { expect(headers['Ably-Agent'].indexOf('nodejs') > -1, 'Verify agent').to.be.ok; } - originalDo.call(rest.http, method, rest, path, headers, body, params, callback); + originalDo.call(rest.http, method, path, headers, body, params, callback); } rest.http.do = testRequestHandler; diff --git a/test/rest/message.test.js b/test/rest/message.test.js index 389e0b7d74..090e6eba70 100644 --- a/test/rest/message.test.js +++ b/test/rest/message.test.js @@ -157,8 +157,8 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async originalPublish.apply(channel, arguments); }; - Ably.Rest.Platform.Http.doUri = function (method, rest, uri, headers, body, params, callback) { - originalDoUri(method, rest, uri, headers, body, params, function (err) { + Ably.Rest.Platform.Http.doUri = function (method, uri, headers, body, params, callback) { + originalDoUri(method, uri, headers, body, params, function (err) { if (err) { callback(err); return; diff --git a/test/rest/request.test.js b/test/rest/request.test.js index 7055f46191..c123e94606 100644 --- a/test/rest/request.test.js +++ b/test/rest/request.test.js @@ -25,7 +25,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async restTestOnJsonMsgpack('request_version', function (rest) { const version = 150; // arbitrarily chosen - function testRequestHandler(_, __, ___, headers) { + function testRequestHandler(_, __, headers) { try { expect('X-Ably-Version' in headers, 'Verify version header exists').to.be.ok; expect(headers['X-Ably-Version']).to.equal(version.toString(), 'Verify version number sent in request');