Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
226 changes: 226 additions & 0 deletions ably.d.ts

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion scripts/moduleReport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { gzip } from 'zlib';
import Table from 'cli-table';

// The maximum size we allow for a minimal useful Realtime bundle (i.e. one that can subscribe to a channel)
const minimalUsefulRealtimeBundleSizeThresholdsKiB = { raw: 106, gzip: 32 };
const minimalUsefulRealtimeBundleSizeThresholdsKiB = { raw: 107, gzip: 33 };

const baseClientNames = ['BaseRest', 'BaseRealtime'];

Expand Down
30 changes: 27 additions & 3 deletions src/common/lib/client/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,15 @@ class Auth {
*/
async authorize(tokenParams: API.TokenParams | null, authOptions: AuthOptions | null): Promise<API.TokenDetails>;

async authorize(
authorize(...args: unknown[]): Promise<API.TokenDetails> {
Utils.detectV1Callback(args, 0);
return this._authorizeImpl(
args[0] as Record<string, any> | null | undefined,
args[1] as AuthOptions | null | undefined,
);
}

private async _authorizeImpl(
tokenParams?: Record<string, any> | null,
authOptions?: AuthOptions | null,
): Promise<API.TokenDetails> {
Expand Down Expand Up @@ -390,7 +398,15 @@ class Auth {
*/
async requestToken(tokenParams: API.TokenParams | null, authOptions: AuthOptions): Promise<API.TokenDetails>;

async requestToken(tokenParams?: API.TokenParams | null, authOptions?: AuthOptions): Promise<API.TokenDetails> {
requestToken(...args: unknown[]): Promise<API.TokenDetails> {
Utils.detectV1Callback(args, 0);
return this._requestTokenImpl(args[0] as API.TokenParams | null | undefined, args[1] as AuthOptions | undefined);
}

private async _requestTokenImpl(
tokenParams?: API.TokenParams | null,
authOptions?: AuthOptions,
): Promise<API.TokenDetails> {
/* RSA8e: if authOptions passed in, they're used instead of stored, don't merge them */
const resolvedAuthOptions = authOptions || this.authOptions;
const resolvedTokenParams = tokenParams || Utils.copy(this.tokenParams);
Expand Down Expand Up @@ -744,7 +760,15 @@ 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.
*/
async createTokenRequest(tokenParams: API.TokenParams | null, authOptions: any): Promise<API.TokenRequest> {
createTokenRequest(...args: unknown[]): Promise<API.TokenRequest> {
Utils.detectV1Callback(args, 0);
return this._createTokenRequestImpl(args[0] as API.TokenParams | null, args[1]);
}

private async _createTokenRequestImpl(
tokenParams: API.TokenParams | null,
authOptions: any,
): Promise<API.TokenRequest> {
/* RSA9h: if authOptions passed in, they're used instead of stored, don't merge them */
authOptions = authOptions || this.authOptions;
tokenParams = tokenParams || Utils.copy<API.TokenParams>(this.tokenParams);
Expand Down
8 changes: 7 additions & 1 deletion src/common/lib/client/connection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import ConnectionManager from '../transport/connectionmanager';
import Logger from '../util/logger';
import ConnectionStateChange from './connectionstatechange';
import ErrorInfo from '../types/errorinfo';
import * as Utils from '../util/utils';
import { NormalisedClientOptions } from '../../types/ClientOptions';
import BaseRealtime from './baserealtime';
import Platform from 'common/platform';
Expand Down Expand Up @@ -46,7 +47,12 @@ class Connection extends EventEmitter {
this.connectionManager.requestState({ state: 'connecting' });
}

async ping(): Promise<number> {
ping(...args: unknown[]): Promise<number> {
Utils.detectV1Callback(args, 0);
return this._pingImpl();
}

private async _pingImpl(): Promise<number> {
Logger.logAction(this.logger, Logger.LOG_MINOR, 'Connection.ping()', '');
return this.connectionManager.ping();
}
Expand Down
21 changes: 18 additions & 3 deletions src/common/lib/client/realtimechannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -250,7 +250,12 @@ class RealtimeChannel extends EventEmitter {
return false;
}

async publish(...args: any[]): Promise<API.PublishResult> {
publish(...args: unknown[]): Promise<API.PublishResult> {
Utils.detectV1Callback(args, 0);
return this._publishImpl(args);
}

private async _publishImpl(args: any[]): Promise<API.PublishResult> {
const first = args[0],
second = args[1];
let messages: Message[];
Expand Down Expand Up @@ -453,7 +458,12 @@ class RealtimeChannel extends EventEmitter {
this.send(msg);
}

async subscribe(...args: unknown[] /* [event], listener */): Promise<ChannelStateChange | null> {
subscribe(...args: unknown[] /* [event], listener */): Promise<ChannelStateChange | null> {
Utils.detectV1Callback(args, 2);
return this._subscribeImpl(args);
}
Comment thread
umair-ably marked this conversation as resolved.

private async _subscribeImpl(args: unknown[]): Promise<ChannelStateChange | null> {
const [event, listener] = RealtimeChannel.processListenerArgs(args);

if (this.state === 'failed') {
Expand Down Expand Up @@ -985,7 +995,12 @@ class RealtimeChannel extends EventEmitter {
}
}

history = async function (
history = function (this: RealtimeChannel, ...args: unknown[]): Promise<PaginatedResult<Message>> {
Utils.detectV1Callback(args, 0);
return this._historyImpl(args[0] as RealtimeHistoryParams | null);
} as any;

private _historyImpl = async function (
this: RealtimeChannel,
params: RealtimeHistoryParams | null,
): Promise<PaginatedResult<Message>> {
Expand Down
42 changes: 36 additions & 6 deletions src/common/lib/client/realtimepresence.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,14 +54,24 @@ class RealtimePresence extends EventEmitter {
this.pendingPresence = [];
}

async enter(data: unknown): Promise<void> {
enter(...args: unknown[]): Promise<void> {
Utils.detectV1Callback(args, 0);
return this._enterImpl(args[0]);
}

private async _enterImpl(data: unknown): Promise<void> {
if (isAnonymousOrWildcard(this)) {
throw new ErrorInfo('clientId must be specified to enter a presence channel', 40012, 400);
}
return this._enterOrUpdateClient(undefined, undefined, data, 'enter');
}

async update(data: unknown): Promise<void> {
update(...args: unknown[]): Promise<void> {
Utils.detectV1Callback(args, 0);
return this._updateImpl(args[0]);
}

private async _updateImpl(data: unknown): Promise<void> {
if (isAnonymousOrWildcard(this)) {
throw new ErrorInfo('clientId must be specified to update presence data', 40012, 400);
}
Expand Down Expand Up @@ -129,7 +139,12 @@ class RealtimePresence extends EventEmitter {
}
}

async leave(data: unknown): Promise<void> {
leave(...args: unknown[]): Promise<void> {
Utils.detectV1Callback(args, 0);
return this._leaveImpl(args[0]);
}

private async _leaveImpl(data: unknown): Promise<void> {
if (isAnonymousOrWildcard(this)) {
throw new ErrorInfo('clientId must have been specified to enter or leave a presence channel', 40012, 400);
}
Expand Down Expand Up @@ -176,7 +191,12 @@ class RealtimePresence extends EventEmitter {
}
}

async get(params?: RealtimePresenceParams): Promise<PresenceMessage[]> {
get(...args: unknown[]): Promise<PresenceMessage[]> {
Utils.detectV1Callback(args, 0);
return this._getImpl(args[0] as RealtimePresenceParams | undefined);
}

private async _getImpl(params?: RealtimePresenceParams): Promise<PresenceMessage[]> {
const waitForSync = !params || ('waitForSync' in params ? params.waitForSync : true);

function toMessages(members: PresenceMap): PresenceMessage[] {
Expand Down Expand Up @@ -204,7 +224,12 @@ class RealtimePresence extends EventEmitter {
return toMessages(this.members);
}

async history(params: RealtimeHistoryParams | null): Promise<PaginatedResult<PresenceMessage>> {
history(...args: unknown[]): Promise<PaginatedResult<PresenceMessage>> {
Utils.detectV1Callback(args, 0);
return this._historyImpl(args[0] as RealtimeHistoryParams | null);
}

private async _historyImpl(params: RealtimeHistoryParams | null): Promise<PaginatedResult<PresenceMessage>> {
Logger.logAction(this.logger, Logger.LOG_MICRO, 'RealtimePresence.history()', 'channel = ' + this.name);
// We fetch this first so that any plugin-not-provided error takes priority over other errors
const restMixin = this.channel.client.rest.presenceMixin;
Expand Down Expand Up @@ -407,7 +432,12 @@ class RealtimePresence extends EventEmitter {
});
}

async subscribe(..._args: unknown[] /* [event], listener */): Promise<void> {
subscribe(..._args: unknown[] /* [event], listener */): Promise<void> {
Utils.detectV1Callback(_args, 2);
return this._subscribeImpl(_args);
Comment thread
umair-ably marked this conversation as resolved.
}

private async _subscribeImpl(_args: unknown[]): Promise<void> {
const args = RealtimeChannel.processListenerArgs(_args);
const event = args[0];
const listener = args[1];
Expand Down
4 changes: 4 additions & 0 deletions src/common/lib/types/errorinfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ export interface IPartialErrorInfo extends Error {
cause?: ErrorInfo | PartialErrorInfo;
href?: string;
detail?: Record<string, string>;
hint?: string;
Comment thread
umair-ably marked this conversation as resolved.
}

function toString(err: ErrorInfo | PartialErrorInfo) {
Expand All @@ -16,6 +17,7 @@ function toString(err: ErrorInfo | PartialErrorInfo) {
if (err.statusCode) result += '; statusCode=' + err.statusCode;
if (err.code) result += '; code=' + err.code;
if (err.cause) result += '; cause=' + Utils.inspectError(err.cause);
if (err.hint) result += '; hint=' + err.hint;
if (err.detail && Object.keys(err.detail).length > 0) result += '; detail=' + JSON.stringify(err.detail);
if (err.href && !(err.message && err.message.indexOf('help.ably.io') > -1)) result += '; see ' + err.href + ' ';
result += ']';
Expand All @@ -42,6 +44,7 @@ export default class ErrorInfo extends Error implements IPartialErrorInfo, API.E
cause?: ErrorInfo;
href?: string;
detail?: Record<string, string>;
hint?: string;

constructor(message: string, code: number, statusCode: number, cause?: ErrorInfo, detail?: Record<string, string>) {
super(message);
Expand Down Expand Up @@ -82,6 +85,7 @@ export class PartialErrorInfo extends Error implements IPartialErrorInfo {
cause?: ErrorInfo | PartialErrorInfo;
href?: string;
detail?: Record<string, string>;
hint?: string;

constructor(
message: string,
Expand Down
32 changes: 32 additions & 0 deletions src/common/lib/util/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -263,6 +263,38 @@ export function isErrorInfoOrPartialErrorInfo(err: unknown): err is ErrorInfo |
return typeof err == 'object' && err !== null && (err instanceof ErrorInfo || err instanceof PartialErrorInfo);
}

/**
* Detect a v1-style trailing callback on a public method's args list and throw a
* steering error. v2 removed callback support from these methods, but agents trained
* on v1 docs keep passing callbacks — without this check, the callback is silently
* swallowed and the call hangs (subscribe) or no-ops (publish).
*
* Apply only to methods whose v1 form accepted a Node-style (err, result) callback
* and which are promise-only in v2. Do not apply to APIs that legitimately accept a
* trailing function (e.g. ClientOptions.authCallback).
*
* Fires when the trailing arg is a function AND either:
* - `args.length > v2TrailingFnArity` (the trailing fn is beyond the arity at
* which v2 legitimately ends in a function), or
* - the second-to-last arg is also a function — none of the v2 forms take two
* trailing functions, so e.g. `subscribe(listener, callback)` is always v1
* even though it matches the arity of `subscribe(event, listener)`.
*
* @param v2TrailingFnArity - The arity at which v2 legitimately ends in a function
* (e.g. `subscribe(event, listener)` → 2). For methods where v2 never ends in a
* function (`authorize`, `publish`, `requestToken`, ...), pass 0.
*/
export function detectV1Callback(args: ArrayLike<unknown>, v2TrailingFnArity: number): void {
const n = args.length;
if (typeof args[n - 1] !== 'function') return;
if (n <= v2TrailingFnArity && typeof args[n - 2] !== 'function') return;
const err = new ErrorInfo('v1 callback signature is no longer supported.', 40025, 400);
err.hint =
'v2 uses Promises — drop the trailing callback and `await` the returned promise. ' +
'See https://github.com/ably/ably-js/blob/main/docs/migration-guides/v2/lib.md';
throw err;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be calling out "go and use the Promise signature" explicitly rather than "dont use a callback"

}

export function inspectError(err: unknown): string {
if (
err instanceof Error ||
Expand Down
6 changes: 3 additions & 3 deletions src/platform/react-hooks/src/hooks/useChannel.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as Ably from 'ably';
import { useCallback, useEffect, useMemo, useRef } from 'react';
import { useEffect, useMemo, useRef } from 'react';
import { ChannelParameters } from '../AblyReactHooks.js';
import { useAbly } from './useAbly.js';
import { useStateErrors } from './useStateErrors.js';
Expand Down Expand Up @@ -56,8 +56,8 @@ export function useChannel(

const ablyMessageCallbackRef = useRef(ablyMessageCallback);

const history: Ably.RealtimeChannel['history'] = useCallback(
(params?: Ably.RealtimeHistoryParams) => channel.history(params),
const history: Ably.RealtimeChannel['history'] = useMemo(
() => ((params?: Ably.RealtimeHistoryParams) => channel.history(params)) as Ably.RealtimeChannel['history'],
[channel],
);

Expand Down
36 changes: 36 additions & 0 deletions test/realtime/auth.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1643,5 +1643,41 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, Helper, async
});
};
});

/**
* v1-style trailing-callback shape on Auth.{authorize, requestToken,
* createTokenRequest} throws synchronously with a steering ErrorInfo.
*/
[
{ method: 'authorize', invoke: (auth) => auth.authorize(null, null, () => {}) },
{ method: 'requestToken', invoke: (auth) => auth.requestToken(null, null, () => {}) },
{ method: 'createTokenRequest', invoke: (auth) => auth.createTokenRequest(null, null, () => {}) },
].forEach(function (testCase) {
it('v1_callback_auth_' + testCase.method + '_throws_synchronously', function (done) {
var helper = this.test.helper,
realtime = helper.AblyRealtime({ autoConnect: false });

try {
testCase.invoke(realtime.auth);
helper.closeAndFinish(
done,
realtime,
new Error('expected auth.' + testCase.method + '() to throw on v1 callback shape'),
);
} catch (err) {
try {
expect(err.code).to.equal(40025);
expect(err.message).to.contain('v1 callback signature');
expect(err.message).to.contain('no longer supported');
expect(err.hint).to.be.a('string');
expect(err.hint).to.contain('v2 uses Promises');
expect(err.hint).to.contain('https://github.com/ably/ably-js/blob/main/docs/migration-guides/v2/lib.md');
helper.closeAndFinish(done, realtime);
} catch (assertionErr) {
helper.closeAndFinish(done, realtime, assertionErr);
}
}
});
});
});
});
55 changes: 55 additions & 0 deletions test/realtime/channel.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,61 @@ define(['ably', 'shared_helper', 'async', 'chai'], function (Ably, Helper, async
});
});

/**
* v1-style trailing-callback shape on RealtimeChannel.{publish, subscribe,
* history} throws synchronously with a steering ErrorInfo whose message
* diagnoses *what* went wrong and whose hint prescribes the v2 replacement
* call.
*/
[
{ method: 'publish', invoke: (ch) => ch.publish('event', { hello: 'world' }, () => {}) },
{
method: 'subscribe',
invoke: (ch) =>
ch.subscribe(
'event',
() => {},
() => {},
),
},
{
method: 'subscribe_listener_callback',
invoke: (ch) =>
ch.subscribe(
() => {},
() => {},
),
},
{ method: 'history', invoke: (ch) => ch.history(null, () => {}) },
].forEach(function (testCase) {
it('v1_callback_' + testCase.method + '_throws_synchronously', function (done) {
var helper = this.test.helper,
realtime = helper.AblyRealtime({ autoConnect: false }),
channel = realtime.channels.get('v1cb_channel_' + testCase.method);

try {
testCase.invoke(channel);
helper.closeAndFinish(
done,
realtime,
new Error('expected ' + testCase.method + '() to throw on v1 callback shape'),
);
} catch (err) {
try {
expect(err.code).to.equal(40025);
expect(err.message).to.contain('v1 callback signature');
expect(err.message).to.contain('no longer supported');
expect(err.hint).to.be.a('string');
expect(err.hint).to.contain('v2 uses Promises');
expect(err.hint).to.contain('https://github.com/ably/ably-js/blob/main/docs/migration-guides/v2/lib.md');
helper.closeAndFinish(done, realtime);
} catch (assertionErr) {
helper.closeAndFinish(done, realtime, assertionErr);
}
}
});
});

/**
* A channel attach that times out should be retried
*
Expand Down
Loading
Loading