From af3c761ab08ba2759876cf1b2d8decbb6f6e6f07 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 15:22:53 +0000 Subject: [PATCH] Convert Resource request methods to use promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `throwError` parameter here is to handle the fact that, when using an HttpPaginatedResponse, PaginatedResource wishes to have access to both the error that comes from the HTTP client _and_ the response body, before deciding how to handle an error (see the `returnErrOnly` function). I’ve converted Resource.do to return a promise here, but I’ll do the full conversion of that method in a separate commit for readability’s sake. --- src/common/lib/client/paginatedresource.ts | 118 +++---------- src/common/lib/client/push.ts | 142 ++++++--------- src/common/lib/client/resource.ts | 195 ++++++++++++++++++--- src/common/lib/client/rest.ts | 85 +++------ src/common/lib/client/restchannel.ts | 20 +-- src/common/lib/client/restchannelmixin.ts | 15 +- 6 files changed, 298 insertions(+), 277 deletions(-) diff --git a/src/common/lib/client/paginatedresource.ts b/src/common/lib/client/paginatedresource.ts index ba054026cd..dab858daae 100644 --- a/src/common/lib/client/paginatedresource.ts +++ b/src/common/lib/client/paginatedresource.ts @@ -1,6 +1,6 @@ import * as Utils from '../util/utils'; import Logger from '../util/logger'; -import Resource from './resource'; +import Resource, { ResourceResult } from './resource'; import { IPartialErrorInfo } from '../types/errorinfo'; import BaseClient from './baseclient'; import { RequestCallbackHeaders } from 'common/types/http'; @@ -59,115 +59,63 @@ class PaginatedResource { } async get(params: Record): Promise> { - return new Promise((resolve) => { - Resource.get( - this.client, - this.path, - this.headers, - params, - this.envelope, - (err, body, headers, unpacked, statusCode) => { - resolve(this.handlePage(err, body, headers, unpacked, statusCode)); - } - ); - }); + const result = await Resource.get(this.client, this.path, this.headers, params, this.envelope, false); + return this.handlePage(result); } async delete(params: Record): Promise> { - return new Promise((resolve) => { - Resource.delete( - this.client, - this.path, - this.headers, - params, - this.envelope, - (err, body, headers, unpacked, statusCode) => { - resolve(this.handlePage(err, body, headers, unpacked, statusCode)); - } - ); - }); + const result = await Resource.delete(this.client, this.path, this.headers, params, this.envelope, false); + return this.handlePage(result); } async post(params: Record, body: unknown): Promise> { - return new Promise((resolve) => { - Resource.post( - this.client, - this.path, - body, - this.headers, - params, - this.envelope, - (err, responseBody, headers, unpacked, statusCode) => { - resolve(this.handlePage(err, responseBody, headers, unpacked, statusCode)); - } - ); - }); + const result = await Resource.post(this.client, this.path, body, this.headers, params, this.envelope, false); + return this.handlePage(result); } async put(params: Record, body: unknown): Promise> { - return new Promise((resolve) => { - Resource.put( - this.client, - this.path, - body, - this.headers, - params, - this.envelope, - (err, responseBody, headers, unpacked, statusCode) => { - resolve(this.handlePage(err, responseBody, headers, unpacked, statusCode)); - } - ); - }); + const result = await Resource.put(this.client, this.path, body, this.headers, params, this.envelope, false); + return this.handlePage(result); } async patch(params: Record, body: unknown): Promise> { - return new Promise((resolve) => { - Resource.patch( - this.client, - this.path, - body, - this.headers, - params, - this.envelope, - (err, responseBody, headers, unpacked, statusCode) => { - resolve(this.handlePage(err, responseBody, headers, unpacked, statusCode)); - } - ); - }); + const result = await Resource.patch(this.client, this.path, body, this.headers, params, this.envelope, false); + return this.handlePage(result); } - async handlePage( - err: IPartialErrorInfo | null, - body: unknown, - headers: RequestCallbackHeaders | undefined, - unpacked: boolean | undefined, - statusCode: number | undefined - ): Promise> { - if (err && returnErrOnly(err, body, this.useHttpPaginatedResponse)) { + async handlePage(result: ResourceResult): Promise> { + if (result.err && returnErrOnly(result.err, result.body, this.useHttpPaginatedResponse)) { Logger.logAction( Logger.LOG_ERROR, 'PaginatedResource.handlePage()', - 'Unexpected error getting resource: err = ' + Utils.inspectError(err) + 'Unexpected error getting resource: err = ' + Utils.inspectError(result.err) ); - throw err; + throw result.err; } let items, linkHeader, relParams; try { - items = await this.bodyHandler(body, headers || {}, unpacked); + items = await this.bodyHandler(result.body, result.headers || {}, result.unpacked); } catch (e) { /* If we got an error, the failure to parse the body is almost certainly * due to that, so throw that in preference over the parse error */ - throw err || e; + throw result.err || e; } - if (headers && (linkHeader = headers['Link'] || headers['link'])) { + if (result.headers && (linkHeader = result.headers['Link'] || result.headers['link'])) { relParams = parseRelLinks(linkHeader); } if (this.useHttpPaginatedResponse) { - return new HttpPaginatedResponse(this, items, headers || {}, statusCode as number, relParams, err); + return new HttpPaginatedResponse( + this, + items, + result.headers || {}, + result.statusCode as number, + relParams, + result.err + ); } else { return new PaginatedResult(this, items, relParams); } @@ -220,18 +168,8 @@ export class PaginatedResult { * the rest of a multipage set of results can always be done with GET */ async get(params: any): Promise> { const res = this.resource; - return new Promise((resolve) => { - Resource.get( - res.client, - res.path, - res.headers, - params, - res.envelope, - function (err, body, headers, unpacked, statusCode) { - resolve(res.handlePage(err, body, headers, unpacked, statusCode)); - } - ); - }); + const result = await Resource.get(res.client, res.path, res.headers, params, res.envelope, false); + return res.handlePage(result); } } diff --git a/src/common/lib/client/push.ts b/src/common/lib/client/push.ts index e6ab5b3083..4ec11dcba8 100644 --- a/src/common/lib/client/push.ts +++ b/src/common/lib/client/push.ts @@ -40,11 +40,7 @@ class Admin { if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); const requestBody = Utils.encodeBody(body, client._MsgPack, format); - return new Promise((resolve, reject) => { - Resource.post(client, '/push/publish', requestBody, headers, params, null, (err) => - err ? reject(err) : resolve() - ); - }); + await Resource.post(client, '/push/publish', requestBody, headers, params, null, true); } } @@ -67,27 +63,21 @@ class DeviceRegistrations { if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); const requestBody = Utils.encodeBody(body, client._MsgPack, format); - return new Promise((resolve, reject) => { - Resource.put( - client, - '/push/deviceRegistrations/' + encodeURIComponent(device.id), - requestBody, - headers, - params, - null, - (err, body, headers, unpacked) => { - err - ? reject(err) - : resolve( - DeviceDetails.fromResponseBody( - body as Record, - client._MsgPack, - unpacked ? undefined : format - ) as DeviceDetails - ); - } - ); - }); + const response = await Resource.put( + client, + '/push/deviceRegistrations/' + encodeURIComponent(device.id), + requestBody, + headers, + params, + null, + true + ); + + return DeviceDetails.fromResponseBody( + response.body as Record, + client._MsgPack, + response.unpacked ? undefined : format + ) as DeviceDetails; } async get(deviceIdOrDetails: any): Promise { @@ -106,26 +96,20 @@ class DeviceRegistrations { Utils.mixin(headers, client.options.headers); - return new Promise((resolve, reject) => { - Resource.get( - client, - '/push/deviceRegistrations/' + encodeURIComponent(deviceId), - headers, - {}, - null, - function (err, body, headers, unpacked) { - err - ? reject(err) - : resolve( - DeviceDetails.fromResponseBody( - body as Record, - client._MsgPack, - unpacked ? undefined : format - ) as DeviceDetails - ); - } - ); - }); + const response = await Resource.get( + client, + '/push/deviceRegistrations/' + encodeURIComponent(deviceId), + headers, + {}, + null, + true + ); + + return DeviceDetails.fromResponseBody( + response.body as Record, + client._MsgPack, + response.unpacked ? undefined : format + ) as DeviceDetails; } async list(params: any): Promise> { @@ -168,16 +152,14 @@ class DeviceRegistrations { if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); - return new Promise((resolve, reject) => { - Resource['delete']( - client, - '/push/deviceRegistrations/' + encodeURIComponent(deviceId), - headers, - params, - null, - (err) => (err ? reject(err) : resolve()) - ); - }); + await Resource['delete']( + client, + '/push/deviceRegistrations/' + encodeURIComponent(deviceId), + headers, + params, + null, + true + ); } async removeWhere(params: any): Promise { @@ -189,11 +171,7 @@ class DeviceRegistrations { if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); - return new Promise((resolve, reject) => { - Resource['delete'](client, '/push/deviceRegistrations', headers, params, null, (err) => - err ? reject(err) : resolve() - ); - }); + await Resource['delete'](client, '/push/deviceRegistrations', headers, params, null, true); } } @@ -216,27 +194,21 @@ class ChannelSubscriptions { if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); const requestBody = Utils.encodeBody(body, client._MsgPack, format); - return new Promise((resolve, reject) => { - Resource.post( - client, - '/push/channelSubscriptions', - requestBody, - headers, - params, - null, - function (err, body, headers, unpacked) { - err - ? reject(err) - : resolve( - PushChannelSubscription.fromResponseBody( - body as Record, - client._MsgPack, - unpacked ? undefined : format - ) as PushChannelSubscription - ); - } - ); - }); + const response = await Resource.post( + client, + '/push/channelSubscriptions', + requestBody, + headers, + params, + null, + true + ); + + return PushChannelSubscription.fromResponseBody( + response.body as Record, + client._MsgPack, + response.unpacked ? undefined : format + ) as PushChannelSubscription; } async list(params: any): Promise> { @@ -269,11 +241,7 @@ class ChannelSubscriptions { if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); - return new Promise((resolve, reject) => { - Resource['delete'](client, '/push/channelSubscriptions', headers, params, null, (err) => - err ? reject(err) : resolve() - ); - }); + await Resource['delete'](client, '/push/channelSubscriptions', headers, params, null, true); } /* ChannelSubscriptions have no unique id; removing one is equivalent to removeWhere by its properties */ diff --git a/src/common/lib/client/resource.ts b/src/common/lib/client/resource.ts index 270895542e..32bc70062f 100644 --- a/src/common/lib/client/resource.ts +++ b/src/common/lib/client/resource.ts @@ -141,66 +141,191 @@ export type ResourceCallback = ( statusCode?: number ) => void; +export interface ResourceResponse { + body?: T; + headers?: RequestCallbackHeaders; + unpacked?: boolean; + statusCode?: number; +} + +export interface ResourceResult extends ResourceResponse { + /** + * Any error returned by the underlying HTTP client. + */ + err: IPartialErrorInfo | null; +} + class Resource { - static get( + /** + * @param throwError Whether to throw any error returned by the underlying HTTP client. + * + * If you specify `true`, then this method will return a `ResourceResponse`, and if the underlying HTTP client returns an error, this method call will throw that error. If you specify `false`, then it will return a `ResourceResult`, whose `err` property contains any error that was returned by the underlying HTTP client. + */ + static async get( + client: BaseClient, + path: string, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: true + ): Promise>; + static async get( client: BaseClient, path: string, headers: Record, params: Record, envelope: Utils.Format | null, - callback: ResourceCallback - ): void { - Resource.do(HttpMethods.Get, client, path, null, headers, params, envelope, callback); + throwError: false + ): Promise>; + static async get( + client: BaseClient, + path: string, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: boolean + ): Promise | ResourceResult> { + return Resource.do(HttpMethods.Get, client, path, null, headers, params, envelope, throwError ?? false); } - static delete( + /** + * @param throwError Whether to throw any error returned by the underlying HTTP client. + * + * If you specify `true`, then this method will return a `ResourceResponse`, and if the underlying HTTP client returns an error, this method call will throw that error. If you specify `false`, then it will return a `ResourceResult`, whose `err` property contains any error that was returned by the underlying HTTP client. + */ + static async delete( client: BaseClient, path: string, headers: Record, params: Record, envelope: Utils.Format | null, - callback: ResourceCallback - ): void { - Resource.do(HttpMethods.Delete, client, path, null, headers, params, envelope, callback); + throwError: true + ): Promise>; + static async delete( + client: BaseClient, + path: string, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: false + ): Promise>; + static async delete( + client: BaseClient, + path: string, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: boolean + ): Promise | ResourceResult> { + return Resource.do(HttpMethods.Delete, client, path, null, headers, params, envelope, throwError); } - static post( + /** + * @param throwError Whether to throw any error returned by the underlying HTTP client. + * + * If you specify `true`, then this method will return a `ResourceResponse`, and if the underlying HTTP client returns an error, this method call will throw that error. If you specify `false`, then it will return a `ResourceResult`, whose `err` property contains any error that was returned by the underlying HTTP client. + */ + static async post( + client: BaseClient, + path: string, + body: unknown, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: true + ): Promise>; + static async post( + client: BaseClient, + path: string, + body: unknown, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: false + ): Promise>; + static async post( client: BaseClient, path: string, body: unknown, headers: Record, params: Record, envelope: Utils.Format | null, - callback: ResourceCallback - ): void { - Resource.do(HttpMethods.Post, client, path, body, headers, params, envelope, callback); + throwError: boolean + ): Promise | ResourceResult> { + return Resource.do(HttpMethods.Post, client, path, body, headers, params, envelope, throwError); } - static patch( + /** + * @param throwError Whether to throw any error returned by the underlying HTTP client. + * + * If you specify `true`, then this method will return a `ResourceResponse`, and if the underlying HTTP client returns an error, this method call will throw that error. If you specify `false`, then it will return a `ResourceResult`, whose `err` property contains any error that was returned by the underlying HTTP client. + */ + static async patch( + client: BaseClient, + path: string, + body: unknown, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: true + ): Promise>; + static async patch( client: BaseClient, path: string, body: unknown, headers: Record, params: Record, envelope: Utils.Format | null, - callback: ResourceCallback - ): void { - Resource.do(HttpMethods.Patch, client, path, body, headers, params, envelope, callback); + throwError: false + ): Promise>; + static async patch( + client: BaseClient, + path: string, + body: unknown, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: boolean + ): Promise | ResourceResult> { + return Resource.do(HttpMethods.Patch, client, path, body, headers, params, envelope, throwError); } - static put( + /** + * @param throwError Whether to throw any error returned by the underlying HTTP client. + * + * If you specify `true`, then this method will return a `ResourceResponse`, and if the underlying HTTP client returns an error, this method call will throw that error. If you specify `false`, then it will return a `ResourceResult`, whose `err` property contains any error that was returned by the underlying HTTP client. + */ + static async put( client: BaseClient, path: string, body: unknown, headers: Record, params: Record, envelope: Utils.Format | null, - callback: ResourceCallback - ): void { - Resource.do(HttpMethods.Put, client, path, body, headers, params, envelope, callback); + throwError: true + ): Promise>; + static async put( + client: BaseClient, + path: string, + body: unknown, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: false + ): Promise>; + static async put( + client: BaseClient, + path: string, + body: unknown, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: boolean + ): Promise | ResourceResult> { + return Resource.do(HttpMethods.Put, client, path, body, headers, params, envelope, throwError); } - static do( + static async do( method: HttpMethods, client: BaseClient, path: string, @@ -208,14 +333,30 @@ class Resource { headers: Record, params: Record, envelope: Utils.Format | null, - callback: ResourceCallback - ): void { + throwError: boolean + ): Promise | ResourceResult> { + let callback: ResourceCallback; + + const promise = new Promise | ResourceResult>((resolve, reject) => { + callback = (err, body, headers, unpacked, statusCode) => { + if (throwError) { + if (err) { + reject(err); + } else { + resolve({ body, headers, unpacked, statusCode }); + } + } else { + resolve({ err, body, headers, unpacked, statusCode }); + } + }; + }); + if (Logger.shouldLog(Logger.LOG_MICRO)) { - callback = logResponseHandler(callback, method, path, params); + callback = logResponseHandler(callback!, method, path, params); } if (envelope) { - callback = callback && unenvelope(callback, client._MsgPack, envelope); + callback = unenvelope(callback!, client._MsgPack, envelope); (params = params || {})['envelope'] = envelope; } @@ -268,7 +409,9 @@ class Resource { }); } - withAuthDetails(client, headers, params, callback, doRequest); + withAuthDetails(client, headers, params, callback!, doRequest); + + return promise; } } diff --git a/src/common/lib/client/rest.ts b/src/common/lib/client/rest.ts index 3468f75e90..8de6a017c3 100644 --- a/src/common/lib/client/rest.ts +++ b/src/common/lib/client/rest.ts @@ -170,25 +170,19 @@ export class Rest { if (this.client.options.headers) Utils.mixin(headers, this.client.options.headers); const requestBody = Utils.encodeBody(requestBodyDTO, this.client._MsgPack, format); - return new Promise((resolve, reject) => { - Resource.post(this.client, '/messages', requestBody, headers, {}, null, (err, body, headers, unpacked) => { - if (err) { - reject(err); - return; - } - const batchResults = ( - unpacked ? body : Utils.decodeBody(body, this.client._MsgPack, format) - ) as BatchPublishResult[]; + const response = await Resource.post(this.client, '/messages', requestBody, headers, {}, null, true); - // I don't love the below type assertions for `resolve` but not sure how to avoid them - if (singleSpecMode) { - (resolve as (result: BatchPublishResult) => void)(batchResults[0]); - } else { - (resolve as (result: BatchPublishResult[]) => void)(batchResults); - } - }); - }); + const batchResults = ( + response.unpacked ? response.body : Utils.decodeBody(response.body, this.client._MsgPack, format) + ) as BatchPublishResult[]; + + // I don't love the below type assertions but not sure how to avoid them + if (singleSpecMode) { + return batchResults[0] as T extends BatchPublishSpec ? BatchPublishResult : BatchPublishResult[]; + } else { + return batchResults as T extends BatchPublishSpec ? BatchPublishResult : BatchPublishResult[]; + } } async batchPresence(channels: string[]): Promise { @@ -199,27 +193,11 @@ export class Rest { const channelsParam = channels.join(','); - return new Promise((resolve, reject) => { - Resource.get( - this.client, - '/presence', - headers, - { channels: channelsParam }, - null, - (err, body, headers, unpacked) => { - if (err) { - reject(err); - return; - } - - const batchResult = ( - unpacked ? body : Utils.decodeBody(body, this.client._MsgPack, format) - ) as BatchPresenceResult; + const response = await Resource.get(this.client, '/presence', headers, { channels: channelsParam }, null, true); - resolve(batchResult); - } - ); - }); + return ( + response.unpacked ? response.body : Utils.decodeBody(response.body, this.client._MsgPack, format) + ) as BatchPresenceResult; } async revokeTokens( @@ -246,28 +224,19 @@ export class Rest { const requestBody = Utils.encodeBody(requestBodyDTO, this.client._MsgPack, format); - return new Promise((resolve, reject) => { - Resource.post( - this.client, - `/keys/${keyName}/revokeTokens`, - requestBody, - headers, - {}, - null, - (err, body, headers, unpacked) => { - if (err) { - reject(err); - return; - } - - const batchResult = ( - unpacked ? body : Utils.decodeBody(body, this.client._MsgPack, format) - ) as TokenRevocationResult; + const response = await Resource.post( + this.client, + `/keys/${keyName}/revokeTokens`, + requestBody, + headers, + {}, + null, + true + ); - resolve(batchResult); - } - ); - }); + return ( + response.unpacked ? response.body : Utils.decodeBody(response.body, this.client._MsgPack, format) + ) as TokenRevocationResult; } setLog(logOptions: LoggerOptions): void { diff --git a/src/common/lib/client/restchannel.ts b/src/common/lib/client/restchannel.ts index 79338a716e..818d0cea5b 100644 --- a/src/common/lib/client/restchannel.ts +++ b/src/common/lib/client/restchannel.ts @@ -126,17 +126,15 @@ class RestChannel { } async _publish(requestBody: unknown, headers: Record, params: any): Promise { - return new Promise((resolve, reject) => { - Resource.post( - this.client, - this.client.rest.channelMixin.basePath(this) + '/messages', - requestBody, - headers, - params, - null, - (err) => (err ? reject(err) : resolve()) - ); - }); + await Resource.post( + this.client, + this.client.rest.channelMixin.basePath(this) + '/messages', + requestBody, + headers, + params, + null, + true + ); } async status(): Promise { diff --git a/src/common/lib/client/restchannelmixin.ts b/src/common/lib/client/restchannelmixin.ts index fa10386a8d..481067355c 100644 --- a/src/common/lib/client/restchannelmixin.ts +++ b/src/common/lib/client/restchannelmixin.ts @@ -44,10 +44,15 @@ export class RestChannelMixin { const format = channel.client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json; const headers = Defaults.defaultPostHeaders(channel.client.options, { format }); - return new Promise((resolve, reject) => { - Resource.get(channel.client, this.basePath(channel), headers, {}, format, (err, result) => - err ? reject(err) : resolve(result!) - ); - }); + const response = await Resource.get( + channel.client, + this.basePath(channel), + headers, + {}, + format, + true + ); + + return response.body!; } }