Merge pull request #1541 from ably/remove-callbacks-part-1

Remove callbacks part 1

src/common/lib/client/baseclient.ts

@@ -4,7 +4,6 @@ import Auth from './auth';
 import { HttpPaginatedResponse, PaginatedResult } from './paginatedresource';
 import ErrorInfo from '../types/errorinfo';
 import Stats from '../types/stats';
-import { StandardCallback } from '../../types/utils';
 import { Http, RequestParams } from '../../types/http';
 import ClientOptions, { NormalisedClientOptions } from '../../types/ClientOptions';
 import * as API from '../../../../ably';
@@ -129,27 +128,23 @@ class BaseClient {
     return Defaults.getHttpScheme(this.options) + host + ':' + Defaults.getPort(this.options, false);
   }
 
-  stats(
-    params: RequestParams,
-    callback: StandardCallback<PaginatedResult<Stats>>
-  ): Promise<PaginatedResult<Stats>> | void {
-    return this.rest.stats(params, callback);
+  async stats(params: RequestParams): Promise<PaginatedResult<Stats>> {
+    return this.rest.stats(params);
   }
 
-  time(params?: RequestParams | StandardCallback<number>, callback?: StandardCallback<number>): Promise<number> | void {
-    return this.rest.time(params, callback);
+  async time(params?: RequestParams): Promise<number> {
+    return this.rest.time(params);
   }
 
-  request(
+  async request(
     method: string,
     path: string,
     version: number,
     params: RequestParams,
     body: unknown,
-    customHeaders: Record<string, string>,
-    callback: StandardCallback<HttpPaginatedResponse<unknown>>
-  ): Promise<HttpPaginatedResponse<unknown>> | void {
-    return this.rest.request(method, path, version, params, body, customHeaders, callback);
+    customHeaders: Record<string, string>
+  ): Promise<HttpPaginatedResponse<unknown>> {
+    return this.rest.request(method, path, version, params, body, customHeaders);
   }
 
   batchPublish<T extends BatchPublishSpec | BatchPublishSpec[]>(

src/common/lib/client/connection.ts

@@ -1,4 +1,3 @@
-import * as Utils from '../util/utils';
 import EventEmitter from '../util/eventemitter';
 import ConnectionManager from '../transport/connectionmanager';
 import Logger from '../util/logger';
@@ -53,12 +52,11 @@ class Connection extends EventEmitter {
     this.connectionManager.requestState({ state: 'connecting' });
   }
 
-  ping(callback: Function): Promise<void> | void {
+  async ping(): Promise<number> {
     Logger.logAction(Logger.LOG_MINOR, 'Connection.ping()', '');
-    if (!callback) {
-      return Utils.promisify(this, 'ping', arguments);
-    }
-    this.connectionManager.ping(null, callback);
+    return new Promise((resolve, reject) => {
+      this.connectionManager.ping(null, (err: unknown, result: number) => (err ? reject(err) : resolve(result)));
+    });
   }
 
   close(): void {

src/common/lib/client/paginatedresource.ts

@@ -1,8 +1,7 @@
 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 { PaginatedResultCallback } from '../../types/utils';
 import BaseClient from './baseclient';
 import { RequestBody, RequestCallbackHeaders } from 'common/types/http';
 
@@ -29,7 +28,7 @@ function parseRelLinks(linkHeader: string | Array<string>) {
 
 function returnErrOnly(err: IPartialErrorInfo, body: unknown, useHPR?: boolean) {
   /* If using httpPaginatedResponse, errors from Ably are returned as part of
-   * the HPR, only do callback(err) for network errors etc. which don't
+   * the HPR, only throw `err` for network errors etc. which don't
    * return a body and/or have no ably-originated error code (non-numeric
    * error codes originate from node) */
   return !(useHPR && (body || typeof err.code === 'number'));
@@ -59,132 +58,76 @@ class PaginatedResource {
     this.useHttpPaginatedResponse = useHttpPaginatedResponse || false;
   }
 
-  get<T1, T2>(params: Record<string, T2>, callback: PaginatedResultCallback<T1>): void {
-    Resource.get(
-      this.client,
-      this.path,
-      this.headers,
-      params,
-      this.envelope,
-      (err, body, headers, unpacked, statusCode) => {
-        this.handlePage(err, body, headers, unpacked, statusCode, callback);
-      }
-    );
+  async get<T1, T2>(params: Record<string, T2>): Promise<PaginatedResult<T1>> {
+    const result = await Resource.get<T1>(this.client, this.path, this.headers, params, this.envelope, false);
+    return this.handlePage(result);
   }
 
-  delete<T1, T2>(params: Record<string, T2>, callback: PaginatedResultCallback<T1>): void {
-    Resource.delete(
-      this.client,
-      this.path,
-      this.headers,
-      params,
-      this.envelope,
-      (err, body, headers, unpacked, statusCode) => {
-        this.handlePage(err, body, headers, unpacked, statusCode, callback);
-      }
-    );
+  async delete<T1, T2>(params: Record<string, T2>): Promise<PaginatedResult<T1>> {
+    const result = await Resource.delete<T1>(this.client, this.path, this.headers, params, this.envelope, false);
+    return this.handlePage(result);
   }
 
-  post<T1, T2>(params: Record<string, T2>, body: RequestBody | null, callback: PaginatedResultCallback<T1>): void {
-    Resource.post(
-      this.client,
-      this.path,
-      body,
-      this.headers,
-      params,
-      this.envelope,
-      (err, responseBody, headers, unpacked, statusCode) => {
-        if (callback) {
-          this.handlePage(err, responseBody, headers, unpacked, statusCode, callback);
-        }
-      }
-    );
+  async post<T1, T2>(params: Record<string, T2>, body: RequestBody | null): Promise<PaginatedResult<T1>> {
+    const result = await Resource.post<T1>(this.client, this.path, body, this.headers, params, this.envelope, false);
+    return this.handlePage(result);
   }
 
-  put<T1, T2>(params: Record<string, T2>, body: RequestBody | null, callback: PaginatedResultCallback<T1>): void {
-    Resource.put(
-      this.client,
-      this.path,
-      body,
-      this.headers,
-      params,
-      this.envelope,
-      (err, responseBody, headers, unpacked, statusCode) => {
-        if (callback) {
-          this.handlePage(err, responseBody, headers, unpacked, statusCode, callback);
-        }
-      }
-    );
+  async put<T1, T2>(params: Record<string, T2>, body: RequestBody | null): Promise<PaginatedResult<T1>> {
+    const result = await Resource.put<T1>(this.client, this.path, body, this.headers, params, this.envelope, false);
+    return this.handlePage(result);
   }
 
-  patch<T1, T2>(params: Record<string, T2>, body: RequestBody | null, callback: PaginatedResultCallback<T1>): void {
-    Resource.patch(
-      this.client,
-      this.path,
-      body,
-      this.headers,
-      params,
-      this.envelope,
-      (err, responseBody, headers, unpacked, statusCode) => {
-        if (callback) {
-          this.handlePage(err, responseBody, headers, unpacked, statusCode, callback);
-        }
-      }
-    );
+  async patch<T1, T2>(params: Record<string, T2>, body: RequestBody | null): Promise<PaginatedResult<T1>> {
+    const result = await Resource.patch<T1>(this.client, this.path, body, this.headers, params, this.envelope, false);
+    return this.handlePage(result);
   }
 
-  handlePage<T>(
-    err: IPartialErrorInfo | null,
-    body: unknown,
-    headers: RequestCallbackHeaders | undefined,
-    unpacked: boolean | undefined,
-    statusCode: number | undefined,
-    callback: PaginatedResultCallback<T>
-  ): void {
-    if (err && returnErrOnly(err, body, this.useHttpPaginatedResponse)) {
+  async handlePage<T>(result: ResourceResult<T>): Promise<PaginatedResult<T>> {
+    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)
       );
-      callback?.(err);
-      return;
+      throw result.err;
     }
 
-    const handleBody = async () => {
-      let items, linkHeader, relParams;
-
-      try {
-        items = await this.bodyHandler(body, headers || {}, 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;
-      }
+    let items, linkHeader, relParams;
 
-      if (headers && (linkHeader = headers['Link'] || headers['link'])) {
-        relParams = parseRelLinks(linkHeader);
-      }
+    try {
+      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 result.err || e;
+    }
 
-      if (this.useHttpPaginatedResponse) {
-        return new HttpPaginatedResponse(this, items, headers || {}, statusCode as number, relParams, err);
-      } else {
-        return new PaginatedResult(this, items, relParams);
-      }
-    };
+    if (result.headers && (linkHeader = result.headers['Link'] || result.headers['link'])) {
+      relParams = parseRelLinks(linkHeader);
+    }
 
-    handleBody()
-      .then((result) => callback(null, result))
-      .catch((err) => callback(err, null));
+    if (this.useHttpPaginatedResponse) {
+      return new HttpPaginatedResponse(
+        this,
+        items,
+        result.headers || {},
+        result.statusCode as number,
+        relParams,
+        result.err
+      );
+    } else {
+      return new PaginatedResult(this, items, relParams);
+    }
   }
 }
 
 export class PaginatedResult<T> {
   resource: PaginatedResource;
   items: T[];
-  first?: (results: PaginatedResultCallback<T>) => void;
-  next?: (results: PaginatedResultCallback<T>) => void;
-  current?: (results: PaginatedResultCallback<T>) => void;
+  first?: () => Promise<PaginatedResult<T>>;
+  next?: () => Promise<PaginatedResult<T> | null>;
+  current?: () => Promise<PaginatedResult<T>>;
   hasNext?: () => boolean;
   isLast?: () => boolean;
 
@@ -195,29 +138,20 @@ export class PaginatedResult<T> {
     const self = this;
     if (relParams) {
       if ('first' in relParams) {
-        this.first = function (callback) {
-          if (!callback) {
-            return Utils.promisify(self, 'first', []);
-          }
-          self.get(relParams.first, callback);
+        this.first = async function () {
+          return self.get(relParams.first);
         };
       }
       if ('current' in relParams) {
-        this.current = function (callback) {
-          if (!callback) {
-            return Utils.promisify(self, 'current', []);
-          }
-          self.get(relParams.current, callback);
+        this.current = async function () {
+          return self.get(relParams.current);
         };
       }
-      this.next = function (callback) {
-        if (!callback) {
-          return Utils.promisify(self, 'next', []);
-        }
+      this.next = async function () {
         if ('next' in relParams) {
-          self.get(relParams.next, callback);
+          return self.get(relParams.next);
         } else {
-          callback(null, null);
+          return null;
         }
       };
 
@@ -232,18 +166,10 @@ export class PaginatedResult<T> {
 
   /* We assume that only the initial request can be a POST, and that accessing
    * the rest of a multipage set of results can always be done with GET */
-  get(params: any, callback: PaginatedResultCallback<T>): void {
+  async get(params: any): Promise<PaginatedResult<T>> {
     const res = this.resource;
-    Resource.get(
-      res.client,
-      res.path,
-      res.headers,
-      params,
-      res.envelope,
-      function (err, body, headers, unpacked, statusCode) {
-        res.handlePage(err, body, headers, unpacked, statusCode, callback);
-      }
-    );
+    const result = await Resource.get<T>(res.client, res.path, res.headers, params, res.envelope, false);
+    return res.handlePage(result);
   }
 }