From 709f66a862788320d96db87521e3def6b5314bf9 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 11 Dec 2023 16:28:43 -0300 Subject: [PATCH 01/44] Update recently-changed tests to use Promise-based API This applies the approach of 43a2d1d to the test changes that were added in `main` subsequent to that commit. --- test/realtime/channel.test.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/test/realtime/channel.test.js b/test/realtime/channel.test.js index 1ed1aab1dc..ac745fb036 100644 --- a/test/realtime/channel.test.js +++ b/test/realtime/channel.test.js @@ -1535,7 +1535,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async var realtime = helper.AblyRealtime(); var channelName = 'attach_returns_state_chnage'; var channel = realtime.channels.get(channelName); - channel.attach(function (err, stateChange) { + whenPromiseSettles(channel.attach(), function (err, stateChange) { if (err) { closeAndFinish(done, realtime, err); return; @@ -1550,7 +1550,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async } // for an already-attached channel, null is returned - channel.attach(function (err, stateChange) { + whenPromiseSettles(channel.attach(), function (err, stateChange) { if (err) { closeAndFinish(done, realtime, err); return; @@ -1571,9 +1571,10 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async var realtime = helper.AblyRealtime(); var channelName = 'subscribe_returns_state_chnage'; var channel = realtime.channels.get(channelName); - channel.subscribe( - function () {}, // message listener - // attach callback + whenPromiseSettles( + channel.subscribe( + function () {} // message listener + ), function (err, stateChange) { if (err) { closeAndFinish(done, realtime, err); @@ -1599,7 +1600,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async var channel = realtime.channels.get(channelName, channelOpts); // attach with rewind but no channel history - hasBacklog should be false - channel.attach(function (err, stateChange) { + whenPromiseSettles(channel.attach(), function (err, stateChange) { if (err) { closeAndFinish(done, realtime, err); return; @@ -1624,12 +1625,12 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async var restChannel = rest.channels.get(channelName); // attach with rewind after publishing - hasBacklog should be true - restChannel.publish('foo', 'bar', function (err) { + whenPromiseSettles(restChannel.publish('foo', 'bar'), function (err) { if (err) { closeAndFinish(done, realtime, err); return; } - rtChannel.attach(function (err, stateChange) { + whenPromiseSettles(rtChannel.attach(), function (err, stateChange) { if (err) { closeAndFinish(done, realtime, err); return; From b6475815346b784e84876cbbdddbb95160376f84 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 11 Dec 2023 16:30:28 -0300 Subject: [PATCH 02/44] Update some missed tests to use Promise-based API This applies the approach of 43a2d1d to some API calls that I missed in that commit. --- test/realtime/channel.test.js | 8 ++++---- test/realtime/crypto.test.js | 9 ++++----- test/realtime/failure.test.js | 6 +++--- test/realtime/message.test.js | 10 ++++------ test/realtime/presence.test.js | 6 +++--- test/realtime/shared/delta_tests.js | 10 +++++----- test/realtime/upgrade.test.js | 16 +++++++--------- 7 files changed, 30 insertions(+), 35 deletions(-) diff --git a/test/realtime/channel.test.js b/test/realtime/channel.test.js index ac745fb036..3ce6edfd0f 100644 --- a/test/realtime/channel.test.js +++ b/test/realtime/channel.test.js @@ -686,7 +686,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async async.series( [ function (cb) { - channel.attach(cb); + whenPromiseSettles(channel.attach(), cb); }, function (cb) { var channelUpdated = false; @@ -1100,13 +1100,13 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async realtime.connection.on('connected', function () { channelByEvent = realtime.channels.get('channelsubscribe1-event'); - channelByEvent.subscribe('event', listenerByEvent, function () { + whenPromiseSettles(channelByEvent.subscribe('event', listenerByEvent), function () { channelByEvent.publish('event', 'data'); channelByListener = realtime.channels.get('channelsubscribe1-listener'); - channelByListener.subscribe(null, listenerNoEvent, function () { + whenPromiseSettles(channelByListener.subscribe(null, listenerNoEvent), function () { channelByListener.publish(null, 'data'); channelAll = realtime.channels.get('channelsubscribe1-all'); - channelAll.subscribe(listenerAllEvents, function () { + whenPromiseSettles(channelAll.subscribe(listenerAllEvents), function () { channelAll.publish(null, 'data'); }); }); diff --git a/test/realtime/crypto.test.js b/test/realtime/crypto.test.js index 3d66366a2f..ded9b31c0a 100644 --- a/test/realtime/crypto.test.js +++ b/test/realtime/crypto.test.js @@ -477,7 +477,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async }); } - channel.attach(function (err) { + whenPromiseSettles(channel.attach(), function (err) { if (err) { closeAndFinish(done, realtime, err); return; @@ -606,9 +606,8 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async return; } var rxChannel = rxRealtime.channels.get(channelName, { cipher: { key: key } }); - rxChannel.subscribe( - 'event0', - function (msg) { + whenPromiseSettles( + rxChannel.subscribe('event0', function (msg) { try { expect(msg.data == messageText).to.be.ok; } catch (err) { @@ -616,7 +615,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async return; } closeAndFinish(done, [txRealtime, rxRealtime]); - }, + }), function () { var txChannel = txRealtime.channels.get(channelName, { cipher: { key: key } }); txChannel.publish('event0', messageText); diff --git a/test/realtime/failure.test.js b/test/realtime/failure.test.js index 23c05a7e3a..ef488fc2f7 100644 --- a/test/realtime/failure.test.js +++ b/test/realtime/failure.test.js @@ -251,7 +251,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async }); }, function (callback) { - failChan.subscribe('event', noop, function (err) { + whenPromiseSettles(failChan.subscribe('event', noop), function (err) { try { expect(err, 'subscribe failed').to.be.ok; expect(err.code).to.equal(channelFailedCode, 'subscribe failure code'); @@ -284,7 +284,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async }); }, function (callback) { - failChan.presence.subscribe('event', noop, function (err) { + whenPromiseSettles(failChan.presence.subscribe('event', noop), function (err) { try { expect(err, 'presence subscribe failed').to.be.ok; expect(err.code).to.equal(channelFailedCode, 'subscribe failure code'); @@ -295,7 +295,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async }); }, function (callback) { - failChan.presence.subscribe('event', noop, function (err) { + whenPromiseSettles(failChan.presence.subscribe('event', noop), function (err) { try { expect(err, 'presence unsubscribe failed').to.be.ok; expect(err.code).to.equal(channelFailedCode, 'subscribe failure code'); diff --git a/test/realtime/message.test.js b/test/realtime/message.test.js index 11f5ff37e4..f80d4e1e8e 100644 --- a/test/realtime/message.test.js +++ b/test/realtime/message.test.js @@ -399,8 +399,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async async.eachSeries( testArguments, function iterator(args, callback) { - args.push(callback); - restChannel.publish.apply(restChannel, args); + whenPromiseSettles(restChannel.publish.apply(restChannel, args), callback); }, function (err) { if (err) { @@ -607,12 +606,11 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async var realtime = helper.AblyRealtime(realtimeOpts); var channel = realtime.channels.get('publish ' + JSON.stringify(realtimeOpts)); /* subscribe to event */ - channel.subscribe( - 'event0', - function () { + whenPromiseSettles( + channel.subscribe('event0', function () { --count; checkFinish(); - }, + }), function () { var dataFn = function () { return 'Hello world at: ' + new Date(); diff --git a/test/realtime/presence.test.js b/test/realtime/presence.test.js index d915eba312..1046e2d43e 100644 --- a/test/realtime/presence.test.js +++ b/test/realtime/presence.test.js @@ -351,8 +351,8 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async var channelName = 'presenceMessageAction'; var clientChannel = clientRealtime.channels.get(channelName); var presence = clientChannel.presence; - presence.subscribe( - function (presenceMessage) { + whenPromiseSettles( + presence.subscribe(function (presenceMessage) { try { expect(presenceMessage.action).to.equal('enter', 'Action should contain string "enter"'); } catch (err) { @@ -360,7 +360,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async return; } closeAndFinish(done, clientRealtime); - }, + }), function onPresenceSubscribe(err) { if (err) { closeAndFinish(done, clientRealtime, err); diff --git a/test/realtime/shared/delta_tests.js b/test/realtime/shared/delta_tests.js index 0de7ebde1f..a70bb9b685 100644 --- a/test/realtime/shared/delta_tests.js +++ b/test/realtime/shared/delta_tests.js @@ -91,7 +91,7 @@ define(['shared_helper', 'async', 'chai'], function (helper, async, chai) { }); async.timesSeries(testData.length, function (i, cb) { - channel.publish(i.toString(), testData[i], cb); + whenPromiseSettles(channel.publish(i.toString(), testData[i]), cb); }); }); @@ -129,7 +129,7 @@ define(['shared_helper', 'async', 'chai'], function (helper, async, chai) { }); async.timesSeries(testData.length, function (i, cb) { - channel.publish(i.toString(), testData[i], cb); + whenPromiseSettles(channel.publish(i.toString(), testData[i]), cb); }); }); @@ -193,7 +193,7 @@ define(['shared_helper', 'async', 'chai'], function (helper, async, chai) { }); async.timesSeries(testData.length, function (i, cb) { - channel.publish(i.toString(), testData[i], cb); + whenPromiseSettles(channel.publish(i.toString(), testData[i]), cb); }); }); @@ -241,7 +241,7 @@ define(['shared_helper', 'async', 'chai'], function (helper, async, chai) { }); async.timesSeries(testData.length, function (i, cb) { - channel.publish(i.toString(), testData[i], cb); + whenPromiseSettles(channel.publish(i.toString(), testData[i]), cb); }); }); @@ -273,7 +273,7 @@ define(['shared_helper', 'async', 'chai'], function (helper, async, chai) { closeAndFinish(done, realtime); }); async.timesSeries(testData.length, function (i, cb) { - channel.publish(i.toString(), testData[i], cb); + whenPromiseSettles(channel.publish(i.toString(), testData[i]), cb); }); }); diff --git a/test/realtime/upgrade.test.js b/test/realtime/upgrade.test.js index 3ae14f839c..fc3779b4b5 100644 --- a/test/realtime/upgrade.test.js +++ b/test/realtime/upgrade.test.js @@ -5,7 +5,7 @@ define(['shared_helper', 'async', 'chai', 'ably'], function (helper, async, chai var rest; var publishIntervalHelper = function (currentMessageNum, channel, dataFn, onPublish) { return function (currentMessageNum) { - channel.publish('event0', dataFn(), function () { + whenPromiseSettles(channel.publish('event0', dataFn()), function () { onPublish(); }); }; @@ -223,12 +223,11 @@ define(['shared_helper', 'async', 'chai', 'ably'], function (helper, async, chai var realtime = helper.AblyRealtime(transportOpts); var channel = realtime.channels.get('upgradepublish0'); /* subscribe to event */ - channel.subscribe( - 'event0', - function () { + whenPromiseSettles( + channel.subscribe('event0', function () { --count; checkFinish(); - }, + }), function () { var dataFn = function () { return 'Hello world at: ' + new Date(); @@ -257,12 +256,11 @@ define(['shared_helper', 'async', 'chai', 'ably'], function (helper, async, chai var realtime = helper.AblyRealtime(transportOpts); var channel = realtime.channels.get('upgradepublish1'); /* subscribe to event */ - channel.subscribe( - 'event0', - function () { + whenPromiseSettles( + channel.subscribe('event0', function () { --count; checkFinish(); - }, + }), function () { var dataFn = function () { return 'Hello world at: ' + new Date(); From 16f3d38aa9a9c00c089c8781027e8651d632c4be Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 8 Jan 2024 15:31:34 +0000 Subject: [PATCH 03/44] Mark Rest.revokeTokens as async Should have done this in ce9a3cf. --- src/common/lib/client/rest.ts | 2 +- test/rest/batch.test.js | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/common/lib/client/rest.ts b/src/common/lib/client/rest.ts index a6a2d3ab88..5340ce09ae 100644 --- a/src/common/lib/client/rest.ts +++ b/src/common/lib/client/rest.ts @@ -270,7 +270,7 @@ export class Rest { ); } - revokeTokens( + async revokeTokens( specifiers: TokenRevocationTargetSpecifier[], options?: TokenRevocationOptions ): Promise { diff --git a/test/rest/batch.test.js b/test/rest/batch.test.js index 9c250c54f4..028ccf1ef6 100644 --- a/test/rest/batch.test.js +++ b/test/rest/batch.test.js @@ -320,14 +320,14 @@ define(['ably', 'shared_helper', 'chai'], function (Ably, helper, chai) { expect(result.results[0].appliesAt).to.be.greaterThan(serverTimeThirtySecondsAfterStartOfTest); }); - it('throws an error when using token auth', function () { + it('throws an error when using token auth', async function () { const rest = helper.AblyRest({ useTokenAuth: true, }); let verifiedError = false; try { - rest.auth.revokeTokens([{ type: 'clientId', value: 'clientId1' }], function () {}); + await rest.auth.revokeTokens([{ type: 'clientId', value: 'clientId1' }], function () {}); } catch (err) { expect(err.statusCode).to.equal(401); expect(err.code).to.equal(40162); From 14ae7c9b8626b15d35e4978bed3022927eea15aa Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 15 Jan 2024 16:35:35 +0000 Subject: [PATCH 04/44] Make Multicaster.call private MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It’s not used outside the class. --- src/common/lib/util/multicaster.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/lib/util/multicaster.ts b/src/common/lib/util/multicaster.ts index 068e3a798b..d09a7c1504 100644 --- a/src/common/lib/util/multicaster.ts +++ b/src/common/lib/util/multicaster.ts @@ -15,7 +15,7 @@ class Multicaster { this.members = (members as Array) || []; } - call(...args: unknown[]): void { + private call(...args: unknown[]): void { for (const member of this.members) { if (member) { try { From 1be691f3f3b70210f996b2bb52a21997c1f75e34 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 15 Jan 2024 16:43:16 +0000 Subject: [PATCH 05/44] Be more explicit about a type A bit more readable, I guess. --- src/common/lib/client/auth.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index b7b9f56002..1342597886 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -1,6 +1,6 @@ import Logger from '../util/logger'; import * as Utils from '../util/utils'; -import Multicaster from '../util/multicaster'; +import Multicaster, { MulticasterInstance } from '../util/multicaster'; import ErrorInfo, { IPartialErrorInfo } from '../types/errorinfo'; import { ErrnoException, RequestCallback, RequestParams } from '../../types/http'; import * as API from '../../../../ably'; @@ -116,7 +116,7 @@ class Auth { client: BaseClient; tokenParams: API.TokenParams; currentTokenRequestId: number | null; - waitingForTokenRequest: ReturnType | null; + waitingForTokenRequest: MulticasterInstance | null; // This initialization is always overwritten and only used to prevent a TypeScript compiler error authOptions: API.AuthOptions = {} as API.AuthOptions; tokenDetails?: API.TokenDetails | null; From d0121257c88bfa2861a82df1179ba33f076ea172 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 11 Dec 2023 11:23:54 -0300 Subject: [PATCH 06/44] Restrict Multicaster callback type Only allow standard (err, result) callbacks, so that we can bridge them to promises. --- src/common/lib/client/auth.ts | 4 +-- src/common/lib/transport/connectionmanager.ts | 4 +-- src/common/lib/util/multicaster.ts | 30 +++++++++---------- 3 files changed, 19 insertions(+), 19 deletions(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index 1342597886..74b49c3a04 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -116,7 +116,7 @@ class Auth { client: BaseClient; tokenParams: API.TokenParams; currentTokenRequestId: number | null; - waitingForTokenRequest: MulticasterInstance | null; + waitingForTokenRequest: MulticasterInstance | null; // This initialization is always overwritten and only used to prevent a TypeScript compiler error authOptions: API.AuthOptions = {} as API.AuthOptions; tokenDetails?: API.TokenDetails | null; @@ -959,7 +959,7 @@ class Auth { /* Request a new token */ const tokenRequestId = (this.currentTokenRequestId = getTokenRequestId()); - this.requestToken(this.tokenParams, this.authOptions, (err: Function, tokenResponse?: API.TokenDetails) => { + this.requestToken(this.tokenParams, this.authOptions, (err: ErrorInfo | null, tokenResponse?: API.TokenDetails) => { if ((this.currentTokenRequestId as number) > tokenRequestId) { Logger.logAction( Logger.LOG_MINOR, diff --git a/src/common/lib/transport/connectionmanager.ts b/src/common/lib/transport/connectionmanager.ts index 99b6581016..04c8bc798b 100644 --- a/src/common/lib/transport/connectionmanager.ts +++ b/src/common/lib/transport/connectionmanager.ts @@ -1951,10 +1951,10 @@ class ConnectionManager extends EventEmitter { * the dup, they'll be lost */ if (lastQueued && !lastQueued.sendAttempted && bundleWith(lastQueued.message, msg, maxSize)) { if (!lastQueued.merged) { - lastQueued.callback = Multicaster.create([lastQueued.callback as any]); + lastQueued.callback = Multicaster.create([lastQueued.callback]); lastQueued.merged = true; } - (lastQueued.callback as MulticasterInstance).push(callback as any); + (lastQueued.callback as MulticasterInstance).push(callback); } else { this.queuedMessages.push(new PendingMessage(msg, callback)); } diff --git a/src/common/lib/util/multicaster.ts b/src/common/lib/util/multicaster.ts index d09a7c1504..81f1fb6344 100644 --- a/src/common/lib/util/multicaster.ts +++ b/src/common/lib/util/multicaster.ts @@ -1,25 +1,25 @@ +import { StandardCallback } from 'common/types/utils'; +import ErrorInfo from 'common/lib/types/errorinfo'; import Logger from './logger'; -type AnyFunction = (...args: any[]) => unknown; - -export interface MulticasterInstance extends Function { - (...args: unknown[]): void; - push: (fn: AnyFunction) => void; +export interface MulticasterInstance extends Function { + (err?: ErrorInfo | null, result?: T): void; + push: (fn: StandardCallback) => void; } -class Multicaster { - members: Array; +class Multicaster { + members: Array>; // Private constructor; use static Multicaster.create instead - private constructor(members?: Array) { - this.members = (members as Array) || []; + private constructor(members?: Array | undefined>) { + this.members = (members as Array>) || []; } - private call(...args: unknown[]): void { + private call(err?: ErrorInfo | null, result?: T): void { for (const member of this.members) { if (member) { try { - member(...args); + member(err, result); } catch (e) { Logger.logAction( Logger.LOG_ERROR, @@ -31,14 +31,14 @@ class Multicaster { } } - push(...args: Array): void { + push(...args: Array>): void { this.members.push(...args); } - static create(members?: Array): MulticasterInstance { + static create(members?: Array | undefined>): MulticasterInstance { const instance = new Multicaster(members); - return Object.assign((...args: unknown[]) => instance.call(...args), { - push: (fn: AnyFunction) => instance.push(fn), + return Object.assign((err?: ErrorInfo | null, result?: T) => instance.call(err, result), { + push: (fn: StandardCallback) => instance.push(fn), }); } } From dfaa6ba865f8921c200a484ce6e8e7c0fad69a76 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 7 Dec 2023 10:21:10 -0300 Subject: [PATCH 07/44] Add a method for bridging Multicaster with promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Preparation for using promises internally in the codebase. Part of #1535; will remove the functional interface to Multicaster if/when it’s no longer needed. --- src/common/lib/util/multicaster.ts | 31 ++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/common/lib/util/multicaster.ts b/src/common/lib/util/multicaster.ts index 81f1fb6344..eb7ba19b28 100644 --- a/src/common/lib/util/multicaster.ts +++ b/src/common/lib/util/multicaster.ts @@ -5,6 +5,18 @@ import Logger from './logger'; export interface MulticasterInstance extends Function { (err?: ErrorInfo | null, result?: T): void; push: (fn: StandardCallback) => void; + /** + * Creates a promise that will be resolved or rejected when this instance is called. + */ + createPromise: () => Promise; + /** + * Syntatic sugar for when working in a context that uses promises; equivalent to calling as a function with arguments (null, result). + */ + resolveAll(result: T): void; + /** + * Syntatic sugar for when working in a context that uses promises; equivalent to calling as a function with arguments (err). + */ + rejectAll(err: ErrorInfo): void; } class Multicaster { @@ -35,10 +47,29 @@ class Multicaster { this.members.push(...args); } + createPromise(): Promise { + return new Promise((resolve, reject) => { + this.push((err, result) => { + err ? reject(err) : resolve(result!); + }); + }); + } + + resolveAll(result: T) { + this.call(null, result); + } + + rejectAll(err: ErrorInfo) { + this.call(err); + } + static create(members?: Array | undefined>): MulticasterInstance { const instance = new Multicaster(members); return Object.assign((err?: ErrorInfo | null, result?: T) => instance.call(err, result), { push: (fn: StandardCallback) => instance.push(fn), + createPromise: () => instance.createPromise(), + resolveAll: (result: T) => instance.resolveAll(result), + rejectAll: (err: ErrorInfo) => instance.rejectAll(err), }); } } From cd6c2d8ea601134b8afdcf10e2d8e9629245153d Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 11 Dec 2023 11:37:21 -0300 Subject: [PATCH 08/44] Add whenPromiseSettles function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Copied from the tests. We’ll use it for bridging between promise-using methods and callback-using methods as we start changing the codebase to use promises internally. --- src/common/lib/util/utils.ts | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/common/lib/util/utils.ts b/src/common/lib/util/utils.ts index 9388ad2db2..4fea24cc8e 100644 --- a/src/common/lib/util/utils.ts +++ b/src/common/lib/util/utils.ts @@ -452,6 +452,22 @@ export function promisify(ob: Record, fnName: string, args: IArg }); } +/** + * Uses a callback to communicate the result of a `Promise`. The first argument passed to the callback will be either an error (when the promise is rejected) or `null` (when the promise is fulfilled). In the case where the promise is fulfilled, the resulting value will be passed to the callback as a second argument. + */ +export function whenPromiseSettles( + promise: Promise, + callback?: (err: E | null, result?: T) => void +) { + promise + .then((result) => { + callback?.(null, result); + }) + .catch((err) => { + callback?.(err); + }); +} + export function decodeBody(body: unknown, MsgPack: MsgPack | null, format?: Format | null): T { if (format == 'msgpack') { if (!MsgPack) { From 4aa7494f494458e574db8570c5f7bdeb7be1a6e8 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 7 Dec 2023 14:40:25 -0300 Subject: [PATCH 09/44] Fix type of ChannelSubscriptions.removeWhere callback --- src/common/lib/client/push.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/lib/client/push.ts b/src/common/lib/client/push.ts index 3d0b7b9f44..6ee772ec44 100644 --- a/src/common/lib/client/push.ts +++ b/src/common/lib/client/push.ts @@ -285,7 +285,7 @@ class ChannelSubscriptions { }).get(params, callback); } - removeWhere(params: any, callback: PaginatedResultCallback) { + removeWhere(params: any, callback: ErrCallback) { const client = this.client, format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, headers = Defaults.defaultGetHeaders(client.options, { format }); From 3e06ea9baf85d77581f00872b95914d595d913ed Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 7 Dec 2023 14:47:02 -0300 Subject: [PATCH 10/44] Fix type of ChannelSubscriptions.save callback --- src/common/lib/client/push.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/common/lib/client/push.ts b/src/common/lib/client/push.ts index 6ee772ec44..5344718414 100644 --- a/src/common/lib/client/push.ts +++ b/src/common/lib/client/push.ts @@ -223,7 +223,7 @@ class ChannelSubscriptions { this.client = client; } - save(subscription: Record, callback: PaginatedResultCallback) { + save(subscription: Record, callback: StandardCallback) { const client = this.client; const body = PushChannelSubscription.fromValues(subscription); const format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, @@ -249,12 +249,13 @@ class ChannelSubscriptions { function (err, body, headers, unpacked) { callback( err, - !err && - PushChannelSubscription.fromResponseBody( - body as Record, - client._MsgPack, - unpacked ? undefined : format - ) + !err + ? (PushChannelSubscription.fromResponseBody( + body as Record, + client._MsgPack, + unpacked ? undefined : format + ) as PushChannelSubscription) + : undefined ); } ); From 0eb0018d3bafac5a2095a6fd57dbd1c2b76c6427 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 7 Dec 2023 15:13:00 -0300 Subject: [PATCH 11/44] Fix return type of Connection.ping --- src/common/lib/client/connection.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/lib/client/connection.ts b/src/common/lib/client/connection.ts index 47170b9430..1447edde04 100644 --- a/src/common/lib/client/connection.ts +++ b/src/common/lib/client/connection.ts @@ -53,7 +53,7 @@ class Connection extends EventEmitter { this.connectionManager.requestState({ state: 'connecting' }); } - ping(callback: Function): Promise | void { + ping(callback: Function): Promise | void { Logger.logAction(Logger.LOG_MINOR, 'Connection.ping()', ''); if (!callback) { return Utils.promisify(this, 'ping', arguments); From 35970d6536eed92202ce407db23dc5a5dace21ea Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 7 Dec 2023 15:42:53 -0300 Subject: [PATCH 12/44] Fix return type of RealtimePresence.get --- src/common/lib/client/realtimepresence.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/lib/client/realtimepresence.ts b/src/common/lib/client/realtimepresence.ts index 51f460a618..1bf1d0a9d6 100644 --- a/src/common/lib/client/realtimepresence.ts +++ b/src/common/lib/client/realtimepresence.ts @@ -251,7 +251,7 @@ class RealtimePresence extends EventEmitter { this: RealtimePresence, params: RealtimePresenceParams, callback: StandardCallback - ): void | Promise { + ): void | Promise { const args = Array.prototype.slice.call(arguments); if (args.length == 1 && typeof args[0] == 'function') args.unshift(null); From 2c560300fa12ebc66912962ac32516a7bebab04c Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 7 Dec 2023 16:41:33 -0300 Subject: [PATCH 13/44] Fix return type of Auth.authorize --- src/common/lib/client/auth.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index 74b49c3a04..a458111d74 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -251,7 +251,7 @@ class Auth { tokenParams: Record | Function | null, authOptions?: API.AuthOptions | null | Function, callback?: Function - ): void | Promise { + ): void | Promise { let _authOptions: API.AuthOptions | null; /* shuffle and normalise arguments as necessary */ if (typeof tokenParams == 'function' && !callback) { From 501dd7489f7d3550ae0850a3d1d65528545fa610 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 7 Dec 2023 16:44:22 -0300 Subject: [PATCH 14/44] Fix return type of Auth.requestToken --- src/common/lib/client/auth.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index a458111d74..155a767281 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -408,7 +408,7 @@ class Auth { tokenParams: API.TokenParams | StandardCallback | null, authOptions?: any | StandardCallback, callback?: StandardCallback - ): void | Promise { + ): void | Promise { /* shuffle and normalise arguments as necessary */ if (typeof tokenParams == 'function' && !callback) { callback = tokenParams; From 66e025b43475afdfdf53c13345dfc9d3bca3e0f9 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 7 Dec 2023 17:04:40 -0300 Subject: [PATCH 15/44] Fix return type of RealtimeChannel.subscribe --- src/common/lib/client/realtimechannel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/lib/client/realtimechannel.ts b/src/common/lib/client/realtimechannel.ts index eb2b94068a..806ffdcb0a 100644 --- a/src/common/lib/client/realtimechannel.ts +++ b/src/common/lib/client/realtimechannel.ts @@ -436,7 +436,7 @@ class RealtimeChannel extends EventEmitter { this.sendMessage(msg, callback || noop); } - subscribe(...args: unknown[] /* [event], listener, [callback] */): void | Promise { + subscribe(...args: unknown[] /* [event], listener, [callback] */): void | Promise { const [event, listener, callback] = RealtimeChannel.processListenerArgs(args); if (!callback) { From 602ceaf6ade8e639737a6b3bd231501234061dc5 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 16:32:51 -0300 Subject: [PATCH 16/44] Fix return type of RealtimeChannel.history --- src/common/lib/client/realtimechannel.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/common/lib/client/realtimechannel.ts b/src/common/lib/client/realtimechannel.ts index 806ffdcb0a..34ba69e7ad 100644 --- a/src/common/lib/client/realtimechannel.ts +++ b/src/common/lib/client/realtimechannel.ts @@ -27,6 +27,7 @@ import { ErrCallback, PaginatedResultCallback, StandardCallback } from '../../ty import BaseRealtime from './baserealtime'; import { ChannelOptions } from '../../types/channel'; import { normaliseChannelOptions } from '../util/defaults'; +import { PaginatedResult } from './paginatedresource'; interface RealtimeHistoryParams { start?: number; @@ -887,7 +888,7 @@ class RealtimeChannel extends EventEmitter { this: RealtimeChannel, params: RealtimeHistoryParams | null, callback: PaginatedResultCallback - ): void | Promise> { + ): void | Promise> { Logger.logAction(Logger.LOG_MICRO, 'RealtimeChannel.history()', 'channel = ' + this.name); /* params and callback are optional; see if params contains the callback */ if (callback === undefined) { From 336695a4b918d7cdcc763b4f49f4e3e1171cd551 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 7 Dec 2023 17:10:33 -0300 Subject: [PATCH 17/44] Remove useless return The return type of `restMixin.history(...)` is `void`. --- src/common/lib/client/realtimechannel.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/common/lib/client/realtimechannel.ts b/src/common/lib/client/realtimechannel.ts index 34ba69e7ad..fba5065ed6 100644 --- a/src/common/lib/client/realtimechannel.ts +++ b/src/common/lib/client/realtimechannel.ts @@ -922,7 +922,7 @@ class RealtimeChannel extends EventEmitter { params.from_serial = this.properties.attachSerial; } - return restMixin.history(this, params, callback); + restMixin.history(this, params, callback); } as any; whenState = ((state: string, listener: ErrCallback) => { From 5e95872ad2e9c12806d7565b355b0ab3ada855da Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 11:52:23 -0300 Subject: [PATCH 18/44] Add Promise overload signatures to some methods This adds Promise overload signatures to all of the methods which: 1. offer dual callback / promise operation, and 2. are called internally in the codebase This will allow us to switch these internal calls to use the promise-based versions of these methods. --- src/common/lib/client/auth.ts | 18 +++++++++++++----- src/common/lib/client/baseclient.ts | 3 +++ src/common/lib/client/realtimechannel.ts | 4 +++- src/common/lib/client/realtimepresence.ts | 19 ++++++++++++++++--- 4 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index 155a767281..1088649d64 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -164,6 +164,7 @@ class Auth { * @param callback (err, tokenDetails) */ authorize(callback: Function): void; + authorize(): Promise; /** * Instructs the library to get a token immediately and ensures Token Auth @@ -190,6 +191,7 @@ class Auth { * @param callback (err, tokenDetails) */ authorize(tokenParams: API.TokenParams | null, callback: Function): void; + authorize(tokenParams: API.TokenParams | null): Promise; /** * Instructs the library to get a token immediately and ensures Token Auth @@ -246,9 +248,10 @@ class Auth { * @param callback (err, tokenDetails) */ authorize(tokenParams: API.TokenParams | null, authOptions: API.AuthOptions | null, callback: Function): void; + authorize(tokenParams: API.TokenParams | null, authOptions: API.AuthOptions | null): Promise; authorize( - tokenParams: Record | Function | null, + tokenParams?: Record | Function | null, authOptions?: API.AuthOptions | null | Function, callback?: Function ): void | Promise { @@ -329,6 +332,7 @@ class Auth { * @param callback (err, tokenDetails) */ requestToken(callback: StandardCallback): void; + requestToken(): Promise; /** * Request an access token @@ -351,6 +355,7 @@ class Auth { * @param callback (err, tokenDetails) */ requestToken(tokenParams: API.TokenParams | null, callback: StandardCallback): void; + requestToken(tokenParams: API.TokenParams | null): Promise; /** * Request an access token @@ -403,9 +408,10 @@ class Auth { authOptions: API.AuthOptions, callback: StandardCallback ): void; + requestToken(tokenParams: API.TokenParams | null, authOptions: API.AuthOptions): Promise; requestToken( - tokenParams: API.TokenParams | StandardCallback | null, + tokenParams?: API.TokenParams | StandardCallback | null, authOptions?: any | StandardCallback, callback?: StandardCallback ): void | Promise { @@ -698,6 +704,8 @@ class Auth { }); } + createTokenRequest(tokenParams: API.TokenParams | null, authOptions: any, callback: Function): void; + createTokenRequest(tokenParams: API.TokenParams | null, authOptions: any): Promise; /** * Create and sign a token request based on the given options. * NOTE this can only be used when the key value is available locally. @@ -733,7 +741,7 @@ class Auth { * * @param callback */ - createTokenRequest(tokenParams: API.TokenParams | null, authOptions: any, callback: Function) { + createTokenRequest(tokenParams: API.TokenParams | null, authOptions: any, callback?: Function) { /* shuffle and normalise arguments as necessary */ if (typeof tokenParams == 'function' && !callback) { callback = tokenParams; @@ -785,7 +793,7 @@ class Auth { } this.getTimestamp(authOptions && authOptions.queryTime, function (err?: ErrorInfo | null, time?: number) { if (err) { - callback(err); + callback!(err); return; } request.timestamp = time; @@ -811,7 +819,7 @@ class Auth { request.mac = request.mac || hmac(signText, keySecret); Logger.logAction(Logger.LOG_MINOR, 'Auth.getTokenRequest()', 'generated signed request'); - callback(null, request); + callback!(null, request); }); } diff --git a/src/common/lib/client/baseclient.ts b/src/common/lib/client/baseclient.ts index bee97b89e4..b5c982ac75 100644 --- a/src/common/lib/client/baseclient.ts +++ b/src/common/lib/client/baseclient.ts @@ -136,6 +136,9 @@ class BaseClient { return this.rest.stats(params, callback); } + time(params: RequestParams, callback: StandardCallback): void; + time(callback: StandardCallback): void; + time(params?: RequestParams): Promise; time(params?: RequestParams | StandardCallback, callback?: StandardCallback): Promise | void { return this.rest.time(params, callback); } diff --git a/src/common/lib/client/realtimechannel.ts b/src/common/lib/client/realtimechannel.ts index fba5065ed6..3bd0ccf0fd 100644 --- a/src/common/lib/client/realtimechannel.ts +++ b/src/common/lib/client/realtimechannel.ts @@ -305,7 +305,9 @@ class RealtimeChannel extends EventEmitter { } } - attach(callback?: StandardCallback): void | Promise { + attach(): Promise; + attach(callback: StandardCallback): void; + attach(callback?: StandardCallback): void | Promise { if (!callback) { return Utils.promisify(this, 'attach', arguments); } diff --git a/src/common/lib/client/realtimepresence.ts b/src/common/lib/client/realtimepresence.ts index 1bf1d0a9d6..196009365f 100644 --- a/src/common/lib/client/realtimepresence.ts +++ b/src/common/lib/client/realtimepresence.ts @@ -123,12 +123,25 @@ class RealtimePresence extends EventEmitter { return this._enterOrUpdateClient(undefined, clientId, data, 'update', callback); } + _enterOrUpdateClient( + id: string | undefined, + clientId: string | undefined, + data: unknown, + action: string + ): Promise; _enterOrUpdateClient( id: string | undefined, clientId: string | undefined, data: unknown, action: string, callback: ErrCallback + ): void; + _enterOrUpdateClient( + id: string | undefined, + clientId: string | undefined, + data: unknown, + action: string, + callback?: ErrCallback ): void | Promise { if (!callback) { if (typeof data === 'function') { @@ -162,7 +175,7 @@ class RealtimePresence extends EventEmitter { encodePresenceMessage(presence, channel.channelOptions as CipherOptions, (err: IPartialErrorInfo) => { if (err) { - callback(err); + callback!(err); return; } switch (channel.state) { @@ -176,7 +189,7 @@ class RealtimePresence extends EventEmitter { case 'attaching': this.pendingPresence.push({ presence: presence, - callback: callback, + callback: callback!, }); break; default: @@ -185,7 +198,7 @@ class RealtimePresence extends EventEmitter { 90001 ); err.code = 90001; - callback(err); + callback!(err); } }); } From df28f8dafc9e5d8ab7d20e132bc50f891c2aa38e Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 11:09:24 -0300 Subject: [PATCH 19/44] Use the promise-based version of callback/promise methods Whenever we call a method that offers dual callback/promise operation, we now use the promise-based version. This in preparation for removing the callback functionality from these methods. --- src/common/lib/client/auth.ts | 43 ++++++++++++----------- src/common/lib/client/realtimepresence.ts | 4 +-- src/common/lib/client/resource.ts | 2 +- src/common/lib/transport/transport.ts | 2 +- test/realtime/auth.test.js | 4 +-- 5 files changed, 29 insertions(+), 26 deletions(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index 1088649d64..b0aa44e3a1 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -554,8 +554,8 @@ class Auth { }; } else if (authOptions.key) { Logger.logAction(Logger.LOG_MINOR, 'Auth.requestToken()', 'using token auth with client-side signing'); - tokenRequestCallback = (params: any, cb: Function) => { - this.createTokenRequest(params, authOptions, cb); + tokenRequestCallback = (params: any, cb: StandardCallback) => { + Utils.whenPromiseSettles(this.createTokenRequest(params, authOptions), cb); }; } else { const msg = @@ -871,7 +871,7 @@ class Auth { */ getTimestamp(queryTime: boolean, callback: StandardCallback): void { if (!this.isTimeOffsetSet() && (queryTime || this.authOptions.queryTime)) { - this.client.time(callback); + Utils.whenPromiseSettles(this.client.time(), callback); } else { callback(null, this.getTimestampUsingOffset()); } @@ -967,24 +967,27 @@ class Auth { /* Request a new token */ const tokenRequestId = (this.currentTokenRequestId = getTokenRequestId()); - this.requestToken(this.tokenParams, this.authOptions, (err: ErrorInfo | null, tokenResponse?: API.TokenDetails) => { - if ((this.currentTokenRequestId as number) > tokenRequestId) { - Logger.logAction( - Logger.LOG_MINOR, - 'Auth._ensureValidAuthCredentials()', - 'Discarding token request response; overtaken by newer one' - ); - return; - } - this.currentTokenRequestId = null; - const callbacks = this.waitingForTokenRequest || noop; - this.waitingForTokenRequest = null; - if (err) { - callbacks(err); - return; + Utils.whenPromiseSettles( + this.requestToken(this.tokenParams, this.authOptions), + (err: ErrorInfo | null, tokenResponse?: API.TokenDetails) => { + if ((this.currentTokenRequestId as number) > tokenRequestId) { + Logger.logAction( + Logger.LOG_MINOR, + 'Auth._ensureValidAuthCredentials()', + 'Discarding token request response; overtaken by newer one' + ); + return; + } + this.currentTokenRequestId = null; + const callbacks = this.waitingForTokenRequest || noop; + this.waitingForTokenRequest = null; + if (err) { + callbacks(err); + return; + } + callbacks(null, (this.tokenDetails = tokenResponse)); } - callbacks(null, (this.tokenDetails = tokenResponse)); - }); + ); } /* User-set: check types, '*' is disallowed, throw any errors */ diff --git a/src/common/lib/client/realtimepresence.ts b/src/common/lib/client/realtimepresence.ts index 196009365f..b0010d74b5 100644 --- a/src/common/lib/client/realtimepresence.ts +++ b/src/common/lib/client/realtimepresence.ts @@ -54,7 +54,7 @@ function waitAttached(channel: RealtimeChannel, callback: ErrCallback, action: ( case 'detached': case 'detaching': case 'attaching': - channel.attach(function (err: Error) { + Utils.whenPromiseSettles(channel.attach(), function (err: Error | null) { if (err) callback(err); else action(); }); @@ -495,7 +495,7 @@ class RealtimePresence extends EventEmitter { ); // RTP17g: Send ENTER containing the member id, clientId and data // attributes. - this._enterOrUpdateClient(entry.id, entry.clientId, entry.data, 'enter', reenterCb); + Utils.whenPromiseSettles(this._enterOrUpdateClient(entry.id, entry.clientId, entry.data, 'enter'), reenterCb); } } diff --git a/src/common/lib/client/resource.ts b/src/common/lib/client/resource.ts index bfb860fd65..69de333a3b 100644 --- a/src/common/lib/client/resource.ts +++ b/src/common/lib/client/resource.ts @@ -233,7 +233,7 @@ class Resource { client.http.do(method, path, headers, body, params, function (err, res, resHeaders, 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) { + Utils.whenPromiseSettles(client.auth.authorize(null, null), function (err: ErrorInfo | null) { if (err) { callback(err); return; diff --git a/src/common/lib/transport/transport.ts b/src/common/lib/transport/transport.ts index 0cdfc2b2a8..5153cd63bf 100644 --- a/src/common/lib/transport/transport.ts +++ b/src/common/lib/transport/transport.ts @@ -162,7 +162,7 @@ abstract class Transport extends EventEmitter { // Ignored. break; case actions.AUTH: - this.auth.authorize(function (err: ErrorInfo) { + Utils.whenPromiseSettles(this.auth.authorize(), function (err: ErrorInfo | null) { if (err) { Logger.logAction( Logger.LOG_ERROR, diff --git a/test/realtime/auth.test.js b/test/realtime/auth.test.js index 166ba4adc5..115173d70a 100644 --- a/test/realtime/auth.test.js +++ b/test/realtime/auth.test.js @@ -688,9 +688,9 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async originalTime = rest.time; /* stub time */ - rest.time = function (callback) { + rest.time = async function () { timeRequestCount += 1; - originalTime.call(rest, callback); + return originalTime.call(rest); }; try { From 7c465d671bea2d8c2cff3d508b49860f471ee8e0 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Thu, 7 Dec 2023 14:39:36 -0300 Subject: [PATCH 20/44] Remove dual callback/promise operation Methods that previously offered the ability to use callbacks or promises now only offer promises. --- src/common/lib/client/auth.ts | 422 +++++++++------------ src/common/lib/client/baseclient.ts | 24 +- src/common/lib/client/connection.ts | 10 +- src/common/lib/client/paginatedresource.ts | 35 +- src/common/lib/client/push.ts | 312 +++++++-------- src/common/lib/client/realtimechannel.ts | 236 +++++------- src/common/lib/client/realtimepresence.ts | 276 ++++++-------- src/common/lib/client/rest.ts | 210 ++++------ src/common/lib/client/restchannel.ts | 98 ++--- src/common/lib/client/restchannelmixin.ts | 19 +- src/common/lib/client/restpresence.ts | 49 +-- src/common/lib/client/restpresencemixin.ts | 44 +-- src/common/lib/util/utils.ts | 8 - 13 files changed, 726 insertions(+), 1017 deletions(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index b0aa44e3a1..0ca538bed7 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -160,11 +160,8 @@ class Auth { * Instructs the library to get a token immediately and ensures Token Auth * is used for all future requests, storing the tokenParams and authOptions * given as the new defaults for subsequent use. - * - * @param callback (err, tokenDetails) */ - authorize(callback: Function): void; - authorize(): Promise; + async authorize(): Promise; /** * Instructs the library to get a token immediately and ensures Token Auth @@ -187,11 +184,8 @@ class Auth { * * - timestamp: (optional) the time in ms since the epoch. If none is specified, * the system will be queried for a time value to use. - * - * @param callback (err, tokenDetails) */ - authorize(tokenParams: API.TokenParams | null, callback: Function): void; - authorize(tokenParams: API.TokenParams | null): Promise; + async authorize(tokenParams: API.TokenParams | null): Promise; /** * Instructs the library to get a token immediately and ensures Token Auth @@ -244,65 +238,51 @@ class Auth { * * - requestHeaders (optional, unsupported, for testing only) extra headers to add to the * requestToken request - * - * @param callback (err, tokenDetails) */ - authorize(tokenParams: API.TokenParams | null, authOptions: API.AuthOptions | null, callback: Function): void; - authorize(tokenParams: API.TokenParams | null, authOptions: API.AuthOptions | null): Promise; - - authorize( - tokenParams?: Record | Function | null, - authOptions?: API.AuthOptions | null | Function, - callback?: Function - ): void | Promise { - let _authOptions: API.AuthOptions | null; - /* shuffle and normalise arguments as necessary */ - if (typeof tokenParams == 'function' && !callback) { - callback = tokenParams; - _authOptions = tokenParams = null; - } else if (typeof authOptions == 'function' && !callback) { - callback = authOptions; - _authOptions = null; - } else { - _authOptions = authOptions as API.AuthOptions; - } - if (!callback) { - return Utils.promisify(this, 'authorize', arguments); - } + async authorize(tokenParams: API.TokenParams | null, authOptions: API.AuthOptions | null): Promise; + async authorize( + tokenParams?: Record | null, + authOptions?: API.AuthOptions | null + ): Promise { /* RSA10a: authorize() call implies token auth. If a key is passed it, we * just check if it doesn't clash and assume we're generating a token from it */ - if (_authOptions && _authOptions.key && this.authOptions.key !== _authOptions.key) { + if (authOptions && authOptions.key && this.authOptions.key !== authOptions.key) { throw new ErrorInfo('Unable to update auth options with incompatible key', 40102, 401); } - this._forceNewToken( - tokenParams as API.TokenParams, - _authOptions, - (err: ErrorInfo, tokenDetails: API.TokenDetails) => { - if (err) { - if ((this.client as BaseRealtime).connection && err.statusCode === HttpStatusCodes.Forbidden) { - /* Per RSA4d & RSA4d1, if the auth server explicitly repudiates our right to - * stay connecticed by returning a 403, we actively disconnect the connection - * even though we may well still have time left in the old token. */ - (this.client as BaseRealtime).connection.connectionManager.actOnErrorFromAuthorize(err); + return new Promise((resolve, reject) => { + this._forceNewToken( + tokenParams ?? null, + authOptions ?? null, + (err: ErrorInfo, tokenDetails: API.TokenDetails) => { + if (err) { + if ((this.client as BaseRealtime).connection && err.statusCode === HttpStatusCodes.Forbidden) { + /* Per RSA4d & RSA4d1, if the auth server explicitly repudiates our right to + * stay connecticed by returning a 403, we actively disconnect the connection + * even though we may well still have time left in the old token. */ + (this.client as BaseRealtime).connection.connectionManager.actOnErrorFromAuthorize(err); + } + reject(err); + return; } - callback?.(err); - return; - } - /* RTC8 - * - When authorize called by an end user and have a realtime connection, - * don't call back till new token has taken effect. - * - Use this.client.connection as a proxy for (this.client instanceof BaseRealtime), - * which doesn't work in node as BaseRealtime isn't part of the vm context for Rest clients */ - if (isRealtime(this.client)) { - this.client.connection.connectionManager.onAuthUpdated(tokenDetails, callback || noop); - } else { - callback?.(null, tokenDetails); + /* RTC8 + * - When authorize called by an end user and have a realtime connection, + * don't call back till new token has taken effect. + * - Use this.client.connection as a proxy for (this.client instanceof BaseRealtime), + * which doesn't work in node as BaseRealtime isn't part of the vm context for Rest clients */ + if (isRealtime(this.client)) { + this.client.connection.connectionManager.onAuthUpdated( + tokenDetails, + (err: unknown, tokenDetails?: API.TokenDetails) => (err ? reject(err) : resolve(tokenDetails!)) + ); + } else { + resolve(tokenDetails); + } } - } - ); + ); + }); } /* For internal use, eg by connectionManager - useful when want to call back @@ -329,10 +309,8 @@ class Auth { /** * Request an access token - * @param callback (err, tokenDetails) */ - requestToken(callback: StandardCallback): void; - requestToken(): Promise; + async requestToken(): Promise; /** * Request an access token @@ -351,11 +329,8 @@ class Auth { * * - timestamp: (optional) the time in ms since the epoch. If none is specified, * the system will be queried for a time value to use. - * - * @param callback (err, tokenDetails) */ - requestToken(tokenParams: API.TokenParams | null, callback: StandardCallback): void; - requestToken(tokenParams: API.TokenParams | null): Promise; + async requestToken(tokenParams: API.TokenParams | null): Promise; /** * Request an access token @@ -400,41 +375,17 @@ class Auth { * * - requestHeaders (optional, unsupported, for testing only) extra headers to add to the * requestToken request - * - * @param callback (err, tokenDetails) */ - requestToken( - tokenParams: API.TokenParams | null, - authOptions: API.AuthOptions, - callback: StandardCallback - ): void; - requestToken(tokenParams: API.TokenParams | null, authOptions: API.AuthOptions): Promise; - - requestToken( - tokenParams?: API.TokenParams | StandardCallback | null, - authOptions?: any | StandardCallback, - callback?: StandardCallback - ): void | Promise { - /* shuffle and normalise arguments as necessary */ - if (typeof tokenParams == 'function' && !callback) { - callback = tokenParams; - authOptions = tokenParams = null; - } else if (typeof authOptions == 'function' && !callback) { - callback = authOptions; - authOptions = null; - } - if (!callback) { - return Utils.promisify(this, 'requestToken', arguments); - } + async requestToken(tokenParams: API.TokenParams | null, authOptions: API.AuthOptions): Promise; + async requestToken(tokenParams?: API.TokenParams | null, authOptions?: any): Promise { /* RSA8e: if authOptions passed in, they're used instead of stored, don't merge them */ authOptions = authOptions || this.authOptions; tokenParams = tokenParams || Utils.copy(this.tokenParams); - const _callback = callback || noop; /* first set up whatever callback will be used to get signed * token requests */ - let tokenRequestCallback, + let tokenRequestCallback: any, client = this.client; if (authOptions.authCallback) { @@ -565,8 +516,7 @@ class Auth { 'Auth()', 'library initialized with a token literal without any way to renew the token when it expires (no authUrl, authCallback, or key). See https://help.ably.io/error/40171 for help' ); - _callback(new ErrorInfo(msg, 40171, 403)); - return; + throw new ErrorInfo(msg, 40171, 403); } /* normalise token params */ @@ -597,115 +547,118 @@ class Auth { ); }; - let tokenRequestCallbackTimeoutExpired = false, - timeoutLength = this.client.options.timeouts.realtimeRequestTimeout, - tokenRequestCallbackTimeout = setTimeout(function () { - tokenRequestCallbackTimeoutExpired = true; - const msg = 'Token request callback timed out after ' + timeoutLength / 1000 + ' seconds'; - Logger.logAction(Logger.LOG_ERROR, 'Auth.requestToken()', msg); - _callback(new ErrorInfo(msg, 40170, 401)); - }, timeoutLength); + return new Promise((resolve, reject) => { + let tokenRequestCallbackTimeoutExpired = false, + timeoutLength = this.client.options.timeouts.realtimeRequestTimeout, + tokenRequestCallbackTimeout = setTimeout(function () { + tokenRequestCallbackTimeoutExpired = true; + const msg = 'Token request callback timed out after ' + timeoutLength / 1000 + ' seconds'; + Logger.logAction(Logger.LOG_ERROR, 'Auth.requestToken()', msg); + reject(new ErrorInfo(msg, 40170, 401)); + }, timeoutLength); - tokenRequestCallback(tokenParams, function (err: ErrorInfo, tokenRequestOrDetails: any, contentType: string) { - if (tokenRequestCallbackTimeoutExpired) return; - clearTimeout(tokenRequestCallbackTimeout); + tokenRequestCallback(tokenParams, function (err: ErrorInfo, tokenRequestOrDetails: any, contentType: string) { + if (tokenRequestCallbackTimeoutExpired) return; + clearTimeout(tokenRequestCallbackTimeout); - if (err) { - Logger.logAction( - Logger.LOG_ERROR, - 'Auth.requestToken()', - 'token request signing call returned error; err = ' + Utils.inspectError(err) - ); - _callback(normaliseAuthcallbackError(err)); - return; - } - /* the response from the callback might be a token string, a signed request or a token details */ - if (typeof tokenRequestOrDetails === 'string') { - if (tokenRequestOrDetails.length === 0) { - _callback(new ErrorInfo('Token string is empty', 40170, 401)); - } else if (tokenRequestOrDetails.length > MAX_TOKEN_LENGTH) { - _callback( - new ErrorInfo( - 'Token string exceeded max permitted length (was ' + tokenRequestOrDetails.length + ' bytes)', - 40170, - 401 - ) + if (err) { + Logger.logAction( + Logger.LOG_ERROR, + 'Auth.requestToken()', + 'token request signing call returned error; err = ' + Utils.inspectError(err) ); - } else if (tokenRequestOrDetails === 'undefined' || tokenRequestOrDetails === 'null') { - /* common failure mode with poorly-implemented authCallbacks */ - _callback(new ErrorInfo('Token string was literal null/undefined', 40170, 401)); - } else if (tokenRequestOrDetails[0] === '{' && !(contentType && contentType.indexOf('application/jwt') > -1)) { - _callback( + reject(normaliseAuthcallbackError(err)); + return; + } + /* the response from the callback might be a token string, a signed request or a token details */ + if (typeof tokenRequestOrDetails === 'string') { + if (tokenRequestOrDetails.length === 0) { + reject(new ErrorInfo('Token string is empty', 40170, 401)); + } else if (tokenRequestOrDetails.length > MAX_TOKEN_LENGTH) { + reject( + new ErrorInfo( + 'Token string exceeded max permitted length (was ' + tokenRequestOrDetails.length + ' bytes)', + 40170, + 401 + ) + ); + } else if (tokenRequestOrDetails === 'undefined' || tokenRequestOrDetails === 'null') { + /* common failure mode with poorly-implemented authCallbacks */ + reject(new ErrorInfo('Token string was literal null/undefined', 40170, 401)); + } else if ( + tokenRequestOrDetails[0] === '{' && + !(contentType && contentType.indexOf('application/jwt') > -1) + ) { + reject( + new ErrorInfo( + "Token was double-encoded; make sure you're not JSON-encoding an already encoded token request or details", + 40170, + 401 + ) + ); + } else { + resolve({ token: tokenRequestOrDetails } as API.TokenDetails); + } + return; + } + if (typeof tokenRequestOrDetails !== 'object') { + const msg = + 'Expected token request callback to call back with a token string or token request/details object, but got a ' + + typeof tokenRequestOrDetails; + Logger.logAction(Logger.LOG_ERROR, 'Auth.requestToken()', msg); + reject(new ErrorInfo(msg, 40170, 401)); + return; + } + const objectSize = JSON.stringify(tokenRequestOrDetails).length; + if (objectSize > MAX_TOKEN_LENGTH && !authOptions.suppressMaxLengthCheck) { + reject( new ErrorInfo( - "Token was double-encoded; make sure you're not JSON-encoding an already encoded token request or details", + 'Token request/details object exceeded max permitted stringified size (was ' + objectSize + ' bytes)', 40170, 401 ) ); - } else { - _callback(null, { token: tokenRequestOrDetails } as API.TokenDetails); + return; } - return; - } - if (typeof tokenRequestOrDetails !== 'object') { - const msg = - 'Expected token request callback to call back with a token string or token request/details object, but got a ' + - typeof tokenRequestOrDetails; - Logger.logAction(Logger.LOG_ERROR, 'Auth.requestToken()', msg); - _callback(new ErrorInfo(msg, 40170, 401)); - return; - } - const objectSize = JSON.stringify(tokenRequestOrDetails).length; - if (objectSize > MAX_TOKEN_LENGTH && !authOptions.suppressMaxLengthCheck) { - _callback( - new ErrorInfo( - 'Token request/details object exceeded max permitted stringified size (was ' + objectSize + ' bytes)', - 40170, - 401 - ) - ); - return; - } - if ('issued' in tokenRequestOrDetails) { - /* a tokenDetails object */ - _callback(null, tokenRequestOrDetails); - return; - } - if (!('keyName' in tokenRequestOrDetails)) { - const msg = - 'Expected token request callback to call back with a token string, token request object, or token details object'; - Logger.logAction(Logger.LOG_ERROR, 'Auth.requestToken()', msg); - _callback(new ErrorInfo(msg, 40170, 401)); - return; - } - /* it's a token request, so make the request */ - tokenRequest( - tokenRequestOrDetails, - function ( - err?: ErrorInfo | ErrnoException | null, - tokenResponse?: API.TokenDetails | string, - headers?: Record, - unpacked?: boolean - ) { - if (err) { - Logger.logAction( - Logger.LOG_ERROR, - 'Auth.requestToken()', - 'token request API call returned error; err = ' + Utils.inspectError(err) - ); - _callback(normaliseAuthcallbackError(err)); - return; - } - if (!unpacked) tokenResponse = JSON.parse(tokenResponse as string); - Logger.logAction(Logger.LOG_MINOR, 'Auth.getToken()', 'token received'); - _callback(null, tokenResponse as API.TokenDetails); + if ('issued' in tokenRequestOrDetails) { + /* a tokenDetails object */ + resolve(tokenRequestOrDetails); + return; } - ); + if (!('keyName' in tokenRequestOrDetails)) { + const msg = + 'Expected token request callback to call back with a token string, token request object, or token details object'; + Logger.logAction(Logger.LOG_ERROR, 'Auth.requestToken()', msg); + reject(new ErrorInfo(msg, 40170, 401)); + return; + } + /* it's a token request, so make the request */ + tokenRequest( + tokenRequestOrDetails, + function ( + err?: ErrorInfo | ErrnoException | null, + tokenResponse?: API.TokenDetails | string, + headers?: Record, + unpacked?: boolean + ) { + if (err) { + Logger.logAction( + Logger.LOG_ERROR, + 'Auth.requestToken()', + 'token request API call returned error; err = ' + Utils.inspectError(err) + ); + reject(normaliseAuthcallbackError(err)); + return; + } + if (!unpacked) tokenResponse = JSON.parse(tokenResponse as string); + Logger.logAction(Logger.LOG_MINOR, 'Auth.getToken()', 'token received'); + resolve(tokenResponse as API.TokenDetails); + } + ); + }); }); } - createTokenRequest(tokenParams: API.TokenParams | null, authOptions: any, callback: Function): void; - createTokenRequest(tokenParams: API.TokenParams | null, authOptions: any): Promise; /** * Create and sign a token request based on the given options. * NOTE this can only be used when the key value is available locally. @@ -738,88 +691,73 @@ class Auth { * * - timestamp: (optional) the time in ms since the epoch. If none is specified, * the system will be queried for a time value to use. - * - * @param callback */ - createTokenRequest(tokenParams: API.TokenParams | null, authOptions: any, callback?: Function) { - /* shuffle and normalise arguments as necessary */ - if (typeof tokenParams == 'function' && !callback) { - callback = tokenParams; - authOptions = tokenParams = null; - } else if (typeof authOptions == 'function' && !callback) { - callback = authOptions; - authOptions = null; - } - if (!callback) { - return Utils.promisify(this, 'createTokenRequest', arguments); - } - + async createTokenRequest(tokenParams: API.TokenParams | null, authOptions: any): Promise { /* RSA9h: if authOptions passed in, they're used instead of stored, don't merge them */ authOptions = authOptions || this.authOptions; tokenParams = tokenParams || Utils.copy(this.tokenParams); const key = authOptions.key; if (!key) { - callback(new ErrorInfo('No key specified', 40101, 403)); - return; + throw new ErrorInfo('No key specified', 40101, 403); } const keyParts = key.split(':'), keyName = keyParts[0], keySecret = keyParts[1]; if (!keySecret) { - callback(new ErrorInfo('Invalid key specified', 40101, 403)); - return; + throw new ErrorInfo('Invalid key specified', 40101, 403); } if (tokenParams.clientId === '') { - callback(new ErrorInfo('clientId can’t be an empty string', 40012, 400)); - return; + throw new ErrorInfo('clientId can’t be an empty string', 40012, 400); } if ('capability' in tokenParams) { tokenParams.capability = c14n(tokenParams.capability); } - const request = Utils.mixin({ keyName: keyName }, tokenParams), + const request: Partial = Utils.mixin({ keyName: keyName }, tokenParams), clientId = tokenParams.clientId || '', ttl = tokenParams.ttl || '', capability = tokenParams.capability || ''; - ((authoriseCb) => { - if (request.timestamp) { - authoriseCb(); - return; - } - this.getTimestamp(authOptions && authOptions.queryTime, function (err?: ErrorInfo | null, time?: number) { - if (err) { - callback!(err); + return new Promise((resolve, reject) => { + ((authoriseCb) => { + if (request.timestamp) { + authoriseCb(); return; } - request.timestamp = time; - authoriseCb(); + this.getTimestamp(authOptions && authOptions.queryTime, function (err?: ErrorInfo | null, time?: number) { + if (err) { + reject(err); + return; + } + request.timestamp = time; + authoriseCb(); + }); + })(function () { + /* nonce */ + /* NOTE: there is no expectation that the client + * specifies the nonce; this is done by the library + * However, this can be overridden by the client + * simply for testing purposes. */ + const nonce = request.nonce || (request.nonce = random()), + timestamp = request.timestamp; + + const signText = + request.keyName + '\n' + ttl + '\n' + capability + '\n' + clientId + '\n' + timestamp + '\n' + nonce + '\n'; + + /* mac */ + /* NOTE: there is no expectation that the client + * specifies the mac; this is done by the library + * However, this can be overridden by the client + * simply for testing purposes. */ + request.mac = request.mac || hmac(signText, keySecret); + + Logger.logAction(Logger.LOG_MINOR, 'Auth.getTokenRequest()', 'generated signed request'); + resolve(request as API.TokenRequest); }); - })(function () { - /* nonce */ - /* NOTE: there is no expectation that the client - * specifies the nonce; this is done by the library - * However, this can be overridden by the client - * simply for testing purposes. */ - const nonce = request.nonce || (request.nonce = random()), - timestamp = request.timestamp; - - const signText = - request.keyName + '\n' + ttl + '\n' + capability + '\n' + clientId + '\n' + timestamp + '\n' + nonce + '\n'; - - /* mac */ - /* NOTE: there is no expectation that the client - * specifies the mac; this is done by the library - * However, this can be overridden by the client - * simply for testing purposes. */ - request.mac = request.mac || hmac(signText, keySecret); - - Logger.logAction(Logger.LOG_MINOR, 'Auth.getTokenRequest()', 'generated signed request'); - callback!(null, request); }); } diff --git a/src/common/lib/client/baseclient.ts b/src/common/lib/client/baseclient.ts index b5c982ac75..e445ee6361 100644 --- a/src/common/lib/client/baseclient.ts +++ b/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,30 +128,23 @@ class BaseClient { return Defaults.getHttpScheme(this.options) + host + ':' + Defaults.getPort(this.options, false); } - stats( - params: RequestParams, - callback: StandardCallback> - ): Promise> | void { - return this.rest.stats(params, callback); + async stats(params: RequestParams): Promise> { + return this.rest.stats(params); } - time(params: RequestParams, callback: StandardCallback): void; - time(callback: StandardCallback): void; - time(params?: RequestParams): Promise; - time(params?: RequestParams | StandardCallback, callback?: StandardCallback): Promise | void { - return this.rest.time(params, callback); + async time(params?: RequestParams): Promise { + return this.rest.time(params); } - request( + async request( method: string, path: string, version: number, params: RequestParams, body: unknown, - customHeaders: Record, - callback: StandardCallback> - ): Promise> | void { - return this.rest.request(method, path, version, params, body, customHeaders, callback); + customHeaders: Record + ): Promise> { + return this.rest.request(method, path, version, params, body, customHeaders); } batchPublish( diff --git a/src/common/lib/client/connection.ts b/src/common/lib/client/connection.ts index 1447edde04..0f822ee1ec 100644 --- a/src/common/lib/client/connection.ts +++ b/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 { + async ping(): Promise { 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 { diff --git a/src/common/lib/client/paginatedresource.ts b/src/common/lib/client/paginatedresource.ts index 0513949be8..4ff93711cf 100644 --- a/src/common/lib/client/paginatedresource.ts +++ b/src/common/lib/client/paginatedresource.ts @@ -182,9 +182,9 @@ class PaginatedResource { export class PaginatedResult { resource: PaginatedResource; items: T[]; - first?: (results: PaginatedResultCallback) => void; - next?: (results: PaginatedResultCallback) => void; - current?: (results: PaginatedResultCallback) => void; + first?: () => Promise>; + next?: () => Promise | null>; + current?: () => Promise>; hasNext?: () => boolean; isLast?: () => boolean; @@ -195,29 +195,26 @@ export class PaginatedResult { 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 new Promise((resolve, reject) => { + self.get(relParams.first, (err, result) => (err ? reject(err) : resolve(result))); + }); }; } 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 new Promise((resolve, reject) => { + self.get(relParams.current, (err, result) => (err ? reject(err) : resolve(result))); + }); }; } - 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 new Promise((resolve, reject) => { + self.get(relParams.next, (err, result) => (err ? reject(err) : resolve(result))); + }); } else { - callback(null, null); + return null; } }; diff --git a/src/common/lib/client/push.ts b/src/common/lib/client/push.ts index 5344718414..3c1b10edfc 100644 --- a/src/common/lib/client/push.ts +++ b/src/common/lib/client/push.ts @@ -1,10 +1,9 @@ import * as Utils from '../util/utils'; import DeviceDetails from '../types/devicedetails'; import Resource from './resource'; -import PaginatedResource from './paginatedresource'; +import PaginatedResource, { PaginatedResult } from './paginatedresource'; import ErrorInfo from '../types/errorinfo'; import PushChannelSubscription from '../types/pushchannelsubscription'; -import { ErrCallback, PaginatedResultCallback, StandardCallback } from '../../types/utils'; import BaseClient from './baseclient'; import Defaults from '../util/defaults'; @@ -29,23 +28,23 @@ class Admin { this.channelSubscriptions = new ChannelSubscriptions(client); } - publish(recipient: any, payload: any, callback: ErrCallback) { + async publish(recipient: any, payload: any): Promise { const client = this.client; const format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, headers = Defaults.defaultPostHeaders(client.options, { format }), params = {}; const body = Utils.mixin({ recipient: recipient }, payload); - if (typeof callback !== 'function') { - return Utils.promisify(this, 'publish', arguments); - } - Utils.mixin(headers, client.options.headers); if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); const requestBody = Utils.encodeBody(body, client._MsgPack, format); - Resource.post(client, '/push/publish', requestBody, headers, params, null, (err) => callback(err)); + return new Promise((resolve, reject) => { + Resource.post(client, '/push/publish', requestBody, headers, params, null, (err) => + err ? reject(err) : resolve() + ); + }); } } @@ -56,163 +55,147 @@ class DeviceRegistrations { this.client = client; } - save(device: any, callback: StandardCallback) { + async save(device: any): Promise { const client = this.client; const body = DeviceDetails.fromValues(device); const format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, headers = Defaults.defaultPostHeaders(client.options, { format }), params = {}; - if (typeof callback !== 'function') { - return Utils.promisify(this, 'save', arguments); - } - Utils.mixin(headers, client.options.headers); if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); const requestBody = Utils.encodeBody(body, client._MsgPack, format); - Resource.put( - client, - '/push/deviceRegistrations/' + encodeURIComponent(device.id), - requestBody, - headers, - params, - null, - (err, body, headers, unpacked) => { - callback( - err, - !err - ? (DeviceDetails.fromResponseBody( - body as Record, - client._MsgPack, - unpacked ? undefined : format - ) as DeviceDetails) - : undefined - ); - } - ); + 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 + ); + } + ); + }); } - get(deviceIdOrDetails: any, callback: StandardCallback) { + async get(deviceIdOrDetails: any): Promise { const client = this.client, format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, headers = Defaults.defaultGetHeaders(client.options, { format }), deviceId = deviceIdOrDetails.id || deviceIdOrDetails; - if (typeof callback !== 'function') { - return Utils.promisify(this, 'get', arguments); - } - if (typeof deviceId !== 'string' || !deviceId.length) { - callback( - new ErrorInfo( - 'First argument to DeviceRegistrations#get must be a deviceId string or DeviceDetails', - 40000, - 400 - ) + throw new ErrorInfo( + 'First argument to DeviceRegistrations#get must be a deviceId string or DeviceDetails', + 40000, + 400 ); - return; } Utils.mixin(headers, client.options.headers); - Resource.get( - client, - '/push/deviceRegistrations/' + encodeURIComponent(deviceId), - headers, - {}, - null, - function (err, body, headers, unpacked) { - callback( - err, - !err - ? (DeviceDetails.fromResponseBody( - body as Record, - client._MsgPack, - unpacked ? undefined : format - ) as DeviceDetails) - : undefined - ); - } - ); + 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 + ); + } + ); + }); } - list(params: any, callback: PaginatedResultCallback) { + async list(params: any): Promise> { const client = this.client, format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, envelope = this.client.http.supportsLinkHeaders ? undefined : format, headers = Defaults.defaultGetHeaders(client.options, { format }); - if (typeof callback !== 'function') { - return Utils.promisify(this, 'list', arguments); - } - Utils.mixin(headers, client.options.headers); - new PaginatedResource(client, '/push/deviceRegistrations', headers, envelope, async function ( - body, - headers, - unpacked - ) { - return DeviceDetails.fromResponseBody( - body as Record[], - client._MsgPack, - unpacked ? undefined : format - ); - }).get(params, callback); + return new Promise((resolve, reject) => { + new PaginatedResource(client, '/push/deviceRegistrations', headers, envelope, async function ( + body, + headers, + unpacked + ) { + return DeviceDetails.fromResponseBody( + body as Record[], + client._MsgPack, + unpacked ? undefined : format + ); + }).get(params, (err, result) => (err ? reject(err) : resolve(result))); + }); } - remove(deviceIdOrDetails: any, callback: ErrCallback) { + async remove(deviceIdOrDetails: any): Promise { const client = this.client, format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, headers = Defaults.defaultGetHeaders(client.options, { format }), params = {}, deviceId = deviceIdOrDetails.id || deviceIdOrDetails; - if (typeof callback !== 'function') { - return Utils.promisify(this, 'remove', arguments); - } - if (typeof deviceId !== 'string' || !deviceId.length) { - callback( - new ErrorInfo( - 'First argument to DeviceRegistrations#remove must be a deviceId string or DeviceDetails', - 40000, - 400 - ) + throw new ErrorInfo( + 'First argument to DeviceRegistrations#remove must be a deviceId string or DeviceDetails', + 40000, + 400 ); - return; } Utils.mixin(headers, client.options.headers); if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); - Resource['delete']( - client, - '/push/deviceRegistrations/' + encodeURIComponent(deviceId), - headers, - params, - null, - (err) => callback(err) - ); + return new Promise((resolve, reject) => { + Resource['delete']( + client, + '/push/deviceRegistrations/' + encodeURIComponent(deviceId), + headers, + params, + null, + (err) => (err ? reject(err) : resolve()) + ); + }); } - removeWhere(params: any, callback: ErrCallback) { + async removeWhere(params: any): Promise { const client = this.client, format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, headers = Defaults.defaultGetHeaders(client.options, { format }); - if (typeof callback !== 'function') { - return Utils.promisify(this, 'removeWhere', arguments); - } - Utils.mixin(headers, client.options.headers); if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); - Resource['delete'](client, '/push/deviceRegistrations', headers, params, null, (err) => callback(err)); + return new Promise((resolve, reject) => { + Resource['delete'](client, '/push/deviceRegistrations', headers, params, null, (err) => + err ? reject(err) : resolve() + ); + }); } } @@ -223,112 +206,105 @@ class ChannelSubscriptions { this.client = client; } - save(subscription: Record, callback: StandardCallback) { + async save(subscription: Record): Promise { const client = this.client; const body = PushChannelSubscription.fromValues(subscription); const format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, headers = Defaults.defaultPostHeaders(client.options, { format }), params = {}; - if (typeof callback !== 'function') { - return Utils.promisify(this, 'save', arguments); - } - Utils.mixin(headers, client.options.headers); if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); const requestBody = Utils.encodeBody(body, client._MsgPack, format); - Resource.post( - client, - '/push/channelSubscriptions', - requestBody, - headers, - params, - null, - function (err, body, headers, unpacked) { - callback( - err, - !err - ? (PushChannelSubscription.fromResponseBody( - body as Record, - client._MsgPack, - unpacked ? undefined : format - ) as PushChannelSubscription) - : undefined - ); - } - ); + 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 + ); + } + ); + }); } - list(params: any, callback: PaginatedResultCallback) { + async list(params: any): Promise> { const client = this.client, format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, envelope = this.client.http.supportsLinkHeaders ? undefined : format, headers = Defaults.defaultGetHeaders(client.options, { format }); - if (typeof callback !== 'function') { - return Utils.promisify(this, 'list', arguments); - } - Utils.mixin(headers, client.options.headers); - new PaginatedResource(client, '/push/channelSubscriptions', headers, envelope, async function ( - body, - headers, - unpacked - ) { - return PushChannelSubscription.fromResponseBody( - body as Record[], - client._MsgPack, - unpacked ? undefined : format - ); - }).get(params, callback); + return new Promise((resolve, reject) => { + new PaginatedResource(client, '/push/channelSubscriptions', headers, envelope, async function ( + body, + headers, + unpacked + ) { + return PushChannelSubscription.fromResponseBody( + body as Record[], + client._MsgPack, + unpacked ? undefined : format + ); + }).get(params, (err, result) => (err ? reject(err) : resolve(result))); + }); } - removeWhere(params: any, callback: ErrCallback) { + async removeWhere(params: any): Promise { const client = this.client, format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, headers = Defaults.defaultGetHeaders(client.options, { format }); - if (typeof callback !== 'function') { - return Utils.promisify(this, 'removeWhere', arguments); - } - Utils.mixin(headers, client.options.headers); if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); - Resource['delete'](client, '/push/channelSubscriptions', headers, params, null, (err) => callback(err)); + return new Promise((resolve, reject) => { + Resource['delete'](client, '/push/channelSubscriptions', headers, params, null, (err) => + err ? reject(err) : resolve() + ); + }); } /* ChannelSubscriptions have no unique id; removing one is equivalent to removeWhere by its properties */ remove = ChannelSubscriptions.prototype.removeWhere; - listChannels(params: any, callback: PaginatedResultCallback) { + async listChannels(params: any): Promise> { const client = this.client, format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, envelope = this.client.http.supportsLinkHeaders ? undefined : format, headers = Defaults.defaultGetHeaders(client.options, { format }); - if (typeof callback !== 'function') { - return Utils.promisify(this, 'listChannels', arguments); - } - Utils.mixin(headers, client.options.headers); if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); - new PaginatedResource(client, '/push/channels', headers, envelope, async function (body, headers, unpacked) { - const parsedBody = ( - !unpacked && format ? Utils.decodeBody(body, client._MsgPack, format) : body - ) as Array; - - for (let i = 0; i < parsedBody.length; i++) { - parsedBody[i] = String(parsedBody[i]); - } - return parsedBody; - }).get(params, callback); + return new Promise((resolve, reject) => { + new PaginatedResource(client, '/push/channels', headers, envelope, async function (body, headers, unpacked) { + const parsedBody = ( + !unpacked && format ? Utils.decodeBody(body, client._MsgPack, format) : body + ) as Array; + + for (let i = 0; i < parsedBody.length; i++) { + parsedBody[i] = String(parsedBody[i]); + } + return parsedBody; + }).get(params, (err, result) => (err ? reject(err) : resolve(result))); + }); } } diff --git a/src/common/lib/client/realtimechannel.ts b/src/common/lib/client/realtimechannel.ts index 3bd0ccf0fd..0f5838d5f6 100644 --- a/src/common/lib/client/realtimechannel.ts +++ b/src/common/lib/client/realtimechannel.ts @@ -17,13 +17,13 @@ import Message, { EncodingDecodingContext, } from '../types/message'; import ChannelStateChange from './channelstatechange'; -import ErrorInfo, { IPartialErrorInfo, PartialErrorInfo } from '../types/errorinfo'; +import ErrorInfo, { PartialErrorInfo } from '../types/errorinfo'; import PresenceMessage, { decode as decodePresenceMessage } from '../types/presencemessage'; import ConnectionErrors from '../transport/connectionerrors'; import * as API from '../../../../ably'; import ConnectionManager from '../transport/connectionmanager'; import ConnectionStateChange from './connectionstatechange'; -import { ErrCallback, PaginatedResultCallback, StandardCallback } from '../../types/utils'; +import { ErrCallback, StandardCallback } from '../../types/utils'; import BaseRealtime from './baserealtime'; import { ChannelOptions } from '../../types/channel'; import { normaliseChannelOptions } from '../util/defaults'; @@ -143,32 +143,18 @@ class RealtimeChannel extends EventEmitter { } static processListenerArgs(args: unknown[]): any[] { - /* [event], listener, [callback] */ + /* [event], listener */ args = Array.prototype.slice.call(args); if (typeof args[0] === 'function') { args.unshift(null); } - if (args[args.length - 1] == undefined) { - args.pop(); - } return args; } - setOptions(options?: API.ChannelOptions, callback?: ErrCallback): void | Promise { - if (!callback) { - return Utils.promisify(this, 'setOptions', arguments); - } - const _callback = - callback || - function (err?: IPartialErrorInfo | null) { - if (err) { - Logger.logAction(Logger.LOG_ERROR, 'RealtimeChannel.setOptions()', 'Set options failed: ' + err.toString()); - } - }; + async setOptions(options?: API.ChannelOptions): Promise { const err = validateChannelOptions(options); if (err) { - _callback(err); - return; + throw err; } this.channelOptions = normaliseChannelOptions(this.client._Crypto ?? null, options); if (this._decodingContext) this._decodingContext.channelOptions = this.channelOptions; @@ -180,25 +166,24 @@ class RealtimeChannel extends EventEmitter { * rejecting messages until we have confirmation that the options have changed, * which would unnecessarily lose message continuity. */ this.attachImpl(); - // Ignore 'attaching' -- could be just due to to a resume & reattach, should not - // call back setOptions until we're definitely attached with the new options (or - // else in a terminal state) - this._allChannelChanges.once( - ['attached', 'update', 'detached', 'failed'], - function (this: { event: string }, stateChange: ConnectionStateChange) { - switch (this.event) { - case 'update': - case 'attached': - _callback?.(null); - return; - default: - _callback?.(stateChange.reason); - return; + return new Promise((resolve, reject) => { + // Ignore 'attaching' -- could be just due to to a resume & reattach, should not + // call back setOptions until we're definitely attached with the new options (or + // else in a terminal state) + this._allChannelChanges.once( + ['attached', 'update', 'detached', 'failed'], + function (this: { event: string }, stateChange: ConnectionStateChange) { + switch (this.event) { + case 'update': + case 'attached': + resolve(); + break; + default: + reject(stateChange.reason); + } } - } - ); - } else { - _callback(); + ); + }); } } @@ -226,19 +211,14 @@ class RealtimeChannel extends EventEmitter { return false; } - publish(...args: any[]): void | Promise { + async publish(...args: any[]): Promise { let messages = args[0]; let argCount = args.length; - let callback = args[argCount - 1]; - if (typeof callback !== 'function') { - return Utils.promisify(this, 'publish', arguments); - } if (!this.connectionManager.activeState()) { - callback(this.connectionManager.getError()); - return; + throw this.connectionManager.getError(); } - if (argCount == 2) { + if (argCount == 1) { if (Utils.isObject(messages)) messages = [messageFromValues(messages)]; else if (Utils.isArray(messages)) messages = messagesFromValuesArray(messages); else @@ -251,28 +231,30 @@ class RealtimeChannel extends EventEmitter { messages = [messageFromValues({ name: args[0], data: args[1] })]; } const maxMessageSize = this.client.options.maxMessageSize; - encodeMessagesArray(messages, this.channelOptions as CipherOptions, (err: Error | null) => { - if (err) { - callback(err); - return; - } - /* RSL1i */ - const size = getMessagesSize(messages); - if (size > maxMessageSize) { - callback( - new ErrorInfo( - 'Maximum size of messages that can be published at once exceeded ( was ' + - size + - ' bytes; limit is ' + - maxMessageSize + - ' bytes)', - 40009, - 400 - ) - ); - return; - } - this._publish(messages, callback); + return new Promise((resolve, reject) => { + encodeMessagesArray(messages, this.channelOptions as CipherOptions, (err: Error | null) => { + if (err) { + reject(err); + return; + } + /* RSL1i */ + const size = getMessagesSize(messages); + if (size > maxMessageSize) { + reject( + new ErrorInfo( + 'Maximum size of messages that can be published at once exceeded ( was ' + + size + + ' bytes; limit is ' + + maxMessageSize + + ' bytes)', + 40009, + 400 + ) + ); + return; + } + this._publish(messages, (err) => (err ? reject(err) : resolve())); + }); }); } @@ -305,18 +287,14 @@ class RealtimeChannel extends EventEmitter { } } - attach(): Promise; - attach(callback: StandardCallback): void; - attach(callback?: StandardCallback): void | Promise { - if (!callback) { - return Utils.promisify(this, 'attach', arguments); - } + async attach(): Promise { if (this.state === 'attached') { - callback(null, null); - return; + return null; } - this._attach(false, null, callback); + return new Promise((resolve, reject) => { + this._attach(false, null, (err, result) => (err ? reject(err) : resolve(result!))); + }); } _attach( @@ -387,48 +365,43 @@ class RealtimeChannel extends EventEmitter { this.sendMessage(attachMsg, noop); } - detach(callback: ErrCallback): void | Promise { - if (!callback) { - return Utils.promisify(this, 'detach', arguments); - } + async detach(): Promise { const connectionManager = this.connectionManager; if (!connectionManager.activeState()) { - callback(connectionManager.getError()); - return; + throw connectionManager.getError(); } switch (this.state) { case 'suspended': this.notifyState('detached'); - callback(); - break; + return; case 'detached': - callback(); - break; + return; case 'failed': - callback(new ErrorInfo('Unable to detach; channel state = failed', 90001, 400)); - break; + throw new ErrorInfo('Unable to detach; channel state = failed', 90001, 400); default: this.requestState('detaching'); // eslint-disable-next-line no-fallthrough case 'detaching': - this.once(function (this: { event: string }, stateChange: ChannelStateChange) { - switch (this.event) { - case 'detached': - callback(); - break; - case 'attached': - case 'suspended': - case 'failed': - callback( - stateChange.reason || - connectionManager.getError() || - new ErrorInfo('Unable to detach; reason unknown; state = ' + this.event, 90000, 500) - ); - break; - case 'attaching': - callback(new ErrorInfo('Detach request superseded by a subsequent attach request', 90000, 409)); - break; - } + return new Promise((resolve, reject) => { + this.once(function (this: { event: string }, stateChange: ChannelStateChange) { + switch (this.event) { + case 'detached': + resolve(); + break; + case 'attached': + case 'suspended': + case 'failed': + reject( + stateChange.reason || + connectionManager.getError() || + new ErrorInfo('Unable to detach; reason unknown; state = ' + this.event, 90000, 500) + ); + break; + case 'attaching': + reject(new ErrorInfo('Detach request superseded by a subsequent attach request', 90000, 409)); + break; + } + }); }); } } @@ -439,16 +412,11 @@ class RealtimeChannel extends EventEmitter { this.sendMessage(msg, callback || noop); } - subscribe(...args: unknown[] /* [event], listener, [callback] */): void | Promise { - const [event, listener, callback] = RealtimeChannel.processListenerArgs(args); - - if (!callback) { - return Utils.promisify(this, 'subscribe', [event, listener]); - } + async subscribe(...args: unknown[] /* [event], listener */): Promise { + const [event, listener] = RealtimeChannel.processListenerArgs(args); if (this.state === 'failed') { - callback?.(ErrorInfo.fromValues(this.invalidStateError())); - return; + throw ErrorInfo.fromValues(this.invalidStateError()); } // Filtered @@ -458,7 +426,7 @@ class RealtimeChannel extends EventEmitter { this.subscriptions.on(event, listener); } - return this.attach(callback || noop); + return this.attach(); } unsubscribe(...args: unknown[] /* [event], listener */): void { @@ -886,45 +854,33 @@ class RealtimeChannel extends EventEmitter { } } - history = function ( + history = async function ( this: RealtimeChannel, - params: RealtimeHistoryParams | null, - callback: PaginatedResultCallback - ): void | Promise> { + params: RealtimeHistoryParams | null + ): Promise> { Logger.logAction(Logger.LOG_MICRO, 'RealtimeChannel.history()', 'channel = ' + this.name); - /* params and callback are optional; see if params contains the callback */ - if (callback === undefined) { - if (typeof params == 'function') { - callback = params; - params = null; - } else { - return Utils.promisify(this, 'history', arguments); - } - } // We fetch this first so that any module-not-provided error takes priority over other errors const restMixin = this.client.rest.channelMixin; if (params && params.untilAttach) { if (this.state !== 'attached') { - callback(new ErrorInfo('option untilAttach requires the channel to be attached', 40000, 400)); - return; + throw new ErrorInfo('option untilAttach requires the channel to be attached', 40000, 400); } if (!this.properties.attachSerial) { - callback( - new ErrorInfo( - 'untilAttach was specified and channel is attached, but attachSerial is not defined', - 40000, - 400 - ) + throw new ErrorInfo( + 'untilAttach was specified and channel is attached, but attachSerial is not defined', + 40000, + 400 ); - return; } delete params.untilAttach; params.from_serial = this.properties.attachSerial; } - restMixin.history(this, params, callback); + return new Promise((resolve, reject) => { + restMixin.history(this, params, (err, result) => (err ? reject(err) : resolve(result))); + }); } as any; whenState = ((state: string, listener: ErrCallback) => { @@ -959,8 +915,8 @@ class RealtimeChannel extends EventEmitter { } } - status(callback?: StandardCallback): void | Promise { - return this.client.rest.channelMixin.status(this, callback); + async status(): Promise { + return this.client.rest.channelMixin.status(this); } } diff --git a/src/common/lib/client/realtimepresence.ts b/src/common/lib/client/realtimepresence.ts index b0010d74b5..abaf126e6b 100644 --- a/src/common/lib/client/realtimepresence.ts +++ b/src/common/lib/client/realtimepresence.ts @@ -11,7 +11,7 @@ import RealtimeChannel from './realtimechannel'; import Multicaster from '../util/multicaster'; import ChannelStateChange from './channelstatechange'; import { CipherOptions } from '../types/message'; -import { ErrCallback, PaginatedResultCallback, StandardCallback } from '../../types/utils'; +import { ErrCallback } from '../../types/utils'; import { PaginatedResult } from './paginatedresource'; interface RealtimePresenceParams { @@ -101,61 +101,37 @@ class RealtimePresence extends EventEmitter { this.pendingPresence = []; } - enter(data: unknown, callback: ErrCallback): void | Promise { + async enter(data: unknown): Promise { if (isAnonymousOrWildcard(this)) { throw new ErrorInfo('clientId must be specified to enter a presence channel', 40012, 400); } - return this._enterOrUpdateClient(undefined, undefined, data, 'enter', callback); + return this._enterOrUpdateClient(undefined, undefined, data, 'enter'); } - update(data: unknown, callback: ErrCallback): void | Promise { + async update(data: unknown): Promise { if (isAnonymousOrWildcard(this)) { throw new ErrorInfo('clientId must be specified to update presence data', 40012, 400); } - return this._enterOrUpdateClient(undefined, undefined, data, 'update', callback); + return this._enterOrUpdateClient(undefined, undefined, data, 'update'); } - enterClient(clientId: string, data: unknown, callback: ErrCallback): void | Promise { - return this._enterOrUpdateClient(undefined, clientId, data, 'enter', callback); + async enterClient(clientId: string, data: unknown): Promise { + return this._enterOrUpdateClient(undefined, clientId, data, 'enter'); } - updateClient(clientId: string, data: unknown, callback: ErrCallback): void | Promise { - return this._enterOrUpdateClient(undefined, clientId, data, 'update', callback); + async updateClient(clientId: string, data: unknown): Promise { + return this._enterOrUpdateClient(undefined, clientId, data, 'update'); } - _enterOrUpdateClient( + async _enterOrUpdateClient( id: string | undefined, clientId: string | undefined, data: unknown, action: string - ): Promise; - _enterOrUpdateClient( - id: string | undefined, - clientId: string | undefined, - data: unknown, - action: string, - callback: ErrCallback - ): void; - _enterOrUpdateClient( - id: string | undefined, - clientId: string | undefined, - data: unknown, - action: string, - callback?: ErrCallback - ): void | Promise { - if (!callback) { - if (typeof data === 'function') { - callback = data as ErrCallback; - data = null; - } else { - return Utils.promisify(this, '_enterOrUpdateClient', [id, clientId, data, action]); - } - } - + ): Promise { const channel = this.channel; if (!channel.connectionManager.activeState()) { - callback(channel.connectionManager.getError()); - return; + throw channel.connectionManager.getError(); } Logger.logAction( @@ -173,57 +149,49 @@ class RealtimePresence extends EventEmitter { presence.clientId = clientId; } - encodePresenceMessage(presence, channel.channelOptions as CipherOptions, (err: IPartialErrorInfo) => { - if (err) { - callback!(err); - return; - } - switch (channel.state) { - case 'attached': - channel.sendPresence(presence, callback); - break; - case 'initialized': - case 'detached': - channel.attach(); - // eslint-disable-next-line no-fallthrough - case 'attaching': - this.pendingPresence.push({ - presence: presence, - callback: callback!, - }); - break; - default: - err = new PartialErrorInfo( - 'Unable to ' + action + ' presence channel while in ' + channel.state + ' state', - 90001 - ); - err.code = 90001; - callback!(err); - } + return new Promise((resolve, reject) => { + encodePresenceMessage(presence, channel.channelOptions as CipherOptions, (err: IPartialErrorInfo) => { + if (err) { + reject(err); + return; + } + switch (channel.state) { + case 'attached': + channel.sendPresence(presence, (err) => (err ? reject(err) : resolve())); + break; + case 'initialized': + case 'detached': + channel.attach(); + // eslint-disable-next-line no-fallthrough + case 'attaching': + this.pendingPresence.push({ + presence: presence, + callback: (err) => (err ? reject(err) : resolve()), + }); + break; + default: + err = new PartialErrorInfo( + 'Unable to ' + action + ' presence channel while in ' + channel.state + ' state', + 90001 + ); + err.code = 90001; + reject(err); + } + }); }); } - leave(data: unknown, callback: ErrCallback): void | Promise { + async leave(data: unknown): Promise { if (isAnonymousOrWildcard(this)) { throw new ErrorInfo('clientId must have been specified to enter or leave a presence channel', 40012, 400); } - return this.leaveClient(undefined, data, callback); + return this.leaveClient(undefined, data); } - leaveClient(clientId?: string, data?: unknown, callback?: ErrCallback): void | Promise { - if (!callback) { - if (typeof data === 'function') { - callback = data as ErrCallback; - data = null; - } else { - return Utils.promisify(this, 'leaveClient', [clientId, data]); - } - } - + async leaveClient(clientId?: string, data?: unknown): Promise { const channel = this.channel; if (!channel.connectionManager.activeState()) { - callback?.(channel.connectionManager.getError()); - return; + throw channel.connectionManager.getError(); } Logger.logAction( @@ -237,92 +205,74 @@ class RealtimePresence extends EventEmitter { presence.clientId = clientId; } - switch (channel.state) { - case 'attached': - channel.sendPresence(presence, callback); - break; - case 'attaching': - this.pendingPresence.push({ - presence: presence, - callback: callback, - }); - break; - case 'initialized': - case 'failed': { - /* we're not attached; therefore we let any entered status - * timeout by itself instead of attaching just in order to leave */ - const err = new PartialErrorInfo('Unable to leave presence channel (incompatible state)', 90001); - callback?.(err); - break; + return new Promise((resolve, reject) => { + switch (channel.state) { + case 'attached': + channel.sendPresence(presence, (err) => (err ? reject(err) : resolve())); + break; + case 'attaching': + this.pendingPresence.push({ + presence: presence, + callback: (err) => (err ? reject(err) : resolve()), + }); + break; + case 'initialized': + case 'failed': { + /* we're not attached; therefore we let any entered status + * timeout by itself instead of attaching just in order to leave */ + const err = new PartialErrorInfo('Unable to leave presence channel (incompatible state)', 90001); + reject(err); + break; + } + default: + reject(channel.invalidStateError()); } - default: - callback?.(channel.invalidStateError()); - } + }); } - get( - this: RealtimePresence, - params: RealtimePresenceParams, - callback: StandardCallback - ): void | Promise { - const args = Array.prototype.slice.call(arguments); - if (args.length == 1 && typeof args[0] == 'function') args.unshift(null); - - params = args[0] as RealtimePresenceParams; - callback = args[1] as StandardCallback; + async get(params?: RealtimePresenceParams): Promise { const waitForSync = !params || ('waitForSync' in params ? params.waitForSync : true); - if (!callback) { - return Utils.promisify(this, 'get', args); - } - - function returnMembers(members: PresenceMap) { - callback(null, params ? members.list(params) : members.values()); - } - - /* Special-case the suspended state: can still get (stale) presence set if waitForSync is false */ - if (this.channel.state === 'suspended') { - if (waitForSync) { - callback( - ErrorInfo.fromValues({ - statusCode: 400, - code: 91005, - message: 'Presence state is out of sync due to channel being in the SUSPENDED state', - }) - ); - } else { - returnMembers(this.members); + return new Promise((resolve, reject) => { + function returnMembers(members: PresenceMap) { + resolve(params ? members.list(params) : members.values()); } - return; - } - waitAttached(this.channel, callback, () => { - const members = this.members; - if (waitForSync) { - members.waitSync(function () { - returnMembers(members); - }); - } else { - returnMembers(members); + /* Special-case the suspended state: can still get (stale) presence set if waitForSync is false */ + if (this.channel.state === 'suspended') { + if (waitForSync) { + reject( + ErrorInfo.fromValues({ + statusCode: 400, + code: 91005, + message: 'Presence state is out of sync due to channel being in the SUSPENDED state', + }) + ); + } else { + returnMembers(this.members); + } + return; } + + waitAttached( + this.channel, + (err) => reject(err), + () => { + const members = this.members; + if (waitForSync) { + members.waitSync(function () { + returnMembers(members); + }); + } else { + returnMembers(members); + } + } + ); }); } - history( - params: RealtimeHistoryParams | null, - callback: PaginatedResultCallback - ): void | Promise> { + async history(params: RealtimeHistoryParams | null): Promise> { Logger.logAction(Logger.LOG_MICRO, 'RealtimePresence.history()', 'channel = ' + this.name); - /* params and callback are optional; see if params contains the callback */ - if (callback === undefined) { - if (typeof params == 'function') { - callback = params; - params = null; - } else { - return Utils.promisify(this, 'history', arguments); - } - } - // We fetch this first so that any module-not-provided error takes priority over other errors const restMixin = this.channel.client.rest.presenceMixin; @@ -331,17 +281,15 @@ class RealtimePresence extends EventEmitter { delete params.untilAttach; params.from_serial = this.channel.properties.attachSerial; } else { - callback( - new ErrorInfo( - 'option untilAttach requires the channel to be attached, was: ' + this.channel.state, - 40000, - 400 - ) + throw new ErrorInfo( + 'option untilAttach requires the channel to be attached, was: ' + this.channel.state, + 40000, + 400 ); } } - return restMixin.history(this, params, callback); + return restMixin.history(this, params); } setPresence(presenceSet: PresenceMessage[], isSync: boolean, syncChannelSerial?: string): void { @@ -514,24 +462,18 @@ class RealtimePresence extends EventEmitter { }); } - subscribe(..._args: unknown[] /* [event], listener, [callback] */): void | Promise { + async subscribe(..._args: unknown[] /* [event], listener */): Promise { const args = RealtimeChannel.processListenerArgs(_args); const event = args[0]; const listener = args[1]; - let callback = args[2]; const channel = this.channel; - if (!callback) { - return Utils.promisify(this, 'subscribe', [event, listener]); - } - if (channel.state === 'failed') { - callback(ErrorInfo.fromValues(channel.invalidStateError())); - return; + throw ErrorInfo.fromValues(channel.invalidStateError()); } this.subscriptions.on(event, listener); - channel.attach(callback); + await channel.attach(); } unsubscribe(..._args: unknown[] /* [event], listener */): void { diff --git a/src/common/lib/client/rest.ts b/src/common/lib/client/rest.ts index 5340ce09ae..ba445b95d8 100644 --- a/src/common/lib/client/rest.ts +++ b/src/common/lib/client/rest.ts @@ -8,7 +8,6 @@ import ErrorInfo from '../types/errorinfo'; import Stats from '../types/stats'; import HttpMethods from '../../constants/HttpMethods'; import { ChannelOptions } from '../../types/channel'; -import { PaginatedResultCallback, StandardCallback } from '../../types/utils'; import { RequestBody, RequestParams } from '../../types/http'; import * as API from '../../../../ably'; import Resource from './resource'; @@ -35,7 +34,6 @@ type TokenRevocationSuccessResult = API.TokenRevocationSuccessResult; type TokenRevocationFailureResult = API.TokenRevocationFailureResult; type TokenRevocationResult = BatchResult; -const noop = function () {}; export class Rest { private readonly client: BaseClient; readonly channels: Channels; @@ -50,83 +48,62 @@ export class Rest { this.push = new Push(this.client); } - stats( - params: RequestParams, - callback: StandardCallback> - ): Promise> | void { - /* params and callback are optional; see if params contains the callback */ - if (callback === undefined) { - if (typeof params == 'function') { - callback = params; - params = null; - } else { - return Utils.promisify(this, 'stats', [params]) as Promise>; - } - } + async stats(params: RequestParams): Promise> { const headers = Defaults.defaultGetHeaders(this.client.options), format = this.client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, envelope = this.client.http.supportsLinkHeaders ? undefined : format; Utils.mixin(headers, this.client.options.headers); - new PaginatedResource(this.client, '/stats', headers, envelope, function (body, headers, unpacked) { - const statsValues = unpacked ? body : JSON.parse(body as string); - for (let i = 0; i < statsValues.length; i++) statsValues[i] = Stats.fromValues(statsValues[i]); - return statsValues; - }).get(params as Record, callback); + return new Promise((resolve, reject) => { + new PaginatedResource(this.client, '/stats', headers, envelope, function (body, headers, unpacked) { + const statsValues = unpacked ? body : JSON.parse(body as string); + for (let i = 0; i < statsValues.length; i++) statsValues[i] = Stats.fromValues(statsValues[i]); + return statsValues; + }).get(params as Record, (err, result) => (err ? reject(err) : resolve(result))); + }); } - time(params?: RequestParams | StandardCallback, callback?: StandardCallback): Promise | void { - /* params and callback are optional; see if params contains the callback */ - if (callback === undefined) { - if (typeof params == 'function') { - callback = params; - params = null; - } else { - return Utils.promisify(this, 'time', [params]) as Promise; - } - } - - const _callback = callback || noop; - + async time(params?: RequestParams): Promise { const headers = Defaults.defaultGetHeaders(this.client.options); if (this.client.options.headers) Utils.mixin(headers, this.client.options.headers); const timeUri = (host: string) => { return this.client.baseUri(host) + '/time'; }; - this.client.http.do( - HttpMethods.Get, - timeUri, - headers, - null, - params as RequestParams, - (err, res, headers, unpacked) => { - if (err) { - _callback(err); - return; - } - if (!unpacked) res = JSON.parse(res as string); - const time = (res as number[])[0]; - if (!time) { - _callback(new ErrorInfo('Internal error (unexpected result type from GET /time)', 50000, 500)); - return; + return new Promise((resolve, reject) => { + this.client.http.do( + HttpMethods.Get, + timeUri, + headers, + null, + params as RequestParams, + (err, res, headers, unpacked) => { + if (err) { + reject(err); + return; + } + if (!unpacked) res = JSON.parse(res as string); + const time = (res as number[])[0]; + if (!time) { + reject(new ErrorInfo('Internal error (unexpected result type from GET /time)', 50000, 500)); + return; + } + /* calculate time offset only once for this device by adding to the prototype */ + this.client.serverTimeOffset = time - Utils.now(); + resolve(time); } - /* calculate time offset only once for this device by adding to the prototype */ - this.client.serverTimeOffset = time - Utils.now(); - _callback(null, time); - } - ); + ); + }); } - request( + async request( method: string, path: string, version: number, params: RequestParams, body: unknown, - customHeaders: Record, - callback: StandardCallback> - ): Promise> | void { + customHeaders: Record + ): Promise> { const [encoder, decoder, format] = (() => { if (this.client.options.useBinaryProtocol) { if (!this.client._MsgPack) { @@ -145,12 +122,6 @@ export class Rest { ? Defaults.defaultGetHeaders(this.client.options, { format, protocolVersion: version }) : Defaults.defaultPostHeaders(this.client.options, { format, protocolVersion: version }); - if (callback === undefined) { - return Utils.promisify(this, 'request', [method, path, version, params, body, customHeaders]) as Promise< - HttpPaginatedResponse - >; - } - if (typeof body !== 'string') { body = encoder(body) ?? null; } @@ -173,31 +144,22 @@ export class Rest { throw new ErrorInfo('Unsupported method ' + _method, 40500, 405); } - if (Utils.arrIn(Platform.Http.methodsWithBody, _method)) { - paginatedResource[_method as HttpMethods.Post]( - params, - body as RequestBody, - callback as PaginatedResultCallback - ); - } else { - paginatedResource[_method as HttpMethods.Get | HttpMethods.Delete]( - params, - callback as PaginatedResultCallback - ); - } + return new Promise((resolve, reject) => { + if (Utils.arrIn(Platform.Http.methodsWithBody, _method)) { + paginatedResource[_method as HttpMethods.Post](params!, body as RequestBody, (err, result) => + err ? reject(err) : resolve(result) + ); + } else { + paginatedResource[_method as HttpMethods.Get | HttpMethods.Delete](params!, (err, result) => + err ? reject(err) : resolve(result) + ); + } + }); } - batchPublish( + async batchPublish( specOrSpecs: T - ): Promise; - batchPublish( - specOrSpecs: T, - callback?: StandardCallback - ): void | Promise { - if (callback === undefined) { - return Utils.promisify(this, 'batchPublish', [specOrSpecs]); - } - + ): Promise { let requestBodyDTO: BatchPublishSpec[]; let singleSpecMode: boolean; if (Utils.isArray(specOrSpecs)) { @@ -214,34 +176,28 @@ export class Rest { if (this.client.options.headers) Utils.mixin(headers, this.client.options.headers); const requestBody = Utils.encodeBody(requestBodyDTO, this.client._MsgPack, format); - Resource.post(this.client, '/messages', requestBody, headers, {}, null, (err, body, headers, unpacked) => { - if (err) { - callback(err); - return; - } + 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 batchResults = ( + unpacked ? body : Utils.decodeBody(body, this.client._MsgPack, format) + ) as BatchPublishResult[]; - // I don't love the below type assertions for `callback` but not sure how to avoid them - if (singleSpecMode) { - (callback as StandardCallback)(null, batchResults[0]); - } else { - (callback as StandardCallback)(null, batchResults); - } + // 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); + } + }); }); } - batchPresence(channels: string[]): Promise; - batchPresence( - channels: string[], - callback?: StandardCallback - ): void | Promise { - if (callback === undefined) { - return Utils.promisify(this, 'batchPresence', [channels]); - } - + async batchPresence(channels: string[]): Promise { const format = this.client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, headers = Defaults.defaultPostHeaders(this.client.options, { format }); @@ -249,25 +205,27 @@ export class Rest { const channelsParam = channels.join(','); - Resource.get( - this.client, - '/presence', - headers, - { channels: channelsParam }, - null, - (err, body, headers, unpacked) => { - if (err) { - callback(err); - return; - } + 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 batchResult = ( + unpacked ? body : Utils.decodeBody(body, this.client._MsgPack, format) + ) as BatchPresenceResult; - callback(null, batchResult); - } - ); + resolve(batchResult); + } + ); + }); } async revokeTokens( diff --git a/src/common/lib/client/restchannel.ts b/src/common/lib/client/restchannel.ts index 31b2b39fd7..cb201c3925 100644 --- a/src/common/lib/client/restchannel.ts +++ b/src/common/lib/client/restchannel.ts @@ -13,7 +13,6 @@ import ErrorInfo from '../types/errorinfo'; import { PaginatedResult } from './paginatedresource'; import Resource, { ResourceCallback } from './resource'; import { ChannelOptions } from '../../types/channel'; -import { PaginatedResultCallback, StandardCallback } from '../../types/utils'; import BaseRest from './baseclient'; import * as API from '../../../../ably'; import Defaults, { normaliseChannelOptions } from '../util/defaults'; @@ -46,46 +45,29 @@ class RestChannel { this.channelOptions = normaliseChannelOptions(this.client._Crypto ?? null, options); } - history( - params: RestHistoryParams | null, - callback: PaginatedResultCallback - ): Promise> | void { + async history(params: RestHistoryParams | null): Promise> { Logger.logAction(Logger.LOG_MICRO, 'RestChannel.history()', 'channel = ' + this.name); - /* params and callback are optional; see if params contains the callback */ - if (callback === undefined) { - if (typeof params == 'function') { - callback = params; - params = null; - } else { - return Utils.promisify(this, 'history', arguments); - } - } - - this.client.rest.channelMixin.history(this, params, callback); + return new Promise((resolve, reject) => { + this.client.rest.channelMixin.history(this, params, (err, result) => (err ? reject(err) : resolve(result))); + }); } - publish(): void | Promise { - const argCount = arguments.length, - first = arguments[0], - second = arguments[1]; - let callback = arguments[argCount - 1]; + async publish(...args: any[]): Promise { + const first = args[0], + second = args[1]; let messages: Array; let params: any; - if (typeof callback !== 'function') { - return Utils.promisify(this, 'publish', arguments); - } - if (typeof first === 'string' || first === null) { /* (name, data, ...) */ messages = [messageFromValues({ name: first, data: second })]; - params = arguments[2]; + params = args[2]; } else if (Utils.isObject(first)) { messages = [messageFromValues(first)]; - params = arguments[1]; + params = args[1]; } else if (Utils.isArray(first)) { messages = messagesFromValuesArray(first); - params = arguments[1]; + params = args[1]; } else { throw new ErrorInfo( 'The single-argument form of publish() expects a message object or an array of message objects', @@ -94,8 +76,8 @@ class RestChannel { ); } - if (typeof params !== 'object' || !params) { - /* No params supplied (so after-message argument is just the callback or undefined) */ + if (!params) { + /* No params supplied */ params = {}; } @@ -114,31 +96,35 @@ class RestChannel { }); } - encodeMessagesArray(messages, this.channelOptions as CipherOptions, (err: Error) => { - if (err) { - callback(err); - return; - } - - /* RSL1i */ - const size = getMessagesSize(messages), - maxMessageSize = options.maxMessageSize; - if (size > maxMessageSize) { - callback( - new ErrorInfo( - 'Maximum size of messages that can be published at once exceeded ( was ' + - size + - ' bytes; limit is ' + - maxMessageSize + - ' bytes)', - 40009, - 400 - ) + return new Promise((resolve, reject) => { + encodeMessagesArray(messages, this.channelOptions as CipherOptions, (err: Error) => { + if (err) { + reject(err); + return; + } + + /* RSL1i */ + const size = getMessagesSize(messages), + maxMessageSize = options.maxMessageSize; + if (size > maxMessageSize) { + reject( + new ErrorInfo( + 'Maximum size of messages that can be published at once exceeded ( was ' + + size + + ' bytes; limit is ' + + maxMessageSize + + ' bytes)', + 40009, + 400 + ) + ); + return; + } + + this._publish(serializeMessage(messages, client._MsgPack, format), headers, params, (err) => + err ? reject(err) : resolve() ); - return; - } - - this._publish(serializeMessage(messages, client._MsgPack, format), headers, params, callback); + }); }); } @@ -159,8 +145,8 @@ class RestChannel { ); } - status(callback?: StandardCallback): void | Promise { - return this.client.rest.channelMixin.status(this, callback); + async status(): Promise { + return this.client.rest.channelMixin.status(this); } } diff --git a/src/common/lib/client/restchannelmixin.ts b/src/common/lib/client/restchannelmixin.ts index 967ef19b0e..609ddf6721 100644 --- a/src/common/lib/client/restchannelmixin.ts +++ b/src/common/lib/client/restchannelmixin.ts @@ -2,7 +2,7 @@ import * as API from '../../../../ably'; import RestChannel from './restchannel'; import RealtimeChannel from './realtimechannel'; import * as Utils from '../util/utils'; -import { PaginatedResultCallback, StandardCallback } from '../../types/utils'; +import { PaginatedResultCallback } from '../../types/utils'; import Message, { fromResponseBody as messageFromResponseBody } from '../types/message'; import Defaults from '../util/defaults'; import PaginatedResource from './paginatedresource'; @@ -15,8 +15,6 @@ export interface RestHistoryParams { limit?: number; } -const noop = function () {}; - export class RestChannelMixin { static basePath(channel: RestChannel | RealtimeChannel) { return '/channels/' + encodeURIComponent(channel.name); @@ -44,17 +42,14 @@ export class RestChannelMixin { }).get(params as Record, callback); } - static status( - channel: RestChannel | RealtimeChannel, - callback?: StandardCallback - ): void | Promise { - if (typeof callback !== 'function') { - return Utils.promisify(this, 'status', [channel]); - } - + static async status(channel: RestChannel | RealtimeChannel): Promise { const format = channel.client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json; const headers = Defaults.defaultPostHeaders(channel.client.options, { format }); - Resource.get(channel.client, this.basePath(channel), headers, {}, format, callback || noop); + return new Promise((resolve, reject) => { + Resource.get(channel.client, this.basePath(channel), headers, {}, format, (err, result) => + err ? reject(err) : resolve(result!) + ); + }); } } diff --git a/src/common/lib/client/restpresence.ts b/src/common/lib/client/restpresence.ts index 8997f53526..ec745dd5bb 100644 --- a/src/common/lib/client/restpresence.ts +++ b/src/common/lib/client/restpresence.ts @@ -3,7 +3,6 @@ import Logger from '../util/logger'; import PaginatedResource, { PaginatedResult } from './paginatedresource'; import PresenceMessage, { fromResponseBody as presenceMessageFromResponseBody } from '../types/presencemessage'; import { CipherOptions } from '../types/message'; -import { PaginatedResultCallback } from '../../types/utils'; import RestChannel from './restchannel'; import Defaults from '../util/defaults'; @@ -14,17 +13,8 @@ class RestPresence { this.channel = channel; } - get(params: any, callback: PaginatedResultCallback): void | Promise { + async get(params: any): Promise> { Logger.logAction(Logger.LOG_MICRO, 'RestPresence.get()', 'channel = ' + this.channel.name); - /* params and callback are optional; see if params contains the callback */ - if (callback === undefined) { - if (typeof params == 'function') { - callback = params; - params = null; - } else { - return Utils.promisify(this, 'get', arguments); - } - } const client = this.channel.client, format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, envelope = this.channel.client.http.supportsLinkHeaders ? undefined : format, @@ -33,28 +23,27 @@ class RestPresence { Utils.mixin(headers, client.options.headers); const options = this.channel.channelOptions; - new PaginatedResource( - client, - this.channel.client.rest.presenceMixin.basePath(this), - headers, - envelope, - async function (body, headers, unpacked) { - return await presenceMessageFromResponseBody( - body as Record[], - options as CipherOptions, - client._MsgPack, - unpacked ? undefined : format - ); - } - ).get(params, callback); + return new Promise((resolve, reject) => { + new PaginatedResource( + client, + this.channel.client.rest.presenceMixin.basePath(this), + headers, + envelope, + async function (body, headers, unpacked) { + return await presenceMessageFromResponseBody( + body as Record[], + options as CipherOptions, + client._MsgPack, + unpacked ? undefined : format + ); + } + ).get(params, (err, result) => (err ? reject(err) : resolve(result))); + }); } - history( - params: any, - callback: PaginatedResultCallback - ): void | Promise> { + async history(params: any): Promise> { Logger.logAction(Logger.LOG_MICRO, 'RestPresence.history()', 'channel = ' + this.channel.name); - return this.channel.client.rest.presenceMixin.history(this, params, callback); + return this.channel.client.rest.presenceMixin.history(this, params); } } diff --git a/src/common/lib/client/restpresencemixin.ts b/src/common/lib/client/restpresencemixin.ts index 296a1ec6ff..e3920a7f34 100644 --- a/src/common/lib/client/restpresencemixin.ts +++ b/src/common/lib/client/restpresencemixin.ts @@ -1,7 +1,6 @@ import RestPresence from './restpresence'; import RealtimePresence from './realtimepresence'; import * as Utils from '../util/utils'; -import { PaginatedResultCallback } from '../../types/utils'; import Defaults from '../util/defaults'; import PaginatedResource, { PaginatedResult } from './paginatedresource'; import PresenceMessage, { fromResponseBody as presenceMessageFromResponseBody } from '../types/presencemessage'; @@ -13,21 +12,10 @@ export class RestPresenceMixin { return RestChannelMixin.basePath(presence.channel) + '/presence'; } - static history( + static async history( presence: RestPresence | RealtimePresence, - params: any, - callback: PaginatedResultCallback - ): void | Promise> { - /* params and callback are optional; see if params contains the callback */ - if (callback === undefined) { - if (typeof params == 'function') { - callback = params; - params = null; - } else { - return Utils.promisify(this, 'history', [presence, params]); - } - } - + params: any + ): Promise> { const client = presence.channel.client, format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, envelope = presence.channel.client.http.supportsLinkHeaders ? undefined : format, @@ -36,17 +24,19 @@ export class RestPresenceMixin { Utils.mixin(headers, client.options.headers); const options = presence.channel.channelOptions; - new PaginatedResource(client, this.basePath(presence) + '/history', headers, envelope, async function ( - body, - headers, - unpacked - ) { - return await presenceMessageFromResponseBody( - body as Record[], - options as CipherOptions, - client._MsgPack, - unpacked ? undefined : format - ); - }).get(params, callback); + return new Promise((resolve, reject) => { + new PaginatedResource(client, this.basePath(presence) + '/history', headers, envelope, async function ( + body, + headers, + unpacked + ) { + return await presenceMessageFromResponseBody( + body as Record[], + options as CipherOptions, + client._MsgPack, + unpacked ? undefined : format + ); + }).get(params, (err, result) => (err ? reject(err) : resolve(result))); + }); } } diff --git a/src/common/lib/util/utils.ts b/src/common/lib/util/utils.ts index 4fea24cc8e..0d68a8b041 100644 --- a/src/common/lib/util/utils.ts +++ b/src/common/lib/util/utils.ts @@ -444,14 +444,6 @@ export const trim = (String.prototype.trim as unknown) return str.replace(/^[\s\uFEFF\xA0]+|[\s\uFEFF\xA0]+$/g, ''); }; -export function promisify(ob: Record, fnName: string, args: IArguments | unknown[]): Promise { - return new Promise(function (resolve, reject) { - ob[fnName](...(args as unknown[]), function (err: Error, res: unknown) { - err ? reject(err) : resolve(res as T); - }); - }); -} - /** * Uses a callback to communicate the result of a `Promise`. The first argument passed to the callback will be either an error (when the promise is rejected) or `null` (when the promise is fulfilled). In the case where the promise is fulfilled, the resulting value will be passed to the callback as a second argument. */ From 1ef2b2bc8d6cd67e7440e31bd013e39bd0f2cca9 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 14:45:07 -0300 Subject: [PATCH 21/44] Convert Auth._forceNewToken to use promises --- src/common/lib/client/auth.ts | 76 +++++++++---------- src/common/lib/transport/connectionmanager.ts | 4 +- 2 files changed, 40 insertions(+), 40 deletions(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index 0ca538bed7..4a7ff7e47b 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -251,44 +251,42 @@ class Auth { throw new ErrorInfo('Unable to update auth options with incompatible key', 40102, 401); } - return new Promise((resolve, reject) => { - this._forceNewToken( - tokenParams ?? null, - authOptions ?? null, - (err: ErrorInfo, tokenDetails: API.TokenDetails) => { - if (err) { - if ((this.client as BaseRealtime).connection && err.statusCode === HttpStatusCodes.Forbidden) { - /* Per RSA4d & RSA4d1, if the auth server explicitly repudiates our right to - * stay connecticed by returning a 403, we actively disconnect the connection - * even though we may well still have time left in the old token. */ - (this.client as BaseRealtime).connection.connectionManager.actOnErrorFromAuthorize(err); - } - reject(err); - return; - } - - /* RTC8 - * - When authorize called by an end user and have a realtime connection, - * don't call back till new token has taken effect. - * - Use this.client.connection as a proxy for (this.client instanceof BaseRealtime), - * which doesn't work in node as BaseRealtime isn't part of the vm context for Rest clients */ - if (isRealtime(this.client)) { - this.client.connection.connectionManager.onAuthUpdated( - tokenDetails, - (err: unknown, tokenDetails?: API.TokenDetails) => (err ? reject(err) : resolve(tokenDetails!)) - ); - } else { - resolve(tokenDetails); - } - } - ); - }); + try { + let tokenDetails = await this._forceNewToken(tokenParams ?? null, authOptions ?? null); + + /* RTC8 + * - When authorize called by an end user and have a realtime connection, + * don't call back till new token has taken effect. + * - Use this.client.connection as a proxy for (this.client instanceof BaseRealtime), + * which doesn't work in node as BaseRealtime isn't part of the vm context for Rest clients */ + if (isRealtime(this.client)) { + return new Promise((resolve, reject) => { + (this.client as BaseRealtime).connection.connectionManager.onAuthUpdated( + tokenDetails, + (err: unknown, tokenDetails?: API.TokenDetails) => (err ? reject(err) : resolve(tokenDetails!)) + ); + }); + } else { + return tokenDetails; + } + } catch (err) { + if ((this.client as BaseRealtime).connection && (err as ErrorInfo).statusCode === HttpStatusCodes.Forbidden) { + /* Per RSA4d & RSA4d1, if the auth server explicitly repudiates our right to + * stay connecticed by returning a 403, we actively disconnect the connection + * even though we may well still have time left in the old token. */ + (this.client as BaseRealtime).connection.connectionManager.actOnErrorFromAuthorize(err as ErrorInfo); + } + throw err; + } } /* For internal use, eg by connectionManager - useful when want to call back * as soon as we have the new token, rather than waiting for it to take * effect on the connection as #authorize does */ - _forceNewToken(tokenParams: API.TokenParams | null, authOptions: API.AuthOptions | null, callback: Function) { + async _forceNewToken( + tokenParams: API.TokenParams | null, + authOptions: API.AuthOptions | null + ): Promise { /* get rid of current token even if still valid */ this.tokenDetails = null; @@ -299,11 +297,13 @@ class Auth { logAndValidateTokenAuthMethod(this.authOptions); - this._ensureValidAuthCredentials(true, (err: ErrorInfo | null, tokenDetails?: API.TokenDetails) => { - /* RSA10g */ - delete this.tokenParams.timestamp; - delete this.authOptions.queryTime; - callback(err, tokenDetails); + return new Promise((resolve, reject) => { + this._ensureValidAuthCredentials(true, (err: ErrorInfo | null, tokenDetails?: API.TokenDetails) => { + /* RSA10g */ + delete this.tokenParams.timestamp; + delete this.authOptions.queryTime; + err ? reject(err) : resolve(tokenDetails!); + }); }); } diff --git a/src/common/lib/transport/connectionmanager.ts b/src/common/lib/transport/connectionmanager.ts index 04c8bc798b..4eafaa58a3 100644 --- a/src/common/lib/transport/connectionmanager.ts +++ b/src/common/lib/transport/connectionmanager.ts @@ -533,7 +533,7 @@ class ConnectionManager extends EventEmitter { ) { this.errorReason = wrappedErr.error; /* re-get a token and try again */ - this.realtime.auth._forceNewToken(null, null, (err: ErrorInfo) => { + Utils.whenPromiseSettles(this.realtime.auth._forceNewToken(null, null), (err: ErrorInfo | null) => { if (err) { this.actOnErrorFromAuthorize(err); return; @@ -1504,7 +1504,7 @@ class ConnectionManager extends EventEmitter { }; if (this.errorReason && Auth.isTokenErr(this.errorReason as ErrorInfo)) { /* Force a refetch of a new token */ - auth._forceNewToken(null, null, authCb); + Utils.whenPromiseSettles(auth._forceNewToken(null, null), authCb); } else { auth._ensureValidAuthCredentials(false, authCb); } From ffea1831828306a4e15e1ec0bb89a0cb8360b9ae Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 14:59:30 -0300 Subject: [PATCH 22/44] Convert Auth.getAuthParams to use promises --- src/common/lib/client/auth.ts | 24 +++--- src/common/lib/client/resource.ts | 11 ++- src/common/lib/transport/comettransport.ts | 4 +- .../lib/transport/websockettransport.ts | 85 ++++++++++--------- 4 files changed, 66 insertions(+), 58 deletions(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index 4a7ff7e47b..aaa29e3c5c 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -765,18 +765,20 @@ class Auth { * Get the auth query params to use for a websocket connection, * based on the current auth parameters */ - getAuthParams(callback: Function) { - if (this.method == 'basic') callback(null, { key: this.key }); + async getAuthParams(): Promise> { + if (this.method == 'basic') return { key: this.key! }; else - this._ensureValidAuthCredentials(false, function (err: ErrorInfo | null, tokenDetails?: API.TokenDetails) { - if (err) { - callback(err); - return; - } - if (!tokenDetails) { - throw new Error('Auth.getAuthParams(): _ensureValidAuthCredentials returned no error or tokenDetails'); - } - callback(null, { access_token: tokenDetails.token }); + return new Promise((resolve, reject) => { + this._ensureValidAuthCredentials(false, function (err: ErrorInfo | null, tokenDetails?: API.TokenDetails) { + if (err) { + reject(err); + return; + } + if (!tokenDetails) { + throw new Error('Auth.getAuthParams(): _ensureValidAuthCredentials returned no error or tokenDetails'); + } + resolve({ access_token: tokenDetails.token }); + }); }); } diff --git a/src/common/lib/client/resource.ts b/src/common/lib/client/resource.ts index 69de333a3b..9457cf5ec6 100644 --- a/src/common/lib/client/resource.ts +++ b/src/common/lib/client/resource.ts @@ -26,10 +26,13 @@ function withAuthDetails( else opCallback(Utils.mixin(authHeaders, headers), params); }); } else { - client.auth.getAuthParams(function (err: Error, authParams: Record) { - if (err) errCallback(err); - else opCallback(headers, Utils.mixin(authParams, params)); - }); + Utils.whenPromiseSettles( + client.auth.getAuthParams(), + function (err: Error | null, authParams?: Record) { + if (err) errCallback(err); + else opCallback(headers, Utils.mixin(authParams!, params)); + } + ); } } diff --git a/src/common/lib/transport/comettransport.ts b/src/common/lib/transport/comettransport.ts index 02420ea393..88e094cd8e 100644 --- a/src/common/lib/transport/comettransport.ts +++ b/src/common/lib/transport/comettransport.ts @@ -84,7 +84,7 @@ abstract class CometTransport extends Transport { this.baseUri = cometScheme + host + ':' + port + '/comet/'; const connectUri = this.baseUri + 'connect'; Logger.logAction(Logger.LOG_MINOR, 'CometTransport.connect()', 'uri: ' + connectUri); - this.auth.getAuthParams((err: Error, authParams: Record) => { + Utils.whenPromiseSettles(this.auth.getAuthParams(), (err: Error | null, authParams?: Record) => { if (err) { this.disconnect(err); return; @@ -93,7 +93,7 @@ abstract class CometTransport extends Transport { return; } this.authParams = authParams; - const connectParams = this.params.getConnectParams(authParams); + const connectParams = this.params.getConnectParams(authParams!); if ('stream' in connectParams) this.stream = connectParams.stream; Logger.logAction( Logger.LOG_MINOR, diff --git a/src/common/lib/transport/websockettransport.ts b/src/common/lib/transport/websockettransport.ts index d56848ec6b..991804491f 100644 --- a/src/common/lib/transport/websockettransport.ts +++ b/src/common/lib/transport/websockettransport.ts @@ -54,49 +54,52 @@ class WebSocketTransport extends Transport { const wsScheme = options.tls ? 'wss://' : 'ws://'; const wsUri = wsScheme + this.wsHost + ':' + Defaults.getPort(options) + '/'; Logger.logAction(Logger.LOG_MINOR, 'WebSocketTransport.connect()', 'uri: ' + wsUri); - this.auth.getAuthParams(function (err: ErrorInfo, authParams: Record) { - if (self.isDisposed) { - return; - } - let paramStr = ''; - for (const param in authParams) paramStr += ' ' + param + ': ' + authParams[param] + ';'; - Logger.logAction(Logger.LOG_MINOR, 'WebSocketTransport.connect()', 'authParams:' + paramStr + ' err: ' + err); - if (err) { - self.disconnect(err); - return; - } - const connectParams = params.getConnectParams(authParams); - try { - const wsConnection = (self.wsConnection = self.createWebSocket(wsUri, connectParams)); - wsConnection.binaryType = Platform.Config.binaryType; - wsConnection.onopen = function () { - self.onWsOpen(); - }; - wsConnection.onclose = function (ev: CloseEvent) { - self.onWsClose(ev); - }; - wsConnection.onmessage = function (ev: MessageEvent) { - self.onWsData(ev.data); - }; - wsConnection.onerror = function (ev: Event) { - self.onWsError(ev as ErrorEvent); - }; - if (isNodeWebSocket(wsConnection)) { - /* node; browsers currently don't have a general eventemitter and can't detect - * pings. Also, no need to reply with a pong explicitly, ws lib handles that */ - wsConnection.on('ping', function () { - self.onActivity(); - }); + Utils.whenPromiseSettles( + this.auth.getAuthParams(), + function (err: ErrorInfo | null, authParams?: Record) { + if (self.isDisposed) { + return; + } + let paramStr = ''; + for (const param in authParams) paramStr += ' ' + param + ': ' + authParams[param] + ';'; + Logger.logAction(Logger.LOG_MINOR, 'WebSocketTransport.connect()', 'authParams:' + paramStr + ' err: ' + err); + if (err) { + self.disconnect(err); + return; + } + const connectParams = params.getConnectParams(authParams!); + try { + const wsConnection = (self.wsConnection = self.createWebSocket(wsUri, connectParams)); + wsConnection.binaryType = Platform.Config.binaryType; + wsConnection.onopen = function () { + self.onWsOpen(); + }; + wsConnection.onclose = function (ev: CloseEvent) { + self.onWsClose(ev); + }; + wsConnection.onmessage = function (ev: MessageEvent) { + self.onWsData(ev.data); + }; + wsConnection.onerror = function (ev: Event) { + self.onWsError(ev as ErrorEvent); + }; + if (isNodeWebSocket(wsConnection)) { + /* node; browsers currently don't have a general eventemitter and can't detect + * pings. Also, no need to reply with a pong explicitly, ws lib handles that */ + wsConnection.on('ping', function () { + self.onActivity(); + }); + } + } catch (e) { + Logger.logAction( + Logger.LOG_ERROR, + 'WebSocketTransport.connect()', + 'Unexpected exception creating websocket: err = ' + ((e as Error).stack || (e as Error).message) + ); + self.disconnect(e as Error); } - } catch (e) { - Logger.logAction( - Logger.LOG_ERROR, - 'WebSocketTransport.connect()', - 'Unexpected exception creating websocket: err = ' + ((e as Error).stack || (e as Error).message) - ); - self.disconnect(e as Error); } - }); + ); } send(message: ProtocolMessage) { From c2f02a0ebb6b9d01bb1facfaf29d7c287a5637bf Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 15:00:53 -0300 Subject: [PATCH 23/44] Convert Auth.getAuthHeaders to use promises --- src/common/lib/client/auth.ts | 24 +++++++++++++----------- src/common/lib/client/resource.ts | 11 +++++++---- test/rest/auth.test.js | 20 ++------------------ 3 files changed, 22 insertions(+), 33 deletions(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index aaa29e3c5c..1519ade7d3 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -786,19 +786,21 @@ class Auth { * Get the authorization header to use for a REST or comet request, * based on the current auth parameters */ - getAuthHeaders(callback: Function) { + async getAuthHeaders(): Promise> { if (this.method == 'basic') { - callback(null, { authorization: 'Basic ' + this.basicKey }); + return { authorization: 'Basic ' + this.basicKey }; } else { - this._ensureValidAuthCredentials(false, function (err: ErrorInfo | null, tokenDetails?: API.TokenDetails) { - if (err) { - callback(err); - return; - } - if (!tokenDetails) { - throw new Error('Auth.getAuthParams(): _ensureValidAuthCredentials returned no error or tokenDetails'); - } - callback(null, { authorization: 'Bearer ' + Utils.toBase64(tokenDetails.token) }); + return new Promise((resolve, reject) => { + this._ensureValidAuthCredentials(false, function (err: ErrorInfo | null, tokenDetails?: API.TokenDetails) { + if (err) { + reject(err); + return; + } + if (!tokenDetails) { + throw new Error('Auth.getAuthParams(): _ensureValidAuthCredentials returned no error or tokenDetails'); + } + resolve({ authorization: 'Bearer ' + Utils.toBase64(tokenDetails.token) }); + }); }); } } diff --git a/src/common/lib/client/resource.ts b/src/common/lib/client/resource.ts index 9457cf5ec6..f3c808ebc8 100644 --- a/src/common/lib/client/resource.ts +++ b/src/common/lib/client/resource.ts @@ -21,10 +21,13 @@ function withAuthDetails( opCallback: Function ) { if (client.http.supportsAuthHeaders) { - client.auth.getAuthHeaders(function (err: Error, authHeaders: Record) { - if (err) errCallback(err); - else opCallback(Utils.mixin(authHeaders, headers), params); - }); + Utils.whenPromiseSettles( + client.auth.getAuthHeaders(), + function (err: Error | null, authHeaders?: Record) { + if (err) errCallback(err); + else opCallback(Utils.mixin(authHeaders!, headers), params); + } + ); } else { Utils.whenPromiseSettles( client.auth.getAuthParams(), diff --git a/test/rest/auth.test.js b/test/rest/auth.test.js index c1aed009bd..0824bcca8d 100644 --- a/test/rest/auth.test.js +++ b/test/rest/auth.test.js @@ -130,15 +130,7 @@ define(['chai', 'shared_helper', 'async', 'globals'], function (chai, helper, as }); it('Token generation with explicit auth', async function () { - var authHeaders = await new Promise((resolve, reject) => { - rest.auth.getAuthHeaders(function (err, authHeaders) { - if (err) { - reject(err); - return; - } - resolve(authHeaders); - }); - }); + const authHeaders = await rest.auth.getAuthHeaders(); rest.auth.authOptions.requestHeaders = authHeaders; var tokenDetails = await rest.auth.requestToken(); delete rest.auth.authOptions.requestHeaders; @@ -149,15 +141,7 @@ define(['chai', 'shared_helper', 'async', 'globals'], function (chai, helper, as }); it('Token generation with explicit auth, different key', async function () { - var authHeaders = await new Promise((resolve, reject) => { - rest.auth.getAuthHeaders(function (err, authHeaders) { - if (err) { - reject(err); - return; - } - resolve(authHeaders); - }); - }); + const authHeaders = await rest.auth.getAuthHeaders(); var testKeyOpts = { key: helper.getTestApp().keys[1].keyStr }; var testCapability = JSON.parse(helper.getTestApp().keys[1].capability); var tokenDetails = await rest.auth.requestToken(null, testKeyOpts); From 9dd15155961f62d778f35e6111792c3f95cdf7c2 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 15:07:59 -0300 Subject: [PATCH 24/44] Convert Auth.getTimestamp to use promises --- src/common/lib/client/auth.ts | 68 +++++++++++++++-------------------- 1 file changed, 28 insertions(+), 40 deletions(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index 1519ade7d3..726eee8690 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -722,43 +722,31 @@ class Auth { ttl = tokenParams.ttl || '', capability = tokenParams.capability || ''; - return new Promise((resolve, reject) => { - ((authoriseCb) => { - if (request.timestamp) { - authoriseCb(); - return; - } - this.getTimestamp(authOptions && authOptions.queryTime, function (err?: ErrorInfo | null, time?: number) { - if (err) { - reject(err); - return; - } - request.timestamp = time; - authoriseCb(); - }); - })(function () { - /* nonce */ - /* NOTE: there is no expectation that the client - * specifies the nonce; this is done by the library - * However, this can be overridden by the client - * simply for testing purposes. */ - const nonce = request.nonce || (request.nonce = random()), - timestamp = request.timestamp; - - const signText = - request.keyName + '\n' + ttl + '\n' + capability + '\n' + clientId + '\n' + timestamp + '\n' + nonce + '\n'; - - /* mac */ - /* NOTE: there is no expectation that the client - * specifies the mac; this is done by the library - * However, this can be overridden by the client - * simply for testing purposes. */ - request.mac = request.mac || hmac(signText, keySecret); - - Logger.logAction(Logger.LOG_MINOR, 'Auth.getTokenRequest()', 'generated signed request'); - resolve(request as API.TokenRequest); - }); - }); + if (!request.timestamp) { + request.timestamp = await this.getTimestamp(authOptions && authOptions.queryTime); + } + + /* nonce */ + /* NOTE: there is no expectation that the client + * specifies the nonce; this is done by the library + * However, this can be overridden by the client + * simply for testing purposes. */ + const nonce = request.nonce || (request.nonce = random()), + timestamp = request.timestamp; + + const signText = + request.keyName + '\n' + ttl + '\n' + capability + '\n' + clientId + '\n' + timestamp + '\n' + nonce + '\n'; + + /* mac */ + /* NOTE: there is no expectation that the client + * specifies the mac; this is done by the library + * However, this can be overridden by the client + * simply for testing purposes. */ + request.mac = request.mac || hmac(signText, keySecret); + + Logger.logAction(Logger.LOG_MINOR, 'Auth.getTokenRequest()', 'generated signed request'); + + return request as API.TokenRequest; } /** @@ -811,11 +799,11 @@ class Auth { * The server time offset from the local time is stored so that * only one request to the server to get the time is ever needed */ - getTimestamp(queryTime: boolean, callback: StandardCallback): void { + async getTimestamp(queryTime: boolean): Promise { if (!this.isTimeOffsetSet() && (queryTime || this.authOptions.queryTime)) { - Utils.whenPromiseSettles(this.client.time(), callback); + return this.client.time(); } else { - callback(null, this.getTimestampUsingOffset()); + return this.getTimestampUsingOffset(); } } From 73b30af292b05fb40409bcc070a72f7b1a093055 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 15:28:53 -0300 Subject: [PATCH 25/44] Convert Auth._ensureValidAuthCredentials to use promises Resolves #1527. --- src/common/lib/client/auth.ts | 127 ++++++++---------- src/common/lib/transport/connectionmanager.ts | 2 +- 2 files changed, 58 insertions(+), 71 deletions(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index 726eee8690..7cd89f0acb 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -21,7 +21,6 @@ type TokenRevocationFailureResult = API.TokenRevocationFailureResult; type TokenRevocationResult = BatchResult; const MAX_TOKEN_LENGTH = Math.pow(2, 17); -function noop() {} function random() { return ('000000' + Math.floor(Math.random() * 1e16)).slice(-16); } @@ -297,14 +296,13 @@ class Auth { logAndValidateTokenAuthMethod(this.authOptions); - return new Promise((resolve, reject) => { - this._ensureValidAuthCredentials(true, (err: ErrorInfo | null, tokenDetails?: API.TokenDetails) => { - /* RSA10g */ - delete this.tokenParams.timestamp; - delete this.authOptions.queryTime; - err ? reject(err) : resolve(tokenDetails!); - }); - }); + try { + return this._ensureValidAuthCredentials(true); + } finally { + /* RSA10g */ + delete this.tokenParams.timestamp; + delete this.authOptions.queryTime; + } } /** @@ -755,19 +753,13 @@ class Auth { */ async getAuthParams(): Promise> { if (this.method == 'basic') return { key: this.key! }; - else - return new Promise((resolve, reject) => { - this._ensureValidAuthCredentials(false, function (err: ErrorInfo | null, tokenDetails?: API.TokenDetails) { - if (err) { - reject(err); - return; - } - if (!tokenDetails) { - throw new Error('Auth.getAuthParams(): _ensureValidAuthCredentials returned no error or tokenDetails'); - } - resolve({ access_token: tokenDetails.token }); - }); - }); + else { + let tokenDetails = await this._ensureValidAuthCredentials(false); + if (!tokenDetails) { + throw new Error('Auth.getAuthParams(): _ensureValidAuthCredentials returned no error or tokenDetails'); + } + return { access_token: tokenDetails.token }; + } } /** @@ -778,18 +770,11 @@ class Auth { if (this.method == 'basic') { return { authorization: 'Basic ' + this.basicKey }; } else { - return new Promise((resolve, reject) => { - this._ensureValidAuthCredentials(false, function (err: ErrorInfo | null, tokenDetails?: API.TokenDetails) { - if (err) { - reject(err); - return; - } - if (!tokenDetails) { - throw new Error('Auth.getAuthParams(): _ensureValidAuthCredentials returned no error or tokenDetails'); - } - resolve({ authorization: 'Bearer ' + Utils.toBase64(tokenDetails.token) }); - }); - }); + const tokenDetails = await this._ensureValidAuthCredentials(false); + if (!tokenDetails) { + throw new Error('Auth.getAuthParams(): _ensureValidAuthCredentials returned no error or tokenDetails'); + } + return { authorization: 'Bearer ' + Utils.toBase64(tokenDetails.token) }; } } @@ -859,65 +844,67 @@ class Auth { /* @param forceSupersede: force a new token request even if there's one in * progress, making all pending callbacks wait for the new one */ - _ensureValidAuthCredentials( - forceSupersede: boolean, - callback: (err: ErrorInfo | null, token?: API.TokenDetails) => void - ) { + async _ensureValidAuthCredentials(forceSupersede: boolean): Promise { const token = this.tokenDetails; if (token) { if (this._tokenClientIdMismatch(token.clientId)) { /* 403 to trigger a permanently failed client - RSA15c */ - callback( - new ErrorInfo( - 'Mismatch between clientId in token (' + token.clientId + ') and current clientId (' + this.clientId + ')', - 40102, - 403 - ) + throw new ErrorInfo( + 'Mismatch between clientId in token (' + token.clientId + ') and current clientId (' + this.clientId + ')', + 40102, + 403 ); - return; } /* RSA4b1 -- if we have a server time offset set already, we can * automatically remove expired tokens. Else just use the cached token. If it is * expired Ably will tell us and we'll discard it then. */ if (!this.isTimeOffsetSet() || !token.expires || token.expires >= this.getTimestampUsingOffset()) { Logger.logAction(Logger.LOG_MINOR, 'Auth.getToken()', 'using cached token; expires = ' + token.expires); - callback(null, token); - return; + return token; } /* expired, so remove and fallthrough to getting a new one */ Logger.logAction(Logger.LOG_MINOR, 'Auth.getToken()', 'deleting expired token'); this.tokenDetails = null; } - (this.waitingForTokenRequest || (this.waitingForTokenRequest = Multicaster.create())).push(callback); + const promise = ( + this.waitingForTokenRequest || (this.waitingForTokenRequest = Multicaster.create()) + ).createPromise(); if (this.currentTokenRequestId !== null && !forceSupersede) { - return; + return promise; } /* Request a new token */ const tokenRequestId = (this.currentTokenRequestId = getTokenRequestId()); - Utils.whenPromiseSettles( - this.requestToken(this.tokenParams, this.authOptions), - (err: ErrorInfo | null, tokenResponse?: API.TokenDetails) => { - if ((this.currentTokenRequestId as number) > tokenRequestId) { - Logger.logAction( - Logger.LOG_MINOR, - 'Auth._ensureValidAuthCredentials()', - 'Discarding token request response; overtaken by newer one' - ); - return; - } - this.currentTokenRequestId = null; - const callbacks = this.waitingForTokenRequest || noop; - this.waitingForTokenRequest = null; - if (err) { - callbacks(err); - return; - } - callbacks(null, (this.tokenDetails = tokenResponse)); - } - ); + + let tokenResponse: API.TokenDetails, + caughtError: ErrorInfo | null = null; + try { + tokenResponse = await this.requestToken(this.tokenParams, this.authOptions); + } catch (err) { + caughtError = err as ErrorInfo; + } + + if ((this.currentTokenRequestId as number) > tokenRequestId) { + Logger.logAction( + Logger.LOG_MINOR, + 'Auth._ensureValidAuthCredentials()', + 'Discarding token request response; overtaken by newer one' + ); + return promise; + } + + this.currentTokenRequestId = null; + const multicaster = this.waitingForTokenRequest; + this.waitingForTokenRequest = null; + if (caughtError) { + multicaster?.rejectAll(caughtError); + return promise; + } + multicaster?.resolveAll((this.tokenDetails = tokenResponse!)); + + return promise; } /* User-set: check types, '*' is disallowed, throw any errors */ diff --git a/src/common/lib/transport/connectionmanager.ts b/src/common/lib/transport/connectionmanager.ts index 4eafaa58a3..1e34a0852c 100644 --- a/src/common/lib/transport/connectionmanager.ts +++ b/src/common/lib/transport/connectionmanager.ts @@ -1506,7 +1506,7 @@ class ConnectionManager extends EventEmitter { /* Force a refetch of a new token */ Utils.whenPromiseSettles(auth._forceNewToken(null, null), authCb); } else { - auth._ensureValidAuthCredentials(false, authCb); + Utils.whenPromiseSettles(auth._ensureValidAuthCredentials(false), authCb); } } } From 5b3338186a6a27ff0107fd40f459a35dbd7e1a0b Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 15:53:09 -0300 Subject: [PATCH 26/44] Convert PaginatedResource.handlePage to use promises --- src/common/lib/client/paginatedresource.ts | 62 ++++++++++------------ 1 file changed, 27 insertions(+), 35 deletions(-) diff --git a/src/common/lib/client/paginatedresource.ts b/src/common/lib/client/paginatedresource.ts index 4ff93711cf..f009cd2dab 100644 --- a/src/common/lib/client/paginatedresource.ts +++ b/src/common/lib/client/paginatedresource.ts @@ -29,7 +29,7 @@ function parseRelLinks(linkHeader: string | Array) { 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')); @@ -67,7 +67,7 @@ class PaginatedResource { params, this.envelope, (err, body, headers, unpacked, statusCode) => { - this.handlePage(err, body, headers, unpacked, statusCode, callback); + Utils.whenPromiseSettles(this.handlePage(err, body, headers, unpacked, statusCode), callback); } ); } @@ -80,7 +80,7 @@ class PaginatedResource { params, this.envelope, (err, body, headers, unpacked, statusCode) => { - this.handlePage(err, body, headers, unpacked, statusCode, callback); + Utils.whenPromiseSettles(this.handlePage(err, body, headers, unpacked, statusCode), callback); } ); } @@ -95,7 +95,7 @@ class PaginatedResource { this.envelope, (err, responseBody, headers, unpacked, statusCode) => { if (callback) { - this.handlePage(err, responseBody, headers, unpacked, statusCode, callback); + Utils.whenPromiseSettles(this.handlePage(err, responseBody, headers, unpacked, statusCode), callback); } } ); @@ -111,7 +111,7 @@ class PaginatedResource { this.envelope, (err, responseBody, headers, unpacked, statusCode) => { if (callback) { - this.handlePage(err, responseBody, headers, unpacked, statusCode, callback); + Utils.whenPromiseSettles(this.handlePage(err, responseBody, headers, unpacked, statusCode), callback); } } ); @@ -127,55 +127,47 @@ class PaginatedResource { this.envelope, (err, responseBody, headers, unpacked, statusCode) => { if (callback) { - this.handlePage(err, responseBody, headers, unpacked, statusCode, callback); + Utils.whenPromiseSettles(this.handlePage(err, responseBody, headers, unpacked, statusCode), callback); } } ); } - handlePage( + async handlePage( err: IPartialErrorInfo | null, body: unknown, headers: RequestCallbackHeaders | undefined, unpacked: boolean | undefined, - statusCode: number | undefined, - callback: PaginatedResultCallback - ): void { + statusCode: number | undefined + ): Promise> { if (err && returnErrOnly(err, body, this.useHttpPaginatedResponse)) { Logger.logAction( Logger.LOG_ERROR, 'PaginatedResource.handlePage()', 'Unexpected error getting resource: err = ' + Utils.inspectError(err) ); - callback?.(err); - return; + throw err; } - const handleBody = async () => { - let items, linkHeader, relParams; + 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; - } - - if (headers && (linkHeader = headers['Link'] || headers['link'])) { - relParams = parseRelLinks(linkHeader); - } + 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; + } - if (this.useHttpPaginatedResponse) { - return new HttpPaginatedResponse(this, items, headers || {}, statusCode as number, relParams, err); - } else { - return new PaginatedResult(this, items, relParams); - } - }; + if (headers && (linkHeader = headers['Link'] || headers['link'])) { + relParams = parseRelLinks(linkHeader); + } - handleBody() - .then((result) => callback(null, result)) - .catch((err) => callback(err, null)); + if (this.useHttpPaginatedResponse) { + return new HttpPaginatedResponse(this, items, headers || {}, statusCode as number, relParams, err); + } else { + return new PaginatedResult(this, items, relParams); + } } } @@ -238,7 +230,7 @@ export class PaginatedResult { params, res.envelope, function (err, body, headers, unpacked, statusCode) { - res.handlePage(err, body, headers, unpacked, statusCode, callback); + Utils.whenPromiseSettles(res.handlePage(err, body, headers, unpacked, statusCode), callback); } ); } From 0d2d90c92e85bdb2ea2e193db97fa80c8ee55e3d Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 15:57:51 -0300 Subject: [PATCH 27/44] Convert PaginatedResult.get to use promises --- src/common/lib/client/paginatedresource.ts | 36 ++++++++++------------ 1 file changed, 16 insertions(+), 20 deletions(-) diff --git a/src/common/lib/client/paginatedresource.ts b/src/common/lib/client/paginatedresource.ts index f009cd2dab..34d4686d6e 100644 --- a/src/common/lib/client/paginatedresource.ts +++ b/src/common/lib/client/paginatedresource.ts @@ -188,23 +188,17 @@ export class PaginatedResult { if (relParams) { if ('first' in relParams) { this.first = async function () { - return new Promise((resolve, reject) => { - self.get(relParams.first, (err, result) => (err ? reject(err) : resolve(result))); - }); + return self.get(relParams.first); }; } if ('current' in relParams) { this.current = async function () { - return new Promise((resolve, reject) => { - self.get(relParams.current, (err, result) => (err ? reject(err) : resolve(result))); - }); + return self.get(relParams.current); }; } this.next = async function () { if ('next' in relParams) { - return new Promise((resolve, reject) => { - self.get(relParams.next, (err, result) => (err ? reject(err) : resolve(result))); - }); + return self.get(relParams.next); } else { return null; } @@ -221,18 +215,20 @@ export class PaginatedResult { /* 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): void { + async get(params: any): Promise> { const res = this.resource; - Resource.get( - res.client, - res.path, - res.headers, - params, - res.envelope, - function (err, body, headers, unpacked, statusCode) { - Utils.whenPromiseSettles(res.handlePage(err, body, headers, unpacked, statusCode), callback); - } - ); + 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)); + } + ); + }); } } From 96655afe58690328886623aca7f369838b9cc247 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 16:14:43 -0300 Subject: [PATCH 28/44] Convert PaginatedResource request methods to use promises --- src/common/lib/client/paginatedresource.ts | 127 +++++++++++---------- src/common/lib/client/push.ts | 68 +++++------ src/common/lib/client/rest.ts | 32 +++--- src/common/lib/client/restchannelmixin.ts | 22 ++-- src/common/lib/client/restpresence.ts | 30 +++-- src/common/lib/client/restpresencemixin.ts | 26 ++--- 6 files changed, 151 insertions(+), 154 deletions(-) diff --git a/src/common/lib/client/paginatedresource.ts b/src/common/lib/client/paginatedresource.ts index 34d4686d6e..2be9eb1325 100644 --- a/src/common/lib/client/paginatedresource.ts +++ b/src/common/lib/client/paginatedresource.ts @@ -2,7 +2,6 @@ import * as Utils from '../util/utils'; import Logger from '../util/logger'; import Resource from './resource'; import { IPartialErrorInfo } from '../types/errorinfo'; -import { PaginatedResultCallback } from '../../types/utils'; import BaseClient from './baseclient'; import { RequestBody, RequestCallbackHeaders } from 'common/types/http'; @@ -59,78 +58,82 @@ class PaginatedResource { this.useHttpPaginatedResponse = useHttpPaginatedResponse || false; } - get(params: Record, callback: PaginatedResultCallback): void { - Resource.get( - this.client, - this.path, - this.headers, - params, - this.envelope, - (err, body, headers, unpacked, statusCode) => { - Utils.whenPromiseSettles(this.handlePage(err, body, headers, unpacked, statusCode), callback); - } - ); + 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)); + } + ); + }); } - delete(params: Record, callback: PaginatedResultCallback): void { - Resource.delete( - this.client, - this.path, - this.headers, - params, - this.envelope, - (err, body, headers, unpacked, statusCode) => { - Utils.whenPromiseSettles(this.handlePage(err, body, headers, unpacked, statusCode), callback); - } - ); + 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)); + } + ); + }); } - post(params: Record, body: RequestBody | null, callback: PaginatedResultCallback): void { - Resource.post( - this.client, - this.path, - body, - this.headers, - params, - this.envelope, - (err, responseBody, headers, unpacked, statusCode) => { - if (callback) { - Utils.whenPromiseSettles(this.handlePage(err, responseBody, headers, unpacked, statusCode), callback); + async post(params: Record, body: RequestBody | null): 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)); } - } - ); + ); + }); } - put(params: Record, body: RequestBody | null, callback: PaginatedResultCallback): void { - Resource.put( - this.client, - this.path, - body, - this.headers, - params, - this.envelope, - (err, responseBody, headers, unpacked, statusCode) => { - if (callback) { - Utils.whenPromiseSettles(this.handlePage(err, responseBody, headers, unpacked, statusCode), callback); + async put(params: Record, body: RequestBody | null): 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)); } - } - ); + ); + }); } - patch(params: Record, body: RequestBody | null, callback: PaginatedResultCallback): void { - Resource.patch( - this.client, - this.path, - body, - this.headers, - params, - this.envelope, - (err, responseBody, headers, unpacked, statusCode) => { - if (callback) { - Utils.whenPromiseSettles(this.handlePage(err, responseBody, headers, unpacked, statusCode), callback); + async patch(params: Record, body: RequestBody | null): 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)); } - } - ); + ); + }); } async handlePage( diff --git a/src/common/lib/client/push.ts b/src/common/lib/client/push.ts index 3c1b10edfc..e6ab5b3083 100644 --- a/src/common/lib/client/push.ts +++ b/src/common/lib/client/push.ts @@ -136,19 +136,17 @@ class DeviceRegistrations { Utils.mixin(headers, client.options.headers); - return new Promise((resolve, reject) => { - new PaginatedResource(client, '/push/deviceRegistrations', headers, envelope, async function ( - body, - headers, - unpacked - ) { - return DeviceDetails.fromResponseBody( - body as Record[], - client._MsgPack, - unpacked ? undefined : format - ); - }).get(params, (err, result) => (err ? reject(err) : resolve(result))); - }); + return new PaginatedResource(client, '/push/deviceRegistrations', headers, envelope, async function ( + body, + headers, + unpacked + ) { + return DeviceDetails.fromResponseBody( + body as Record[], + client._MsgPack, + unpacked ? undefined : format + ); + }).get(params); } async remove(deviceIdOrDetails: any): Promise { @@ -249,19 +247,17 @@ class ChannelSubscriptions { Utils.mixin(headers, client.options.headers); - return new Promise((resolve, reject) => { - new PaginatedResource(client, '/push/channelSubscriptions', headers, envelope, async function ( - body, - headers, - unpacked - ) { - return PushChannelSubscription.fromResponseBody( - body as Record[], - client._MsgPack, - unpacked ? undefined : format - ); - }).get(params, (err, result) => (err ? reject(err) : resolve(result))); - }); + return new PaginatedResource(client, '/push/channelSubscriptions', headers, envelope, async function ( + body, + headers, + unpacked + ) { + return PushChannelSubscription.fromResponseBody( + body as Record[], + client._MsgPack, + unpacked ? undefined : format + ); + }).get(params); } async removeWhere(params: any): Promise { @@ -293,18 +289,16 @@ class ChannelSubscriptions { if (client.options.pushFullWait) Utils.mixin(params, { fullWait: 'true' }); - return new Promise((resolve, reject) => { - new PaginatedResource(client, '/push/channels', headers, envelope, async function (body, headers, unpacked) { - const parsedBody = ( - !unpacked && format ? Utils.decodeBody(body, client._MsgPack, format) : body - ) as Array; + return new PaginatedResource(client, '/push/channels', headers, envelope, async function (body, headers, unpacked) { + const parsedBody = ( + !unpacked && format ? Utils.decodeBody(body, client._MsgPack, format) : body + ) as Array; - for (let i = 0; i < parsedBody.length; i++) { - parsedBody[i] = String(parsedBody[i]); - } - return parsedBody; - }).get(params, (err, result) => (err ? reject(err) : resolve(result))); - }); + for (let i = 0; i < parsedBody.length; i++) { + parsedBody[i] = String(parsedBody[i]); + } + return parsedBody; + }).get(params); } } diff --git a/src/common/lib/client/rest.ts b/src/common/lib/client/rest.ts index ba445b95d8..b9d7a9d6ce 100644 --- a/src/common/lib/client/rest.ts +++ b/src/common/lib/client/rest.ts @@ -55,13 +55,11 @@ export class Rest { Utils.mixin(headers, this.client.options.headers); - return new Promise((resolve, reject) => { - new PaginatedResource(this.client, '/stats', headers, envelope, function (body, headers, unpacked) { - const statsValues = unpacked ? body : JSON.parse(body as string); - for (let i = 0; i < statsValues.length; i++) statsValues[i] = Stats.fromValues(statsValues[i]); - return statsValues; - }).get(params as Record, (err, result) => (err ? reject(err) : resolve(result))); - }); + return new PaginatedResource(this.client, '/stats', headers, envelope, function (body, headers, unpacked) { + const statsValues = unpacked ? body : JSON.parse(body as string); + for (let i = 0; i < statsValues.length; i++) statsValues[i] = Stats.fromValues(statsValues[i]); + return statsValues; + }).get(params as Record); } async time(params?: RequestParams): Promise { @@ -144,17 +142,15 @@ export class Rest { throw new ErrorInfo('Unsupported method ' + _method, 40500, 405); } - return new Promise((resolve, reject) => { - if (Utils.arrIn(Platform.Http.methodsWithBody, _method)) { - paginatedResource[_method as HttpMethods.Post](params!, body as RequestBody, (err, result) => - err ? reject(err) : resolve(result) - ); - } else { - paginatedResource[_method as HttpMethods.Get | HttpMethods.Delete](params!, (err, result) => - err ? reject(err) : resolve(result) - ); - } - }); + if (Utils.arrIn(Platform.Http.methodsWithBody, _method)) { + return paginatedResource[_method as HttpMethods.Post](params, body as RequestBody) as Promise< + HttpPaginatedResponse + >; + } else { + return paginatedResource[_method as HttpMethods.Get | HttpMethods.Delete](params) as Promise< + HttpPaginatedResponse + >; + } } async batchPublish( diff --git a/src/common/lib/client/restchannelmixin.ts b/src/common/lib/client/restchannelmixin.ts index 609ddf6721..24469980e3 100644 --- a/src/common/lib/client/restchannelmixin.ts +++ b/src/common/lib/client/restchannelmixin.ts @@ -33,13 +33,21 @@ export class RestChannelMixin { Utils.mixin(headers, client.options.headers); const options = channel.channelOptions; - new PaginatedResource(client, this.basePath(channel) + '/messages', headers, envelope, async function ( - body, - headers, - unpacked - ) { - return await messageFromResponseBody(body as Message[], options, client._MsgPack, unpacked ? undefined : format); - }).get(params as Record, callback); + Utils.whenPromiseSettles( + new PaginatedResource(client, this.basePath(channel) + '/messages', headers, envelope, async function ( + body, + headers, + unpacked + ) { + return await messageFromResponseBody( + body as Message[], + options, + client._MsgPack, + unpacked ? undefined : format + ); + }).get(params as Record), + callback + ); } static async status(channel: RestChannel | RealtimeChannel): Promise { diff --git a/src/common/lib/client/restpresence.ts b/src/common/lib/client/restpresence.ts index ec745dd5bb..0bbdc79f82 100644 --- a/src/common/lib/client/restpresence.ts +++ b/src/common/lib/client/restpresence.ts @@ -23,22 +23,20 @@ class RestPresence { Utils.mixin(headers, client.options.headers); const options = this.channel.channelOptions; - return new Promise((resolve, reject) => { - new PaginatedResource( - client, - this.channel.client.rest.presenceMixin.basePath(this), - headers, - envelope, - async function (body, headers, unpacked) { - return await presenceMessageFromResponseBody( - body as Record[], - options as CipherOptions, - client._MsgPack, - unpacked ? undefined : format - ); - } - ).get(params, (err, result) => (err ? reject(err) : resolve(result))); - }); + return new PaginatedResource( + client, + this.channel.client.rest.presenceMixin.basePath(this), + headers, + envelope, + async function (body, headers, unpacked) { + return await presenceMessageFromResponseBody( + body as Record[], + options as CipherOptions, + client._MsgPack, + unpacked ? undefined : format + ); + } + ).get(params); } async history(params: any): Promise> { diff --git a/src/common/lib/client/restpresencemixin.ts b/src/common/lib/client/restpresencemixin.ts index e3920a7f34..0a1aacea0f 100644 --- a/src/common/lib/client/restpresencemixin.ts +++ b/src/common/lib/client/restpresencemixin.ts @@ -24,19 +24,17 @@ export class RestPresenceMixin { Utils.mixin(headers, client.options.headers); const options = presence.channel.channelOptions; - return new Promise((resolve, reject) => { - new PaginatedResource(client, this.basePath(presence) + '/history', headers, envelope, async function ( - body, - headers, - unpacked - ) { - return await presenceMessageFromResponseBody( - body as Record[], - options as CipherOptions, - client._MsgPack, - unpacked ? undefined : format - ); - }).get(params, (err, result) => (err ? reject(err) : resolve(result))); - }); + return new PaginatedResource(client, this.basePath(presence) + '/history', headers, envelope, async function ( + body, + headers, + unpacked + ) { + return await presenceMessageFromResponseBody( + body as Record[], + options as CipherOptions, + client._MsgPack, + unpacked ? undefined : format + ); + }).get(params); } } From cd0a6c47c3c47760b1f496d02811bfa9a106ff65 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 8 Dec 2023 16:30:35 -0300 Subject: [PATCH 29/44] Convert RestChannelMixin.history to use promises --- src/common/lib/client/realtimechannel.ts | 4 +-- src/common/lib/client/restchannel.ts | 4 +-- src/common/lib/client/restchannelmixin.ts | 30 ++++++++--------------- 3 files changed, 12 insertions(+), 26 deletions(-) diff --git a/src/common/lib/client/realtimechannel.ts b/src/common/lib/client/realtimechannel.ts index 0f5838d5f6..ea8e615a3d 100644 --- a/src/common/lib/client/realtimechannel.ts +++ b/src/common/lib/client/realtimechannel.ts @@ -878,9 +878,7 @@ class RealtimeChannel extends EventEmitter { params.from_serial = this.properties.attachSerial; } - return new Promise((resolve, reject) => { - restMixin.history(this, params, (err, result) => (err ? reject(err) : resolve(result))); - }); + return restMixin.history(this, params); } as any; whenState = ((state: string, listener: ErrCallback) => { diff --git a/src/common/lib/client/restchannel.ts b/src/common/lib/client/restchannel.ts index cb201c3925..9dc0e12f49 100644 --- a/src/common/lib/client/restchannel.ts +++ b/src/common/lib/client/restchannel.ts @@ -47,9 +47,7 @@ class RestChannel { async history(params: RestHistoryParams | null): Promise> { Logger.logAction(Logger.LOG_MICRO, 'RestChannel.history()', 'channel = ' + this.name); - return new Promise((resolve, reject) => { - this.client.rest.channelMixin.history(this, params, (err, result) => (err ? reject(err) : resolve(result))); - }); + return this.client.rest.channelMixin.history(this, params); } async publish(...args: any[]): Promise { diff --git a/src/common/lib/client/restchannelmixin.ts b/src/common/lib/client/restchannelmixin.ts index 24469980e3..fa10386a8d 100644 --- a/src/common/lib/client/restchannelmixin.ts +++ b/src/common/lib/client/restchannelmixin.ts @@ -2,10 +2,9 @@ import * as API from '../../../../ably'; import RestChannel from './restchannel'; import RealtimeChannel from './realtimechannel'; import * as Utils from '../util/utils'; -import { PaginatedResultCallback } from '../../types/utils'; import Message, { fromResponseBody as messageFromResponseBody } from '../types/message'; import Defaults from '../util/defaults'; -import PaginatedResource from './paginatedresource'; +import PaginatedResource, { PaginatedResult } from './paginatedresource'; import Resource from './resource'; export interface RestHistoryParams { @@ -22,9 +21,8 @@ export class RestChannelMixin { static history( channel: RestChannel | RealtimeChannel, - params: RestHistoryParams | null, - callback: PaginatedResultCallback - ): void { + params: RestHistoryParams | null + ): Promise> { const client = channel.client, format = client.options.useBinaryProtocol ? Utils.Format.msgpack : Utils.Format.json, envelope = channel.client.http.supportsLinkHeaders ? undefined : format, @@ -33,21 +31,13 @@ export class RestChannelMixin { Utils.mixin(headers, client.options.headers); const options = channel.channelOptions; - Utils.whenPromiseSettles( - new PaginatedResource(client, this.basePath(channel) + '/messages', headers, envelope, async function ( - body, - headers, - unpacked - ) { - return await messageFromResponseBody( - body as Message[], - options, - client._MsgPack, - unpacked ? undefined : format - ); - }).get(params as Record), - callback - ); + return new PaginatedResource(client, this.basePath(channel) + '/messages', headers, envelope, async function ( + body, + headers, + unpacked + ) { + return await messageFromResponseBody(body as Message[], options, client._MsgPack, unpacked ? undefined : format); + }).get(params as Record); } static async status(channel: RestChannel | RealtimeChannel): Promise { From 9093f0d33f595645288d0af598e30dc3d77d11c9 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 11 Dec 2023 14:43:57 -0300 Subject: [PATCH 30/44] Change RestChannel._publish to only pass error to callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit That’s all that the `publish` method looks at anyway, and will aid us in converting `_publish` to use promises. --- src/common/lib/client/restchannel.ts | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/common/lib/client/restchannel.ts b/src/common/lib/client/restchannel.ts index 9dc0e12f49..c926cb9568 100644 --- a/src/common/lib/client/restchannel.ts +++ b/src/common/lib/client/restchannel.ts @@ -11,8 +11,9 @@ import Message, { } from '../types/message'; import ErrorInfo from '../types/errorinfo'; import { PaginatedResult } from './paginatedresource'; -import Resource, { ResourceCallback } from './resource'; +import Resource from './resource'; import { ChannelOptions } from '../../types/channel'; +import { ErrCallback } from '../../types/utils'; import BaseRest from './baseclient'; import * as API from '../../../../ably'; import Defaults, { normaliseChannelOptions } from '../util/defaults'; @@ -126,12 +127,7 @@ class RestChannel { }); } - _publish( - requestBody: RequestBody | null, - headers: Record, - params: any, - callback: ResourceCallback - ): void { + _publish(requestBody: RequestBody | null, headers: Record, params: any, callback: ErrCallback): void { Resource.post( this.client, this.client.rest.channelMixin.basePath(this) + '/messages', @@ -139,7 +135,7 @@ class RestChannel { headers, params, null, - callback + (err) => callback(err) ); } From 564e4aa6e43018f60b30ad27b495d266faf4f349 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 11 Dec 2023 14:51:02 -0300 Subject: [PATCH 31/44] Convert RestChannel._publish to use promises --- src/common/lib/client/restchannel.ts | 31 ++++++++++++++-------------- test/rest/message.test.js | 20 +++++++++--------- 2 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/common/lib/client/restchannel.ts b/src/common/lib/client/restchannel.ts index c926cb9568..6b8186df81 100644 --- a/src/common/lib/client/restchannel.ts +++ b/src/common/lib/client/restchannel.ts @@ -13,7 +13,6 @@ import ErrorInfo from '../types/errorinfo'; import { PaginatedResult } from './paginatedresource'; import Resource from './resource'; import { ChannelOptions } from '../../types/channel'; -import { ErrCallback } from '../../types/utils'; import BaseRest from './baseclient'; import * as API from '../../../../ably'; import Defaults, { normaliseChannelOptions } from '../util/defaults'; @@ -95,7 +94,7 @@ class RestChannel { }); } - return new Promise((resolve, reject) => { + await new Promise((resolve, reject) => { encodeMessagesArray(messages, this.channelOptions as CipherOptions, (err: Error) => { if (err) { reject(err); @@ -120,23 +119,25 @@ class RestChannel { return; } - this._publish(serializeMessage(messages, client._MsgPack, format), headers, params, (err) => - err ? reject(err) : resolve() - ); + resolve(); }); }); + + await this._publish(serializeMessage(messages, client._MsgPack, format), headers, params); } - _publish(requestBody: RequestBody | null, headers: Record, params: any, callback: ErrCallback): void { - Resource.post( - this.client, - this.client.rest.channelMixin.basePath(this) + '/messages', - requestBody, - headers, - params, - null, - (err) => callback(err) - ); + async _publish(requestBody: RequestBody | null, 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()) + ); + }); } async status(): Promise { diff --git a/test/rest/message.test.js b/test/rest/message.test.js index 1f5f384b05..b6cd469bef 100644 --- a/test/rest/message.test.js +++ b/test/rest/message.test.js @@ -24,11 +24,11 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async channel = rest.channels.get('rest_implicit_client_id_0'); var originalPublish = channel._publish; - channel._publish = function (requestBody) { + channel._publish = async function (requestBody) { var message = JSON.parse(requestBody)[0]; expect(message.name === 'event0', 'Outgoing message interecepted').to.be.ok; expect(!message.clientId, 'client ID is not added by the client library as it is implicit').to.be.ok; - originalPublish.apply(channel, arguments); + return originalPublish.apply(channel, arguments); }; await channel.publish('event0', null); @@ -46,14 +46,14 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async channel = rest.channels.get('rest_explicit_client_id_0'); var originalPublish = channel._publish; - channel._publish = function (requestBody) { + channel._publish = async function (requestBody) { var message = JSON.parse(requestBody)[0]; expect(message.name === 'event0', 'Outgoing message interecepted').to.be.ok; expect( message.clientId == clientId, 'client ID is added by the client library as it is explicit in the publish' ).to.be.ok; - originalPublish.apply(channel, arguments); + return originalPublish.apply(channel, arguments); }; await channel.publish({ name: 'event0', clientId: clientId }); @@ -76,14 +76,14 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async channel = rest.channels.get('rest_explicit_client_id_1'); var originalPublish = channel._publish; - channel._publish = function (requestBody) { + channel._publish = async function (requestBody) { var message = JSON.parse(requestBody)[0]; expect(message.name === 'event0', 'Outgoing message interecepted').to.be.ok; expect( message.clientId == invalidClientId, 'invalid client ID is added by the client library as it is explicit in the publish' ).to.be.ok; - originalPublish.apply(channel, arguments); + return originalPublish.apply(channel, arguments); }; try { @@ -143,7 +143,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async originalPublish = channel._publish, originalDoUri = Ably.Realtime._Http.doUri; - channel._publish = function (requestBody) { + channel._publish = async function (requestBody) { var messageOne = JSON.parse(requestBody)[0]; var messageTwo = JSON.parse(requestBody)[1]; expect(messageOne.name).to.equal('one', 'Outgoing message 1 interecepted'); @@ -154,7 +154,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async expect(idTwo, 'id set on message 2').to.be.ok; expect(idOne && idOne.split(':')[1]).to.equal('0', 'check zero-based index'); expect(idTwo && idTwo.split(':')[1]).to.equal('1', 'check zero-based index'); - originalPublish.apply(channel, arguments); + return originalPublish.apply(channel, arguments); }; Ably.Rest._Http.doUri = function (method, uri, headers, body, params, callback) { @@ -186,9 +186,9 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async var originalPublish = channel._publish; /* Stub out _publish to check params */ - channel._publish = function (requestBody, headers, params) { + channel._publish = async function (requestBody, headers, params) { expect(params && params.testParam).to.equal('testParamValue'); - originalPublish.apply(channel, arguments); + return originalPublish.apply(channel, arguments); }; await channel.publish('foo', 'bar', { testParam: 'testParamValue' }); From 2001675d4bc86fa6e6e190d8d65813cb5a0a8775 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 15:22:53 +0000 Subject: [PATCH 32/44] 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 2be9eb1325..fb56ea0732 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 { RequestBody, 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: RequestBody | null): 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: RequestBody | null): 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: RequestBody | null): 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 f3c808ebc8..002fb5d44c 100644 --- a/src/common/lib/client/resource.ts +++ b/src/common/lib/client/resource.ts @@ -134,66 +134,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: RequestBody | null, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: true + ): Promise>; + static async post( + client: BaseClient, + path: string, + body: RequestBody | null, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: false + ): Promise>; + static async post( client: BaseClient, path: string, body: RequestBody | null, 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: RequestBody | null, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: true + ): Promise>; + static async patch( client: BaseClient, path: string, body: RequestBody | null, 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: RequestBody | null, + 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: RequestBody | null, 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: RequestBody | null, + headers: Record, + params: Record, + envelope: Utils.Format | null, + throwError: false + ): Promise>; + static async put( + client: BaseClient, + path: string, + body: RequestBody | null, + 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, @@ -201,14 +326,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; } @@ -253,7 +394,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 b9d7a9d6ce..20a8e9b737 100644 --- a/src/common/lib/client/rest.ts +++ b/src/common/lib/client/rest.ts @@ -172,25 +172,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 { @@ -201,27 +195,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( @@ -248,28 +226,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 6b8186df81..20825833ae 100644 --- a/src/common/lib/client/restchannel.ts +++ b/src/common/lib/client/restchannel.ts @@ -127,17 +127,15 @@ class RestChannel { } async _publish(requestBody: RequestBody | null, 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!; } } From 4951ec185ee3047a1f98ff160aeffaf89c34e420 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 16:53:53 +0000 Subject: [PATCH 33/44] Convert Resource.do to use promises Resolves #1528. --- src/common/lib/client/resource.ts | 249 ++++++++++++++---------------- 1 file changed, 115 insertions(+), 134 deletions(-) diff --git a/src/common/lib/client/resource.ts b/src/common/lib/client/resource.ts index 002fb5d44c..90a114810a 100644 --- a/src/common/lib/client/resource.ts +++ b/src/common/lib/client/resource.ts @@ -12,128 +12,95 @@ import { appendingParams as urlFromPathAndParams, paramString, } from 'common/types/http'; +import { ErrnoException } from '../../types/http'; -function withAuthDetails( +async function withAuthDetails( client: BaseClient, headers: RequestCallbackHeaders | undefined, params: Record, - errCallback: Function, opCallback: Function -) { +): Promise> { if (client.http.supportsAuthHeaders) { - Utils.whenPromiseSettles( - client.auth.getAuthHeaders(), - function (err: Error | null, authHeaders?: Record) { - if (err) errCallback(err); - else opCallback(Utils.mixin(authHeaders!, headers), params); - } - ); + const authHeaders = await client.auth.getAuthHeaders(); + return opCallback(Utils.mixin(authHeaders!, headers), params); } else { - Utils.whenPromiseSettles( - client.auth.getAuthParams(), - function (err: Error | null, authParams?: Record) { - if (err) errCallback(err); - else opCallback(headers, Utils.mixin(authParams!, params)); - } - ); + const authParams = await client.auth.getAuthParams(); + return opCallback(headers, Utils.mixin(authParams!, params)); } } function unenvelope( - callback: ResourceCallback, + result: ResourceResult, MsgPack: MsgPack | null, format: Utils.Format | null -): ResourceCallback { - return (err, body, outerHeaders, unpacked, outerStatusCode) => { - if (err && !body) { - callback(err); - return; - } +): ResourceResult { + if (result.err && !result.body) { + return { err: result.err }; + } - if (!unpacked) { - try { - body = Utils.decodeBody(body, MsgPack, format); - } catch (e) { - if (Utils.isErrorInfoOrPartialErrorInfo(e)) { - callback(e); - } else { - callback(new PartialErrorInfo(Utils.inspectError(e), null)); - } - return; + let body = result.body; + + if (!result.unpacked) { + try { + body = Utils.decodeBody(body, MsgPack, format); + } catch (e) { + if (Utils.isErrorInfoOrPartialErrorInfo(e)) { + return { err: e }; + } else { + return { err: new PartialErrorInfo(Utils.inspectError(e), null) }; } } + } - if (!body) { - callback(new PartialErrorInfo('unenvelope(): Response body is missing', null)); - return; - } + if (!body) { + return { err: new PartialErrorInfo('unenvelope(): Response body is missing', null) }; + } - const { statusCode: wrappedStatusCode, response, headers: wrappedHeaders } = body as Record; + const { statusCode: wrappedStatusCode, response, headers: wrappedHeaders } = body as Record; - if (wrappedStatusCode === undefined) { - /* Envelope already unwrapped by the transport */ - callback(err, body, outerHeaders, true, outerStatusCode); - return; - } + if (wrappedStatusCode === undefined) { + /* Envelope already unwrapped by the transport */ + return { ...result, body, unpacked: true }; + } - if (wrappedStatusCode < 200 || wrappedStatusCode >= 300) { - /* handle wrapped errors */ - let wrappedErr = (response && response.error) || err; - if (!wrappedErr) { - wrappedErr = new Error('Error in unenveloping ' + body); - wrappedErr.statusCode = wrappedStatusCode; - } - callback(wrappedErr, response, wrappedHeaders, true, wrappedStatusCode); - return; + if (wrappedStatusCode < 200 || wrappedStatusCode >= 300) { + /* handle wrapped errors */ + let wrappedErr = (response && response.error) || result.err; + if (!wrappedErr) { + wrappedErr = new Error('Error in unenveloping ' + body); + wrappedErr.statusCode = wrappedStatusCode; } + return { err: wrappedErr, body: response, headers: wrappedHeaders, unpacked: true, statusCode: wrappedStatusCode }; + } - callback(err, response, wrappedHeaders, true, wrappedStatusCode); - }; + return { err: result.err, body: response, headers: wrappedHeaders, unpacked: true, statusCode: wrappedStatusCode }; } -function logResponseHandler( - callback: ResourceCallback, - method: HttpMethods, - path: string, - params: Record -): ResourceCallback { - return (err, body, headers, unpacked, statusCode) => { - if (err) { - Logger.logAction( - Logger.LOG_MICRO, - 'Resource.' + method + '()', - 'Received Error; ' + urlFromPathAndParams(path, params) + '; Error: ' + Utils.inspectError(err) - ); - } else { - Logger.logAction( - Logger.LOG_MICRO, - 'Resource.' + method + '()', - 'Received; ' + - urlFromPathAndParams(path, params) + - '; Headers: ' + - paramString(headers as Record) + - '; StatusCode: ' + - statusCode + - '; Body' + - (Platform.BufferUtils.isBuffer(body) - ? ' (Base64): ' + Platform.BufferUtils.base64Encode(body) - : ': ' + Platform.Config.inspect(body)) - ); - } - if (callback) { - callback(err, body as T, headers, unpacked, statusCode); - } - }; +function logResult(result: ResourceResult, method: HttpMethods, path: string, params: Record) { + if (result.err) { + Logger.logAction( + Logger.LOG_MICRO, + 'Resource.' + method + '()', + 'Received Error; ' + urlFromPathAndParams(path, params) + '; Error: ' + Utils.inspectError(result.err) + ); + } else { + Logger.logAction( + Logger.LOG_MICRO, + 'Resource.' + method + '()', + 'Received; ' + + urlFromPathAndParams(path, params) + + '; Headers: ' + + paramString(result.headers as Record) + + '; StatusCode: ' + + result.statusCode + + '; Body: ' + + (Platform.BufferUtils.isBuffer(result.body) + ? ' (Base64): ' + Platform.BufferUtils.base64Encode(result.body) + : ': ' + Platform.Config.inspect(result.body)) + ); + } } -export type ResourceCallback = ( - err: IPartialErrorInfo | null, - body?: T, - headers?: RequestCallbackHeaders, - unpacked?: boolean, - statusCode?: number -) => void; - export interface ResourceResponse { body?: T; headers?: RequestCallbackHeaders; @@ -328,32 +295,15 @@ class Resource { envelope: Utils.Format | null, 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); - } - if (envelope) { - callback = unenvelope(callback!, client._MsgPack, envelope); (params = params || {})['envelope'] = envelope; } - function doRequest(this: any, headers: Record, params: Record) { + async function doRequest( + this: any, + headers: Record, + params: Record + ): Promise> { if (Logger.shouldLog(Logger.LOG_MICRO)) { let decodedBody = body; if (headers['content-type']?.indexOf('msgpack') > 0) { @@ -377,26 +327,57 @@ class Resource { ); } - client.http.do(method, path, headers, body, params, function (err, res, resHeaders, unpacked, statusCode) { - if (err && Auth.isTokenErr(err as ErrorInfo)) { - /* token has expired, so get a new one */ - Utils.whenPromiseSettles(client.auth.authorize(null, null), function (err: ErrorInfo | null) { - if (err) { - callback(err); - return; - } - /* retry ... */ - withAuthDetails(client, headers, params, callback, doRequest); - }); - return; - } - callback(err as ErrorInfo, res as T | undefined, resHeaders, unpacked, statusCode); + type HttpResult = { + error?: ErrnoException | IPartialErrorInfo | null; + body?: unknown; + headers?: RequestCallbackHeaders; + unpacked?: boolean; + statusCode?: number; + }; + + const httpResult = await new Promise((resolve) => { + client.http.do(method, path, headers, body, params, function (error, body, headers, unpacked, statusCode) { + resolve({ error, body, headers, unpacked, statusCode }); + }); }); + + if (httpResult.error && Auth.isTokenErr(httpResult.error as ErrorInfo)) { + /* token has expired, so get a new one */ + await client.auth.authorize(null, null); + /* retry ... */ + return withAuthDetails(client, headers, params, doRequest); + } + + return { + err: httpResult.error as ErrorInfo, + body: httpResult.body as T | undefined, + headers: httpResult.headers, + unpacked: httpResult.unpacked, + statusCode: httpResult.statusCode, + }; + } + + let result = await withAuthDetails(client, headers, params, doRequest); + + if (envelope) { + result = unenvelope(result, client._MsgPack, envelope); } - withAuthDetails(client, headers, params, callback!, doRequest); + if (Logger.shouldLog(Logger.LOG_MICRO)) { + logResult(result, method, path, params); + } + + if (throwError) { + if (result.err) { + throw result.err; + } else { + const response: Omit, 'err'> & Pick>, 'err'> = { ...result }; + delete response.err; + return response; + } + } - return promise; + return result; } } From da6d2064a47250f7bd889dd5da426e4a4549683c Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 09:39:29 +0000 Subject: [PATCH 34/44] Convert message `encrypt` to use promises --- src/common/lib/types/defaultmessage.ts | 7 ++++++- src/common/lib/types/message.ts | 29 ++++++++++++++++---------- 2 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/common/lib/types/defaultmessage.ts b/src/common/lib/types/defaultmessage.ts index 9350098c84..58198b09fa 100644 --- a/src/common/lib/types/defaultmessage.ts +++ b/src/common/lib/types/defaultmessage.ts @@ -10,6 +10,7 @@ import * as API from '../../../../ably'; import Platform from 'common/platform'; import PresenceMessage from './presencemessage'; import { ChannelOptions } from 'common/types/channel'; +import { StandardCallback } from 'common/types/utils'; /** `DefaultMessage` is the class returned by `DefaultRest` and `DefaultRealtime`’s `Message` static property. It introduces the static methods described in the `MessageStatic` interface of the public API of the non tree-shakable version of the library. @@ -29,7 +30,11 @@ export class DefaultMessage extends Message { } // Used by tests - static encode(msg: Message | PresenceMessage, options: CipherOptions, callback: Function): void { + static encode( + msg: T, + options: CipherOptions, + callback: StandardCallback + ): void { encode(msg, options, callback); } diff --git a/src/common/lib/types/message.ts b/src/common/lib/types/message.ts index 828cba66d9..ac36419c31 100644 --- a/src/common/lib/types/message.ts +++ b/src/common/lib/types/message.ts @@ -8,6 +8,7 @@ import { Bufferlike as BrowserBufferlike } from '../../../platform/web/lib/util/ import * as API from '../../../../ably'; import { IUntypedCryptoStatic } from 'common/types/ICryptoStatic'; import { MsgPack } from 'common/types/msgpack'; +import { StandardCallback } from 'common/types/utils'; export type CipherOptions = { channelCipher: { @@ -101,7 +102,7 @@ export async function fromEncodedArray( ); } -function encrypt(msg: Message | PresenceMessage, options: CipherOptions, callback: Function) { +async function encrypt(msg: T, options: CipherOptions): Promise { let data = msg.data, encoding = msg.encoding, cipher = options.channelCipher; @@ -111,18 +112,24 @@ function encrypt(msg: Message | PresenceMessage, options: CipherOptions, callbac data = Platform.BufferUtils.utf8Encode(String(data)); encoding = encoding + 'utf-8/'; } - cipher.encrypt(data, function (err: Error, data: unknown) { - if (err) { - callback(err); - return; - } - msg.data = data; - msg.encoding = encoding + 'cipher+' + cipher.algorithm; - callback(null, msg); + return new Promise((resolve, reject) => { + cipher.encrypt(data, function (err: Error, data: unknown) { + if (err) { + reject(err); + return; + } + msg.data = data; + msg.encoding = encoding + 'cipher+' + cipher.algorithm; + resolve(msg); + }); }); } -export function encode(msg: Message | PresenceMessage, options: CipherOptions, callback: Function): void { +export function encode( + msg: T, + options: CipherOptions, + callback: StandardCallback +): void { const data = msg.data; const nativeDataType = typeof data == 'string' || Platform.BufferUtils.isBuffer(data) || data === null || data === undefined; @@ -137,7 +144,7 @@ export function encode(msg: Message | PresenceMessage, options: CipherOptions, c } if (options != null && options.cipher) { - encrypt(msg, options, callback); + Utils.whenPromiseSettles(encrypt(msg, options), callback); } else { callback(null, msg); } From 019c3156b674a750b4ace1d0d5ad2d565282a58d Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 09:54:31 +0000 Subject: [PATCH 35/44] Convert message `encode` to use promises The changes to the `connectionQueuing` test are because the test assumed that the message would get marked as `sendAttempted` synchronously as soon as `publish` was called. This relied on the fact that when no encryption is required, the Message `encode` function called its callback synchronously. However, this was an implementation detail (this function has an asynchronous signature), and one which is no longer true now that promises are used. --- src/common/lib/client/realtimepresence.ts | 58 ++++++------- src/common/lib/types/defaultmessage.ts | 9 +- src/common/lib/types/message.ts | 31 +++---- test/realtime/connection.test.js | 101 ++++++++++++++-------- test/realtime/crypto.test.js | 8 +- 5 files changed, 110 insertions(+), 97 deletions(-) diff --git a/src/common/lib/client/realtimepresence.ts b/src/common/lib/client/realtimepresence.ts index abaf126e6b..435ecf5c87 100644 --- a/src/common/lib/client/realtimepresence.ts +++ b/src/common/lib/client/realtimepresence.ts @@ -6,7 +6,7 @@ import PresenceMessage, { fromData as presenceMessageFromData, encode as encodePresenceMessage, } from '../types/presencemessage'; -import ErrorInfo, { IPartialErrorInfo, PartialErrorInfo } from '../types/errorinfo'; +import ErrorInfo, { PartialErrorInfo } from '../types/errorinfo'; import RealtimeChannel from './realtimechannel'; import Multicaster from '../util/multicaster'; import ChannelStateChange from './channelstatechange'; @@ -149,36 +149,32 @@ class RealtimePresence extends EventEmitter { presence.clientId = clientId; } - return new Promise((resolve, reject) => { - encodePresenceMessage(presence, channel.channelOptions as CipherOptions, (err: IPartialErrorInfo) => { - if (err) { - reject(err); - return; - } - switch (channel.state) { - case 'attached': - channel.sendPresence(presence, (err) => (err ? reject(err) : resolve())); - break; - case 'initialized': - case 'detached': - channel.attach(); - // eslint-disable-next-line no-fallthrough - case 'attaching': - this.pendingPresence.push({ - presence: presence, - callback: (err) => (err ? reject(err) : resolve()), - }); - break; - default: - err = new PartialErrorInfo( - 'Unable to ' + action + ' presence channel while in ' + channel.state + ' state', - 90001 - ); - err.code = 90001; - reject(err); - } - }); - }); + await encodePresenceMessage(presence, channel.channelOptions as CipherOptions); + switch (channel.state) { + case 'attached': + return new Promise((resolve, reject) => { + channel.sendPresence(presence, (err) => (err ? reject(err) : resolve())); + }); + case 'initialized': + case 'detached': + channel.attach(); + // eslint-disable-next-line no-fallthrough + case 'attaching': + return new Promise((resolve, reject) => { + this.pendingPresence.push({ + presence: presence, + callback: (err) => (err ? reject(err) : resolve()), + }); + }); + default: { + const err = new PartialErrorInfo( + 'Unable to ' + action + ' presence channel while in ' + channel.state + ' state', + 90001 + ); + err.code = 90001; + throw err; + } + } } async leave(data: unknown): Promise { diff --git a/src/common/lib/types/defaultmessage.ts b/src/common/lib/types/defaultmessage.ts index 58198b09fa..8ec587fdf7 100644 --- a/src/common/lib/types/defaultmessage.ts +++ b/src/common/lib/types/defaultmessage.ts @@ -10,7 +10,6 @@ import * as API from '../../../../ably'; import Platform from 'common/platform'; import PresenceMessage from './presencemessage'; import { ChannelOptions } from 'common/types/channel'; -import { StandardCallback } from 'common/types/utils'; /** `DefaultMessage` is the class returned by `DefaultRest` and `DefaultRealtime`’s `Message` static property. It introduces the static methods described in the `MessageStatic` interface of the public API of the non tree-shakable version of the library. @@ -30,12 +29,8 @@ export class DefaultMessage extends Message { } // Used by tests - static encode( - msg: T, - options: CipherOptions, - callback: StandardCallback - ): void { - encode(msg, options, callback); + static async encode(msg: T, options: CipherOptions): Promise { + return encode(msg, options); } // Used by tests diff --git a/src/common/lib/types/message.ts b/src/common/lib/types/message.ts index ac36419c31..d973ec7e5c 100644 --- a/src/common/lib/types/message.ts +++ b/src/common/lib/types/message.ts @@ -125,11 +125,7 @@ async function encrypt(msg: T, options: Cip }); } -export function encode( - msg: T, - options: CipherOptions, - callback: StandardCallback -): void { +export async function encode(msg: T, options: CipherOptions): Promise { const data = msg.data; const nativeDataType = typeof data == 'string' || Platform.BufferUtils.isBuffer(data) || data === null || data === undefined; @@ -144,26 +140,19 @@ export function encode( } if (options != null && options.cipher) { - Utils.whenPromiseSettles(encrypt(msg, options), callback); + return encrypt(msg, options); } else { - callback(null, msg); + return msg; } } -export function encodeArray(messages: Array, options: CipherOptions, callback: Function): void { - let processed = 0; - for (let i = 0; i < messages.length; i++) { - encode(messages[i], options, function (err: Error) { - if (err) { - callback(err); - return; - } - processed++; - if (processed == messages.length) { - callback(null, messages); - } - }); - } +export function encodeArray( + messages: Array, + options: CipherOptions, + callback: StandardCallback> +): void { + const promises = messages.map((message) => encode(message, options)); + return Utils.whenPromiseSettles(Promise.all(promises), callback); } export const serialize = Utils.encodeBody; diff --git a/test/realtime/connection.test.js b/test/realtime/connection.test.js index 04d193c4fe..8d5b9fb163 100644 --- a/test/realtime/connection.test.js +++ b/test/realtime/connection.test.js @@ -178,62 +178,95 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async closeAndFinish(done, realtime, err); return; } + + let transportSendCallback; + /* Sabotage sending the message */ transport.send = function (msg) { if (msg.action == 15) { expect(msg.msgSerial).to.equal(0, 'Expect msgSerial to be 0'); + + if (!transportSendCallback) { + done(new Error('transport.send override called before transportSendCallback populated')); + } + + transportSendCallback(null); } }; - async.parallel( + let publishCallback; + + async.series( [ function (cb) { + transportSendCallback = cb; + /* Sabotaged publish */ whenPromiseSettles(channel.publish('first', null), function (err) { - try { - expect(!err, 'Check publish happened (eventually) without err').to.be.ok; - } catch (err) { - cb(err); - return; + if (!publishCallback) { + done(new Error('publish completed before publishCallback populated')); } - cb(); + publishCallback(err); }); }, - function (cb) { - /* After the disconnect, on reconnect, spy on transport.send again */ - connectionManager.once('transport.pending', function (transport) { - var oldSend = transport.send; - transport.send = function (msg, msgCb) { - if (msg.action === 15) { - if (msg.messages[0].name === 'first') { - try { - expect(msg.msgSerial).to.equal(0, 'Expect msgSerial of original message to still be 0'); - expect(msg.messages.length).to.equal( - 1, - 'Expect second message to not have been merged with the attempted message' - ); - } catch (err) { - cb(err); - return; - } - } else if (msg.messages[0].name === 'second') { + // We wait for transport.send to recieve the message that we just + // published before we proceed to disconnecting the transport, to + // make sure that the message got marked as `sendAttempted`. + + function (cb) { + async.parallel( + [ + function (cb) { + publishCallback = function (err) { try { - expect(msg.msgSerial).to.equal(1, 'Expect msgSerial of new message to be 1'); + expect(!err, 'Check publish happened (eventually) without err').to.be.ok; } catch (err) { cb(err); return; } cb(); - } - } - oldSend.call(transport, msg, msgCb); - }; - channel.publish('second', null); - }); + }; + }, + function (cb) { + /* After the disconnect, on reconnect, spy on transport.send again */ + connectionManager.once('transport.pending', function (transport) { + var oldSend = transport.send; + + transport.send = function (msg, msgCb) { + if (msg.action === 15) { + if (msg.messages[0].name === 'first') { + try { + expect(msg.msgSerial).to.equal(0, 'Expect msgSerial of original message to still be 0'); + expect(msg.messages.length).to.equal( + 1, + 'Expect second message to not have been merged with the attempted message' + ); + } catch (err) { + cb(err); + return; + } + } else if (msg.messages[0].name === 'second') { + try { + expect(msg.msgSerial).to.equal(1, 'Expect msgSerial of new message to be 1'); + } catch (err) { + cb(err); + return; + } + cb(); + } + } + oldSend.call(transport, msg, msgCb); + }; + channel.publish('second', null); + }); - /* Disconnect the transport (will automatically reconnect and resume) () */ - connectionManager.disconnectAllTransports(); + /* Disconnect the transport (will automatically reconnect and resume) () */ + connectionManager.disconnectAllTransports(); + }, + ], + cb + ); }, ], function (err) { diff --git a/test/realtime/crypto.test.js b/test/realtime/crypto.test.js index ded9b31c0a..af44e1ee39 100644 --- a/test/realtime/crypto.test.js +++ b/test/realtime/crypto.test.js @@ -223,7 +223,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async true, function (channelOpts, testMessage, encryptedMessage) { /* encrypt plaintext message; encode() also to handle data that is not already string or buffer */ - Message.encode(testMessage, channelOpts, function () { + whenPromiseSettles(Message.encode(testMessage, channelOpts), function () { /* compare */ testMessageEquality(done, testMessage, encryptedMessage); }); @@ -240,7 +240,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async true, function (channelOpts, testMessage, encryptedMessage) { /* encrypt plaintext message; encode() also to handle data that is not already string or buffer */ - Message.encode(testMessage, channelOpts, function () { + whenPromiseSettles(Message.encode(testMessage, channelOpts), function () { /* compare */ testMessageEquality(done, testMessage, encryptedMessage); }); @@ -314,7 +314,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async 2, false, function (channelOpts, testMessage, encryptedMessage, msgpackEncodedMessage) { - Message.encode(testMessage, channelOpts, function () { + whenPromiseSettles(Message.encode(testMessage, channelOpts), function () { var msgpackFromEncoded = msgpack.encode(testMessage); var msgpackFromEncrypted = msgpack.encode(encryptedMessage); var messageFromMsgpack = Message.fromValues( @@ -348,7 +348,7 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, helper, async 2, false, function (channelOpts, testMessage, encryptedMessage, msgpackEncodedMessage) { - Message.encode(testMessage, channelOpts, function () { + whenPromiseSettles(Message.encode(testMessage, channelOpts), function () { var msgpackFromEncoded = msgpack.encode(testMessage); var msgpackFromEncrypted = msgpack.encode(encryptedMessage); var messageFromMsgpack = Message.fromValues( From 69b64dca2e90393cf4727d44d1398490c049be9f Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 10:01:52 +0000 Subject: [PATCH 36/44] Convert message `encodeArray` to use promises Resolves #1531. --- src/common/lib/client/realtimechannel.ts | 38 ++++++++------------ src/common/lib/client/restchannel.ts | 44 +++++++++--------------- src/common/lib/types/message.ts | 10 ++---- 3 files changed, 33 insertions(+), 59 deletions(-) diff --git a/src/common/lib/client/realtimechannel.ts b/src/common/lib/client/realtimechannel.ts index ea8e615a3d..42090d0e66 100644 --- a/src/common/lib/client/realtimechannel.ts +++ b/src/common/lib/client/realtimechannel.ts @@ -231,30 +231,22 @@ class RealtimeChannel extends EventEmitter { messages = [messageFromValues({ name: args[0], data: args[1] })]; } const maxMessageSize = this.client.options.maxMessageSize; + await encodeMessagesArray(messages, this.channelOptions as CipherOptions); + /* RSL1i */ + const size = getMessagesSize(messages); + if (size > maxMessageSize) { + throw new ErrorInfo( + 'Maximum size of messages that can be published at once exceeded ( was ' + + size + + ' bytes; limit is ' + + maxMessageSize + + ' bytes)', + 40009, + 400 + ); + } return new Promise((resolve, reject) => { - encodeMessagesArray(messages, this.channelOptions as CipherOptions, (err: Error | null) => { - if (err) { - reject(err); - return; - } - /* RSL1i */ - const size = getMessagesSize(messages); - if (size > maxMessageSize) { - reject( - new ErrorInfo( - 'Maximum size of messages that can be published at once exceeded ( was ' + - size + - ' bytes; limit is ' + - maxMessageSize + - ' bytes)', - 40009, - 400 - ) - ); - return; - } - this._publish(messages, (err) => (err ? reject(err) : resolve())); - }); + this._publish(messages, (err) => (err ? reject(err) : resolve())); }); } diff --git a/src/common/lib/client/restchannel.ts b/src/common/lib/client/restchannel.ts index 20825833ae..23c4e9b9d6 100644 --- a/src/common/lib/client/restchannel.ts +++ b/src/common/lib/client/restchannel.ts @@ -94,34 +94,22 @@ class RestChannel { }); } - await new Promise((resolve, reject) => { - encodeMessagesArray(messages, this.channelOptions as CipherOptions, (err: Error) => { - if (err) { - reject(err); - return; - } - - /* RSL1i */ - const size = getMessagesSize(messages), - maxMessageSize = options.maxMessageSize; - if (size > maxMessageSize) { - reject( - new ErrorInfo( - 'Maximum size of messages that can be published at once exceeded ( was ' + - size + - ' bytes; limit is ' + - maxMessageSize + - ' bytes)', - 40009, - 400 - ) - ); - return; - } - - resolve(); - }); - }); + await encodeMessagesArray(messages, this.channelOptions as CipherOptions); + + /* RSL1i */ + const size = getMessagesSize(messages), + maxMessageSize = options.maxMessageSize; + if (size > maxMessageSize) { + throw new ErrorInfo( + 'Maximum size of messages that can be published at once exceeded ( was ' + + size + + ' bytes; limit is ' + + maxMessageSize + + ' bytes)', + 40009, + 400 + ); + } await this._publish(serializeMessage(messages, client._MsgPack, format), headers, params); } diff --git a/src/common/lib/types/message.ts b/src/common/lib/types/message.ts index d973ec7e5c..98e0fa2265 100644 --- a/src/common/lib/types/message.ts +++ b/src/common/lib/types/message.ts @@ -8,7 +8,6 @@ import { Bufferlike as BrowserBufferlike } from '../../../platform/web/lib/util/ import * as API from '../../../../ably'; import { IUntypedCryptoStatic } from 'common/types/ICryptoStatic'; import { MsgPack } from 'common/types/msgpack'; -import { StandardCallback } from 'common/types/utils'; export type CipherOptions = { channelCipher: { @@ -146,13 +145,8 @@ export async function encode(msg: T, option } } -export function encodeArray( - messages: Array, - options: CipherOptions, - callback: StandardCallback> -): void { - const promises = messages.map((message) => encode(message, options)); - return Utils.whenPromiseSettles(Promise.all(promises), callback); +export async function encodeArray(messages: Array, options: CipherOptions): Promise> { + return Promise.all(messages.map((message) => encode(message, options))); } export const serialize = Utils.encodeBody; From 499ca191586b2ebf3b9f186d5d25c0ed617bbdb1 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 11:34:28 +0000 Subject: [PATCH 37/44] Remove leftover documentation comment Missed this in a0105eb. --- src/platform/nodejs/lib/util/crypto.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/src/platform/nodejs/lib/util/crypto.ts b/src/platform/nodejs/lib/util/crypto.ts index d971b842ca..d9b64660a3 100644 --- a/src/platform/nodejs/lib/util/crypto.ts +++ b/src/platform/nodejs/lib/util/crypto.ts @@ -186,7 +186,6 @@ var createCryptoClass = function (bufferUtils: typeof BufferUtils) { * Generate a random encryption key from the supplied keylength (or the * default keyLength if none supplied) as a Buffer * @param keyLength (optional) the required keyLength in bits - * @param callback (optional) (err, key) */ static async generateRandomKey(keyLength?: number): Promise { return new Promise((resolve, reject) => { From 1e15e53cee690d5e8000a078be253e89d88705eb Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 10:29:23 +0000 Subject: [PATCH 38/44] Convert web crypto `generateRandom` to use promises --- src/platform/web/lib/util/crypto.ts | 43 ++++++++++++++--------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/src/platform/web/lib/util/crypto.ts b/src/platform/web/lib/util/crypto.ts index ad8bb8c3e3..5316c0bacd 100644 --- a/src/platform/web/lib/util/crypto.ts +++ b/src/platform/web/lib/util/crypto.ts @@ -6,6 +6,7 @@ import ICipher from '../../../../common/types/ICipher'; import { CryptoDataTypes } from '../../../../common/types/cryptoDataTypes'; import BufferUtils, { Bufferlike, Output as BufferUtilsOutput } from './bufferutils'; import { IPlatformConfig } from 'common/types/IPlatformConfig'; +import * as Utils from '../../../../common/lib/util/utils'; // The type to which ./msgpack.ts deserializes elements of the `bin` or `ext` type type MessagePackBinaryType = ArrayBuffer; @@ -27,24 +28,27 @@ var createCryptoClass = function (config: IPlatformConfig, bufferUtils: typeof B /** * Internal: generate an array of secure random data corresponding to the given length of bytes * @param bytes - * @param callback */ - var generateRandom: (byteLength: number, callback: (error: Error | null, result: ArrayBuffer | null) => void) => void; + var generateRandom: (byteLength: number) => Promise; if (config.getRandomArrayBuffer) { - generateRandom = config.getRandomArrayBuffer; + generateRandom = async (byteLength) => { + return new Promise((resolve, reject) => { + config.getRandomArrayBuffer!(byteLength, (err, result) => (err ? reject(err) : resolve(result!))); + }); + }; } else if (typeof Uint32Array !== 'undefined' && config.getRandomValues) { - generateRandom = function (bytes, callback) { + generateRandom = async function (bytes) { var blockRandomArray = new Uint32Array(DEFAULT_BLOCKLENGTH_WORDS); var words = bytes / 4, nativeArray = words == DEFAULT_BLOCKLENGTH_WORDS ? blockRandomArray : new Uint32Array(words); - config.getRandomValues!(nativeArray, function (err) { - if (typeof callback !== 'undefined') { - callback(err, bufferUtils.toArrayBuffer(nativeArray)); - } + return new Promise((resolve, reject) => { + config.getRandomValues!(nativeArray, (err) => + err ? reject(err) : resolve(bufferUtils.toArrayBuffer(nativeArray)) + ); }); }; } else { - generateRandom = function (bytes, callback) { + generateRandom = async function (bytes) { Logger.logAction( Logger.LOG_MAJOR, 'Ably.Crypto.generateRandom()', @@ -56,7 +60,7 @@ var createCryptoClass = function (config: IPlatformConfig, bufferUtils: typeof B array[i] = Math.floor(Math.random() * UINT32_SUP); } - callback(null, bufferUtils.toArrayBuffer(array)); + return bufferUtils.toArrayBuffer(array); }; } @@ -181,16 +185,11 @@ var createCryptoClass = function (config: IPlatformConfig, bufferUtils: typeof B * @param keyLength (optional) the required keyLength in bits */ static async generateRandomKey(keyLength?: number): Promise { - return new Promise((resolve, reject) => { - generateRandom((keyLength || DEFAULT_KEYLENGTH) / 8, function (err, buf) { - if (err) { - const errorInfo = new ErrorInfo('Failed to generate random key: ' + err.message, 400, 50000, err); - reject(errorInfo); - } else { - resolve(buf!); - } - }); - }); + try { + return await generateRandom((keyLength || DEFAULT_KEYLENGTH) / 8); + } catch (err) { + throw new ErrorInfo('Failed to generate random key: ' + (err as Error).message, 400, 50000, err as Error); + } } /** @@ -300,9 +299,9 @@ var createCryptoClass = function (config: IPlatformConfig, bufferUtils: typeof B return; } - generateRandom(DEFAULT_BLOCKLENGTH, function (err, randomBlock) { + Utils.whenPromiseSettles(generateRandom(DEFAULT_BLOCKLENGTH), function (err, randomBlock) { if (err) { - callback(err, null); + callback(err as Error, null); return; } callback(null, bufferUtils.toArrayBuffer(randomBlock!)); From ec45a10516ee6aa8976cc95afce7037949f62a96 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 11:15:26 +0000 Subject: [PATCH 39/44] Make ICipher.encrypt signature consistent with our other callbacks --- src/common/types/ICipher.ts | 2 +- src/platform/nodejs/lib/util/crypto.ts | 2 +- src/platform/web/lib/util/crypto.ts | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/common/types/ICipher.ts b/src/common/types/ICipher.ts index a7d07c0598..83a7653a78 100644 --- a/src/common/types/ICipher.ts +++ b/src/common/types/ICipher.ts @@ -1,5 +1,5 @@ export default interface ICipher { algorithm: string; - encrypt: (plaintext: InputPlaintext, callback: (error: Error | null, data: OutputCiphertext | null) => void) => void; + encrypt: (plaintext: InputPlaintext, callback: (error: Error | null, data?: OutputCiphertext) => void) => void; decrypt: (ciphertext: InputCiphertext) => Promise; } diff --git a/src/platform/nodejs/lib/util/crypto.ts b/src/platform/nodejs/lib/util/crypto.ts index d9b64660a3..442a75f557 100644 --- a/src/platform/nodejs/lib/util/crypto.ts +++ b/src/platform/nodejs/lib/util/crypto.ts @@ -233,7 +233,7 @@ var createCryptoClass = function (bufferUtils: typeof BufferUtils) { this.blockLength = this.iv.length; } - encrypt(plaintext: InputPlaintext, callback: (error: Error | null, data: OutputCiphertext | null) => void) { + encrypt(plaintext: InputPlaintext, callback: (error: Error | null, data?: OutputCiphertext) => void) { Logger.logAction(Logger.LOG_MICRO, 'CBCCipher.encrypt()', ''); var plaintextBuffer = bufferUtils.toBuffer(plaintext); var plaintextLength = plaintextBuffer.length, diff --git a/src/platform/web/lib/util/crypto.ts b/src/platform/web/lib/util/crypto.ts index 5316c0bacd..0fa0ffb90a 100644 --- a/src/platform/web/lib/util/crypto.ts +++ b/src/platform/web/lib/util/crypto.ts @@ -251,7 +251,7 @@ var createCryptoClass = function (config: IPlatformConfig, bufferUtils: typeof B return output; } - encrypt(plaintext: InputPlaintext, callback: (error: Error | null, data: OutputCiphertext | null) => void) { + encrypt(plaintext: InputPlaintext, callback: (error: Error | null, data?: OutputCiphertext) => void) { Logger.logAction(Logger.LOG_MICRO, 'CBCCipher.encrypt()', ''); const encryptAsync = async () => { @@ -276,7 +276,7 @@ var createCryptoClass = function (config: IPlatformConfig, bufferUtils: typeof B callback(null, ciphertext); }) .catch((error) => { - callback(error, null); + callback(error); }); } From d87123dd7ac10a0cbaf7f961c7ba00d869bff6da Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 11:11:32 +0000 Subject: [PATCH 40/44] Convert Node crypto `generateRandom` to use promises MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit I’ve removed the redundant upfront generation of the IV and just kept the generation in `getIv`; this makes it consistent with the web crypto code. --- src/platform/nodejs/lib/util/crypto.ts | 85 +++++++++++++------------- 1 file changed, 44 insertions(+), 41 deletions(-) diff --git a/src/platform/nodejs/lib/util/crypto.ts b/src/platform/nodejs/lib/util/crypto.ts index 442a75f557..9a3010f47a 100644 --- a/src/platform/nodejs/lib/util/crypto.ts +++ b/src/platform/nodejs/lib/util/crypto.ts @@ -8,6 +8,8 @@ import ICipher from '../../../../common/types/ICipher'; import { CryptoDataTypes } from '../../../../common/types/cryptoDataTypes'; import { Cipher as NodeCipher, CipherKey as NodeCipherKey } from 'crypto'; import BufferUtils, { Bufferlike, Output as BufferUtilsOutput } from './bufferutils'; +import util from 'util'; +import * as Utils from '../../../../common/lib/util/utils'; // The type to which ably-forks/msgpack-js deserializes elements of the `bin` or `ext` type type MessagePackBinaryType = Buffer; @@ -27,12 +29,9 @@ var createCryptoClass = function (bufferUtils: typeof BufferUtils) { /** * Internal: generate a buffer of secure random bytes of the given length * @param bytes - * @param callback (optional) */ - function generateRandom(bytes: number): Buffer; - function generateRandom(bytes: number, callback: (err: Error | null, buf: Buffer) => void): void; - function generateRandom(bytes: number, callback?: (err: Error | null, buf: Buffer) => void) { - return callback === undefined ? crypto.randomBytes(bytes) : crypto.randomBytes(bytes, callback); + async function generateRandom(bytes: number): Promise { + return util.promisify(crypto.randomBytes)(bytes); } /** @@ -188,16 +187,11 @@ var createCryptoClass = function (bufferUtils: typeof BufferUtils) { * @param keyLength (optional) the required keyLength in bits */ static async generateRandomKey(keyLength?: number): Promise { - return new Promise((resolve, reject) => { - generateRandom((keyLength || DEFAULT_KEYLENGTH) / 8, function (err, buf) { - if (err) { - const errorInfo = new ErrorInfo('Failed to generate random key: ' + err.message, 500, 50000, err); - reject(errorInfo); - } else { - resolve(buf!); - } - }); - }); + try { + return generateRandom((keyLength || DEFAULT_KEYLENGTH) / 8); + } catch (err) { + throw new ErrorInfo('Failed to generate random key: ' + (err as Error).message, 500, 50000, err as Error); + } } /** @@ -208,10 +202,9 @@ var createCryptoClass = function (bufferUtils: typeof BufferUtils) { static getCipher(params: IGetCipherParams) { var cipherParams = isInstCipherParams(params) ? (params as CipherParams) : this.getDefaultParams(params); - var iv = params.iv || generateRandom(DEFAULT_BLOCKLENGTH); return { cipherParams: cipherParams, - cipher: new CBCCipher(cipherParams, iv), + cipher: new CBCCipher(cipherParams, params.iv ?? null), }; } } @@ -222,51 +215,61 @@ var createCryptoClass = function (bufferUtils: typeof BufferUtils) { algorithm: string; key: NodeCipherKey; iv: Buffer | null; - encryptCipher: NodeCipher; - blockLength: number; + encryptCipher: NodeCipher | null = null; - constructor(params: CipherParams, iv: Buffer) { + constructor(params: CipherParams, iv: Buffer | null) { this.algorithm = params.algorithm + '-' + String(params.keyLength) + '-' + params.mode; this.key = params.key; this.iv = iv; - this.encryptCipher = crypto.createCipheriv(this.algorithm, this.key, this.iv); - this.blockLength = this.iv.length; } encrypt(plaintext: InputPlaintext, callback: (error: Error | null, data?: OutputCiphertext) => void) { - Logger.logAction(Logger.LOG_MICRO, 'CBCCipher.encrypt()', ''); - var plaintextBuffer = bufferUtils.toBuffer(plaintext); - var plaintextLength = plaintextBuffer.length, - paddedLength = getPaddedLength(plaintextLength), - iv = this.getIv(); - var cipherOut = this.encryptCipher.update( - Buffer.concat([plaintextBuffer, pkcs5Padding[paddedLength - plaintextLength]]) - ); - var ciphertext = Buffer.concat([iv, toBuffer(cipherOut)]); - return callback(null, ciphertext); + const promise = (async () => { + Logger.logAction(Logger.LOG_MICRO, 'CBCCipher.encrypt()', ''); + + const iv = await this.getIv(); + if (!this.encryptCipher) { + this.encryptCipher = crypto.createCipheriv(this.algorithm, this.key, iv); + } + + var plaintextBuffer = bufferUtils.toBuffer(plaintext); + var plaintextLength = plaintextBuffer.length, + paddedLength = getPaddedLength(plaintextLength); + var cipherOut = this.encryptCipher.update( + Buffer.concat([plaintextBuffer, pkcs5Padding[paddedLength - plaintextLength]]) + ); + var ciphertext = Buffer.concat([iv, toBuffer(cipherOut)]); + return ciphertext; + })(); + + Utils.whenPromiseSettles(promise, callback); } async decrypt(ciphertext: InputCiphertext): Promise { - var blockLength = this.blockLength, - decryptCipher = crypto.createDecipheriv(this.algorithm, this.key, ciphertext.slice(0, blockLength)), - plaintext = toBuffer(decryptCipher.update(ciphertext.slice(blockLength))), + var decryptCipher = crypto.createDecipheriv(this.algorithm, this.key, ciphertext.slice(0, DEFAULT_BLOCKLENGTH)), + plaintext = toBuffer(decryptCipher.update(ciphertext.slice(DEFAULT_BLOCKLENGTH))), final = decryptCipher.final(); if (final && final.length) plaintext = Buffer.concat([plaintext, toBuffer(final)]); return plaintext; } - getIv() { + async getIv() { if (this.iv) { var iv = this.iv; this.iv = null; return iv; } - var randomBlock = generateRandom(DEFAULT_BLOCKLENGTH); - /* Since the iv for a new block is the ciphertext of the last, this - * sets a new iv (= aes(randomBlock XOR lastCipherText)) as well as - * returning it */ - return toBuffer(this.encryptCipher.update(randomBlock)); + var randomBlock = await generateRandom(DEFAULT_BLOCKLENGTH); + + if (!this.encryptCipher) { + return randomBlock; + } else { + /* Since the iv for a new block is the ciphertext of the last, this + * sets a new iv (= aes(randomBlock XOR lastCipherText)) as well as + * returning it */ + return toBuffer(this.encryptCipher.update(randomBlock)); + } } } From 32d02a6e96c7da48c486e3b5c523112790ad3ff0 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 11:26:16 +0000 Subject: [PATCH 41/44] Convert web crypto `getIv` to use promises --- src/platform/web/lib/util/crypto.ts | 26 +++++--------------------- 1 file changed, 5 insertions(+), 21 deletions(-) diff --git a/src/platform/web/lib/util/crypto.ts b/src/platform/web/lib/util/crypto.ts index 0fa0ffb90a..2dcb1c0644 100644 --- a/src/platform/web/lib/util/crypto.ts +++ b/src/platform/web/lib/util/crypto.ts @@ -6,7 +6,6 @@ import ICipher from '../../../../common/types/ICipher'; import { CryptoDataTypes } from '../../../../common/types/cryptoDataTypes'; import BufferUtils, { Bufferlike, Output as BufferUtilsOutput } from './bufferutils'; import { IPlatformConfig } from 'common/types/IPlatformConfig'; -import * as Utils from '../../../../common/lib/util/utils'; // The type to which ./msgpack.ts deserializes elements of the `bin` or `ext` type type MessagePackBinaryType = ArrayBuffer; @@ -255,16 +254,7 @@ var createCryptoClass = function (config: IPlatformConfig, bufferUtils: typeof B Logger.logAction(Logger.LOG_MICRO, 'CBCCipher.encrypt()', ''); const encryptAsync = async () => { - const iv = await new Promise((resolve: (iv: IV) => void, reject: (error: Error) => void) => { - this.getIv((error, iv) => { - if (error) { - reject(error); - } else { - resolve(iv!); - } - }); - }); - + const iv = await this.getIv(); const cryptoKey = await crypto.subtle.importKey('raw', this.key, this.webCryptoAlgorithm, false, ['encrypt']); const ciphertext = await crypto.subtle.encrypt({ name: this.webCryptoAlgorithm, iv }, cryptoKey, plaintext); @@ -291,21 +281,15 @@ var createCryptoClass = function (config: IPlatformConfig, bufferUtils: typeof B return crypto.subtle.decrypt({ name: this.webCryptoAlgorithm, iv }, cryptoKey, ciphertextBody); } - getIv(callback: (error: Error | null, iv: ArrayBuffer | null) => void) { + async getIv(): Promise { if (this.iv) { var iv = this.iv; this.iv = null; - callback(null, iv); - return; + return iv; } - Utils.whenPromiseSettles(generateRandom(DEFAULT_BLOCKLENGTH), function (err, randomBlock) { - if (err) { - callback(err as Error, null); - return; - } - callback(null, bufferUtils.toArrayBuffer(randomBlock!)); - }); + const randomBlock = await generateRandom(DEFAULT_BLOCKLENGTH); + return bufferUtils.toArrayBuffer(randomBlock); } } From 2f19925408cabb3159fcc5ff9bddf1b6c78e99e6 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 11:33:14 +0000 Subject: [PATCH 42/44] Convert ICipher.encrypt to use promises Resolves #1532. --- src/common/lib/types/message.ts | 15 ++++-------- src/common/types/ICipher.ts | 2 +- src/platform/nodejs/lib/util/crypto.ts | 33 +++++++++++--------------- src/platform/web/lib/util/crypto.ts | 20 ++++------------ 4 files changed, 24 insertions(+), 46 deletions(-) diff --git a/src/common/lib/types/message.ts b/src/common/lib/types/message.ts index 98e0fa2265..837ab7c0fd 100644 --- a/src/common/lib/types/message.ts +++ b/src/common/lib/types/message.ts @@ -111,17 +111,10 @@ async function encrypt(msg: T, options: Cip data = Platform.BufferUtils.utf8Encode(String(data)); encoding = encoding + 'utf-8/'; } - return new Promise((resolve, reject) => { - cipher.encrypt(data, function (err: Error, data: unknown) { - if (err) { - reject(err); - return; - } - msg.data = data; - msg.encoding = encoding + 'cipher+' + cipher.algorithm; - resolve(msg); - }); - }); + const ciphertext = await cipher.encrypt(data); + msg.data = ciphertext; + msg.encoding = encoding + 'cipher+' + cipher.algorithm; + return msg; } export async function encode(msg: T, options: CipherOptions): Promise { diff --git a/src/common/types/ICipher.ts b/src/common/types/ICipher.ts index 83a7653a78..c508a6cca7 100644 --- a/src/common/types/ICipher.ts +++ b/src/common/types/ICipher.ts @@ -1,5 +1,5 @@ export default interface ICipher { algorithm: string; - encrypt: (plaintext: InputPlaintext, callback: (error: Error | null, data?: OutputCiphertext) => void) => void; + encrypt: (plaintext: InputPlaintext) => Promise; decrypt: (ciphertext: InputCiphertext) => Promise; } diff --git a/src/platform/nodejs/lib/util/crypto.ts b/src/platform/nodejs/lib/util/crypto.ts index 9a3010f47a..330f70a598 100644 --- a/src/platform/nodejs/lib/util/crypto.ts +++ b/src/platform/nodejs/lib/util/crypto.ts @@ -9,7 +9,6 @@ import { CryptoDataTypes } from '../../../../common/types/cryptoDataTypes'; import { Cipher as NodeCipher, CipherKey as NodeCipherKey } from 'crypto'; import BufferUtils, { Bufferlike, Output as BufferUtilsOutput } from './bufferutils'; import util from 'util'; -import * as Utils from '../../../../common/lib/util/utils'; // The type to which ably-forks/msgpack-js deserializes elements of the `bin` or `ext` type type MessagePackBinaryType = Buffer; @@ -223,26 +222,22 @@ var createCryptoClass = function (bufferUtils: typeof BufferUtils) { this.iv = iv; } - encrypt(plaintext: InputPlaintext, callback: (error: Error | null, data?: OutputCiphertext) => void) { - const promise = (async () => { - Logger.logAction(Logger.LOG_MICRO, 'CBCCipher.encrypt()', ''); + async encrypt(plaintext: InputPlaintext): Promise { + Logger.logAction(Logger.LOG_MICRO, 'CBCCipher.encrypt()', ''); - const iv = await this.getIv(); - if (!this.encryptCipher) { - this.encryptCipher = crypto.createCipheriv(this.algorithm, this.key, iv); - } - - var plaintextBuffer = bufferUtils.toBuffer(plaintext); - var plaintextLength = plaintextBuffer.length, - paddedLength = getPaddedLength(plaintextLength); - var cipherOut = this.encryptCipher.update( - Buffer.concat([plaintextBuffer, pkcs5Padding[paddedLength - plaintextLength]]) - ); - var ciphertext = Buffer.concat([iv, toBuffer(cipherOut)]); - return ciphertext; - })(); + const iv = await this.getIv(); + if (!this.encryptCipher) { + this.encryptCipher = crypto.createCipheriv(this.algorithm, this.key, iv); + } - Utils.whenPromiseSettles(promise, callback); + var plaintextBuffer = bufferUtils.toBuffer(plaintext); + var plaintextLength = plaintextBuffer.length, + paddedLength = getPaddedLength(plaintextLength); + var cipherOut = this.encryptCipher.update( + Buffer.concat([plaintextBuffer, pkcs5Padding[paddedLength - plaintextLength]]) + ); + var ciphertext = Buffer.concat([iv, toBuffer(cipherOut)]); + return ciphertext; } async decrypt(ciphertext: InputCiphertext): Promise { diff --git a/src/platform/web/lib/util/crypto.ts b/src/platform/web/lib/util/crypto.ts index 2dcb1c0644..4fa5c34a97 100644 --- a/src/platform/web/lib/util/crypto.ts +++ b/src/platform/web/lib/util/crypto.ts @@ -250,24 +250,14 @@ var createCryptoClass = function (config: IPlatformConfig, bufferUtils: typeof B return output; } - encrypt(plaintext: InputPlaintext, callback: (error: Error | null, data?: OutputCiphertext) => void) { + async encrypt(plaintext: InputPlaintext): Promise { Logger.logAction(Logger.LOG_MICRO, 'CBCCipher.encrypt()', ''); - const encryptAsync = async () => { - const iv = await this.getIv(); - const cryptoKey = await crypto.subtle.importKey('raw', this.key, this.webCryptoAlgorithm, false, ['encrypt']); - const ciphertext = await crypto.subtle.encrypt({ name: this.webCryptoAlgorithm, iv }, cryptoKey, plaintext); + const iv = await this.getIv(); + const cryptoKey = await crypto.subtle.importKey('raw', this.key, this.webCryptoAlgorithm, false, ['encrypt']); + const ciphertext = await crypto.subtle.encrypt({ name: this.webCryptoAlgorithm, iv }, cryptoKey, plaintext); - return this.concat(iv, ciphertext); - }; - - encryptAsync() - .then((ciphertext) => { - callback(null, ciphertext); - }) - .catch((error) => { - callback(error); - }); + return this.concat(iv, ciphertext); } async decrypt(ciphertext: InputCiphertext): Promise { From 509b5c400bdecc07aebd1f891c5a86fd6347bc19 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 12:04:22 +0000 Subject: [PATCH 43/44] Convert IPlatformConfig.getRandomValues to use promises --- src/common/types/IPlatformConfig.d.ts | 2 +- src/platform/nativescript/config.js | 5 +---- src/platform/nodejs/config.ts | 6 +----- src/platform/web/config.ts | 5 +---- src/platform/web/lib/util/crypto.ts | 7 ++----- 5 files changed, 6 insertions(+), 19 deletions(-) diff --git a/src/common/types/IPlatformConfig.d.ts b/src/common/types/IPlatformConfig.d.ts index d6fb7447b4..63fcaad550 100644 --- a/src/common/types/IPlatformConfig.d.ts +++ b/src/common/types/IPlatformConfig.d.ts @@ -23,7 +23,7 @@ export interface ICommonPlatformConfig { */ export interface ISpecificPlatformConfig { addEventListener?: typeof window.addEventListener | typeof global.addEventListener | null; - getRandomValues?: (arr: ArrayBufferView, callback?: (error: Error | null) => void) => void; + getRandomValues?: (arr: ArrayBufferView) => Promise; userAgent?: string | null; inherits?: typeof import('util').inherits; currentUrl?: string; diff --git a/src/platform/nativescript/config.js b/src/platform/nativescript/config.js index ccdece4dd5..9b9f444d79 100644 --- a/src/platform/nativescript/config.js +++ b/src/platform/nativescript/config.js @@ -45,14 +45,11 @@ var Config = { }, TextEncoder: global.TextEncoder, TextDecoder: global.TextDecoder, - getRandomValues: function (arr, callback) { + getRandomValues: async function (arr) { var bytes = randomBytes(arr.length); for (var i = 0; i < arr.length; i++) { arr[i] = bytes[i]; } - if (callback) { - callback(null); - } }, }; diff --git a/src/platform/nodejs/config.ts b/src/platform/nodejs/config.ts index 4947994239..27952ff76b 100644 --- a/src/platform/nodejs/config.ts +++ b/src/platform/nodejs/config.ts @@ -17,17 +17,13 @@ const Config: IPlatformConfig = { stringByteSize: Buffer.byteLength, inherits: util.inherits, addEventListener: null, - getRandomValues: function (arr: ArrayBufferView, callback?: (err: Error | null) => void): void { + getRandomValues: async function (arr: ArrayBufferView): Promise { const bytes = crypto.randomBytes(arr.byteLength); const dataView = new DataView(arr.buffer, arr.byteOffset, arr.byteLength); for (let i = 0; i < bytes.length; i++) { dataView.setUint8(i, bytes[i]); } - - if (callback) { - callback(null); - } }, }; diff --git a/src/platform/web/config.ts b/src/platform/web/config.ts index 24ae69d451..5c3bac6eed 100644 --- a/src/platform/web/config.ts +++ b/src/platform/web/config.ts @@ -71,11 +71,8 @@ const Config: IPlatformConfig = { if (crypto === undefined) { return undefined; } - return function (arr: ArrayBufferView, callback?: (error: Error | null) => void) { + return async function (arr: ArrayBufferView) { crypto.getRandomValues(arr); - if (callback) { - callback(null); - } }; })(globalObject.crypto || msCrypto), isWebworker: isWebWorkerContext(), diff --git a/src/platform/web/lib/util/crypto.ts b/src/platform/web/lib/util/crypto.ts index 4fa5c34a97..c16eeed601 100644 --- a/src/platform/web/lib/util/crypto.ts +++ b/src/platform/web/lib/util/crypto.ts @@ -40,11 +40,8 @@ var createCryptoClass = function (config: IPlatformConfig, bufferUtils: typeof B var blockRandomArray = new Uint32Array(DEFAULT_BLOCKLENGTH_WORDS); var words = bytes / 4, nativeArray = words == DEFAULT_BLOCKLENGTH_WORDS ? blockRandomArray : new Uint32Array(words); - return new Promise((resolve, reject) => { - config.getRandomValues!(nativeArray, (err) => - err ? reject(err) : resolve(bufferUtils.toArrayBuffer(nativeArray)) - ); - }); + await config.getRandomValues!(nativeArray); + return bufferUtils.toArrayBuffer(nativeArray); }; } else { generateRandom = async function (bytes) { From b429c136125426856c9d3d3b4b9ed1b4049b9d4b Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Fri, 5 Jan 2024 12:11:14 +0000 Subject: [PATCH 44/44] Convert IPlatformConfig.getRandomArrayBuffer to use promises Resolves #1530. --- src/common/types/IPlatformConfig.d.ts | 5 +---- src/platform/react-native/config.ts | 8 +++++--- src/platform/web/lib/util/crypto.ts | 6 +----- 3 files changed, 7 insertions(+), 12 deletions(-) diff --git a/src/common/types/IPlatformConfig.d.ts b/src/common/types/IPlatformConfig.d.ts index 63fcaad550..1517f485f1 100644 --- a/src/common/types/IPlatformConfig.d.ts +++ b/src/common/types/IPlatformConfig.d.ts @@ -36,10 +36,7 @@ export interface ISpecificPlatformConfig { atob?: typeof atob | null; TextEncoder?: typeof TextEncoder; TextDecoder?: typeof TextDecoder; - getRandomArrayBuffer?: ( - byteLength: number, - callback: (err: Error | null, result: ArrayBuffer | null) => void - ) => void; + getRandomArrayBuffer?: (byteLength: number) => Promise; isWebworker?: boolean; } diff --git a/src/platform/react-native/config.ts b/src/platform/react-native/config.ts index 6a2f0107b8..72a928a875 100644 --- a/src/platform/react-native/config.ts +++ b/src/platform/react-native/config.ts @@ -31,9 +31,11 @@ export default function (bufferUtils: typeof BufferUtils): IPlatformConfig { TextEncoder: global.TextEncoder, TextDecoder: global.TextDecoder, getRandomArrayBuffer: (function (RNRandomBytes) { - return function (byteLength: number, callback: (err: Error | null, result: ArrayBuffer | null) => void) { - RNRandomBytes.randomBytes(byteLength, function (err: Error | null, base64String: string | null) { - callback(err, base64String ? bufferUtils.toArrayBuffer(bufferUtils.base64Decode(base64String)) : null); + return async function (byteLength: number) { + return new Promise((resolve, reject) => { + RNRandomBytes.randomBytes(byteLength, function (err: Error | null, base64String: string | null) { + err ? reject(err) : resolve(bufferUtils.toArrayBuffer(bufferUtils.base64Decode(base64String!))); + }); }); }; // Installing @types/react-native would fix this but conflicts with @types/node diff --git a/src/platform/web/lib/util/crypto.ts b/src/platform/web/lib/util/crypto.ts index c16eeed601..c54c565a3d 100644 --- a/src/platform/web/lib/util/crypto.ts +++ b/src/platform/web/lib/util/crypto.ts @@ -30,11 +30,7 @@ var createCryptoClass = function (config: IPlatformConfig, bufferUtils: typeof B */ var generateRandom: (byteLength: number) => Promise; if (config.getRandomArrayBuffer) { - generateRandom = async (byteLength) => { - return new Promise((resolve, reject) => { - config.getRandomArrayBuffer!(byteLength, (err, result) => (err ? reject(err) : resolve(result!))); - }); - }; + generateRandom = config.getRandomArrayBuffer; } else if (typeof Uint32Array !== 'undefined' && config.getRandomValues) { generateRandom = async function (bytes) { var blockRandomArray = new Uint32Array(DEFAULT_BLOCKLENGTH_WORDS);