Skip to content

Commit

Permalink
Convert http.d.ts to .ts
Browse files Browse the repository at this point in the history
This fixes some compilation errors that were missed by the fact that it
was a .d.ts file. (See #1445 for the issue that aims to convert all
.d.ts files to .ts.)

This improved type information introduced a compiler error resulting
from us trying to directly access HTTP headers on DOM’s Headers object
with header names as object property names. I can’t see how the existing
code could have been working properly, so I’ve changed the response
header handling in fetchrequest.ts.
  • Loading branch information
lawrence-forooghian committed Nov 8, 2023
1 parent 9973109 commit 1b806ee
Show file tree
Hide file tree
Showing 9 changed files with 116 additions and 164 deletions.
9 changes: 5 additions & 4 deletions src/common/lib/client/paginatedresource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,9 @@ import Resource from './resource';
import ErrorInfo, { IPartialErrorInfo } from '../types/errorinfo';
import { PaginatedResultCallback } from '../../types/utils';
import BaseClient from './baseclient';
import { RequestCallbackHeaders } from 'common/types/http';

export type BodyHandler = (body: unknown, headers: Record<string, string>, unpacked?: boolean) => Promise<any>;
export type BodyHandler = (body: unknown, headers: RequestCallbackHeaders, unpacked?: boolean) => Promise<any>;

function getRelParams(linkUrl: string) {
const urlMatch = linkUrl.match(/^\.\/(\w+)\?(.*)$/);
Expand Down Expand Up @@ -135,7 +136,7 @@ class PaginatedResource {
handlePage<T>(
err: IPartialErrorInfo | null,
body: unknown,
headers: Record<string, string> | undefined,
headers: RequestCallbackHeaders | undefined,
unpacked: boolean | undefined,
statusCode: number | undefined,
callback: PaginatedResultCallback<T>
Expand Down Expand Up @@ -249,14 +250,14 @@ export class PaginatedResult<T> {
export class HttpPaginatedResponse<T> extends PaginatedResult<T> {
statusCode: number;
success: boolean;
headers: Record<string, string>;
headers: RequestCallbackHeaders;
errorCode?: number | null;
errorMessage?: string | null;

constructor(
resource: PaginatedResource,
items: T[],
headers: Record<string, string>,
headers: RequestCallbackHeaders,
statusCode: number,
relParams: any,
err: IPartialErrorInfo | null
Expand Down
48 changes: 17 additions & 31 deletions src/common/lib/client/resource.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,12 @@ import Auth from './auth';
import HttpMethods from '../../constants/HttpMethods';
import ErrorInfo, { IPartialErrorInfo, PartialErrorInfo } from '../types/errorinfo';
import BaseClient from './baseclient';
import { ErrnoException } from '../../types/http';
import { MsgPack } from 'common/types/msgpack';
import { RequestCallbackHeaders } from 'common/types/http';

function withAuthDetails(
client: BaseClient,
headers: Record<string, string>,
headers: RequestCallbackHeaders | undefined,
params: Record<string, any>,
errCallback: Function,
opCallback: Function
Expand Down Expand Up @@ -130,7 +130,7 @@ function logResponseHandler<T>(
export type ResourceCallback<T = unknown> = (
err: IPartialErrorInfo | null,
body?: T,
headers?: Record<string, string>,
headers?: RequestCallbackHeaders,
unpacked?: boolean,
statusCode?: number
) => void;
Expand Down Expand Up @@ -245,35 +245,21 @@ class Resource {
);
}

client.http.do(
method,
client,
path,
headers,
body,
params,
function (
err: ErrorInfo | ErrnoException | null | undefined,
res: any,
headers: Record<string, string>,
unpacked?: boolean,
statusCode?: number
) {
if (err && Auth.isTokenErr(err as ErrorInfo)) {
/* token has expired, so get a new one */
client.auth.authorize(null, null, function (err: ErrorInfo) {
if (err) {
callback(err);
return;
}
/* retry ... */
withAuthDetails(client, headers, params, callback, doRequest);
});
return;
}
callback(err as ErrorInfo, res, headers, unpacked, statusCode);
client.http.do(method, client, 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) {
if (err) {
callback(err);
return;
}
/* retry ... */
withAuthDetails(client, headers, params, callback, doRequest);
});
return;
}
);
callback(err as ErrorInfo, res as T | undefined, headers, unpacked, statusCode);
});
}

withAuthDetails(client, headers, params, callback, doRequest);
Expand Down
9 changes: 2 additions & 7 deletions src/common/lib/client/rest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import Stats from '../types/stats';
import HttpMethods from '../../constants/HttpMethods';
import { ChannelOptions } from '../../types/channel';
import { PaginatedResultCallback, StandardCallback } from '../../types/utils';
import { ErrnoException, RequestParams } from '../../types/http';
import { RequestParams } from '../../types/http';
import * as API from '../../../../ably';
import Resource from './resource';

Expand Down Expand Up @@ -88,12 +88,7 @@ export class Rest {
headers,
null,
params as RequestParams,
(
err?: ErrorInfo | ErrnoException | null,
res?: unknown,
headers?: Record<string, string>,
unpacked?: boolean
) => {
(err, res, headers, unpacked) => {
if (err) {
_callback(err);
return;
Expand Down
4 changes: 2 additions & 2 deletions src/common/platform.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { IPlatformConfig } from './types/IPlatformConfig';
import { IHttp } from './types/http';
import { IHttpStatic } from './types/http';
import { TransportInitialiser } from './lib/transport/connectionmanager';
import IDefaults from './types/IDefaults';
import IWebStorage from './types/IWebStorage';
Expand Down Expand Up @@ -31,7 +31,7 @@ export default class Platform {
comment above.
*/
static Crypto: IUntypedCryptoStatic | null;
static Http: typeof IHttp;
static Http: IHttpStatic;
static Transports: {
order: TransportName[];
// Transport implementations that always come with this platform
Expand Down
23 changes: 14 additions & 9 deletions src/common/types/http.d.ts → src/common/types/http.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,28 @@
import HttpMethods from '../constants/HttpMethods';
import { BaseClient } from '../lib/client/baseclient';
import ErrorInfo from '../lib/types/errorinfo';
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 RequestCallbackHeaders = Partial<Record<string, string | string[]>>;
export type RequestCallback = (
error?: ErrnoException | IPartialErrorInfo | null,
body?: unknown,
headers?: IncomingHttpHeaders,
headers?: RequestCallbackHeaders,
unpacked?: boolean,
statusCode?: number
) => void;
export type RequestParams = Record<string, string> | null;

export declare class IHttp {
constructor(options: NormalisedClientOptions);
static methods: Array<HttpMethods>;
static methodsWithBody: Array<HttpMethods>;
static methodsWithoutBody: Array<HttpMethods>;
export interface IHttpStatic {
new (options: NormalisedClientOptions): IHttp;
methods: Array<HttpMethods>;
methodsWithBody: Array<HttpMethods>;
methodsWithoutBody: Array<HttpMethods>;
}

export interface IHttp {
supportsAuthHeaders: boolean;
supportsLinkHeaders: boolean;
agent?: Agents | null;
Expand All @@ -32,7 +37,7 @@ export declare class IHttp {
body: unknown,
callback: RequestCallback
) => void;
_getHosts: (client: BaseClient | Realtime) => string[];
_getHosts: (client: BaseClient) => string[];
do(
method: HttpMethods,
client: BaseClient | null,
Expand Down
76 changes: 30 additions & 46 deletions src/platform/nodejs/lib/util/http.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,13 @@
import Platform from 'common/platform';
import Defaults from 'common/lib/util/defaults';
import ErrorInfo from 'common/lib/types/errorinfo';
import { ErrnoException, IHttp, PathParameter, RequestCallback, RequestParams } from '../../../../common/types/http';
import {
ErrnoException,
IHttpStatic,
PathParameter,
RequestCallback,
RequestParams,
} from '../../../../common/types/http';
import HttpMethods from '../../../../common/constants/HttpMethods';
import got, { Response, Options, CancelableRequest, Agents } from 'got';
import http from 'http';
Expand Down Expand Up @@ -91,7 +97,7 @@ function getHosts(client: BaseClient): string[] {
return Defaults.getHosts(client.options);
}

const Http: typeof IHttp = class {
const Http: IHttpStatic = class {
static methods = [HttpMethods.Get, HttpMethods.Delete, HttpMethods.Post, HttpMethods.Put, HttpMethods.Patch];
static methodsWithoutBody = [HttpMethods.Get, HttpMethods.Delete];
static methodsWithBody = [HttpMethods.Post, HttpMethods.Put, HttpMethods.Patch];
Expand Down Expand Up @@ -126,23 +132,15 @@ const Http: typeof IHttp = class {
if (currentFallback) {
if (currentFallback.validUntil > Date.now()) {
/* Use stored fallback */
this.doUri(
method,
client,
uriFromHost(currentFallback.host),
headers,
body,
params,
(err?: ErrnoException | ErrorInfo | null, ...args: unknown[]) => {
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);
return;
}
callback(err, ...args);
this.doUri(method, client, 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);
return;
}
);
callback(err, ...args);
});
return;
} else {
/* Fallback expired; remove it and fallthrough to normal sequence */
Expand All @@ -160,28 +158,20 @@ const Http: typeof IHttp = class {

const tryAHost = (candidateHosts: Array<string>, persistOnSuccess?: boolean) => {
const host = candidateHosts.shift();
this.doUri(
method,
client,
uriFromHost(host as string),
headers,
body,
params,
function (err?: ErrnoException | ErrorInfo | null, ...args: unknown[]) {
if (err && shouldFallback(err as ErrnoException) && candidateHosts.length) {
tryAHost(candidateHosts, true);
return;
}
if (persistOnSuccess) {
/* RSC15f */
client._currentFallback = {
host: host as string,
validUntil: Date.now() + client.options.timeouts.fallbackRetryTimeout,
};
}
callback(err, ...args);
this.doUri(method, client, uriFromHost(host as string), headers, body, params, function (err, ...args) {
if (err && shouldFallback(err as ErrnoException) && candidateHosts.length) {
tryAHost(candidateHosts, true);
return;
}
);
if (persistOnSuccess) {
/* RSC15f */
client._currentFallback = {
host: host as string,
validUntil: Date.now() + client.options.timeouts.fallbackRetryTimeout,
};
}
callback(err, ...args);
});
};
tryAHost(hosts);
}
Expand Down Expand Up @@ -260,13 +250,7 @@ const Http: typeof IHttp = class {
null,
null,
connectivityCheckParams,
function (
err?: ErrnoException | ErrorInfo | null,
responseText?: unknown,
headers?: any,
unpacked?: boolean,
statusCode?: number
) {
function (err, responseText, headers, unpacked, statusCode) {
if (!err && !connectivityUrlIsDefault) {
callback(null, isSuccessCode(statusCode as number));
return;
Expand Down
17 changes: 14 additions & 3 deletions src/platform/web/lib/transport/fetchrequest.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import HttpMethods from 'common/constants/HttpMethods';
import BaseClient from 'common/lib/client/baseclient';
import ErrorInfo, { PartialErrorInfo } from 'common/lib/types/errorinfo';
import { RequestCallback, RequestParams } from 'common/types/http';
import { RequestCallback, RequestCallbackHeaders, RequestParams } from 'common/types/http';
import Platform from 'common/platform';
import Defaults from 'common/lib/util/defaults';
import * as Utils from 'common/lib/util/utils';
Expand All @@ -17,6 +17,16 @@ function getAblyError(responseBody: unknown, headers: Headers) {
}
}

function convertHeaders(headers: Headers) {
const result: RequestCallbackHeaders = {};

headers.forEach((value, key) => {
result[key] = value;
});

return result;
}

export default function fetchRequest(
method: HttpMethods,
client: BaseClient | null,
Expand Down Expand Up @@ -64,6 +74,7 @@ export default function fetchRequest(
}
prom.then((body) => {
const unpacked = !!contentType && contentType.indexOf('application/x-msgpack') === -1;
const headers = convertHeaders(res.headers);
if (!res.ok) {
const err =
getAblyError(body, res.headers) ||
Expand All @@ -72,9 +83,9 @@ export default function fetchRequest(
null,
res.status
);
callback(err, body, res.headers, unpacked, res.status);
callback(err, body, headers, unpacked, res.status);
} else {
callback(null, body, res.headers, unpacked, res.status);
callback(null, body, headers, unpacked, res.status);
}
});
})
Expand Down
Loading

0 comments on commit 1b806ee

Please sign in to comment.