Skip to content

Commit

Permalink
Improve deprecation log messages
Browse files Browse the repository at this point in the history
Add more detail that addresses what the user might have been trying to
achieve with their use of the deprecated API, and more detail about how
to change their code.
  • Loading branch information
lawrence-forooghian committed Mar 12, 2024
1 parent 346da17 commit 76200a6
Show file tree
Hide file tree
Showing 7 changed files with 33 additions and 20 deletions.
7 changes: 3 additions & 4 deletions src/common/lib/client/auth.ts
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,9 @@ class Auth {
}

if (_authOptions && 'force' in _authOptions) {
Logger.logAction(
Logger.LOG_ERROR,
'Auth.authorize',
'Deprecation warning: specifying {force: true} in authOptions is no longer necessary, authorize() now always gets a new token. Please remove this, as in version 1.0 and later, having a non-null authOptions will overwrite stored library authOptions, which may not be what you want'
Logger.deprecated(
'The `force` auth option',
'If you’re using this option to force `authorize()` to fetch a new token even if the current token has not expired, this is no longer necessary, as `authorize()` now always fetches a new token. Update your code to no longer pass the `force` auth option. Note that, in general, passing an auth options argument to `authorize()` will overwrite the library’s stored auth options, which may not be what you want. The library currently contains a special case behavior where passing an auth options object which only contains `{ force: true }` will _not_ overwrite the stored options. This special case behavior will be removed alongside support for the `force` option, so if you’re currently passing `authorize()` an auth options object which only contains `{ force: true }`, you should stop passing it an auth options object entirely.'
);
/* Emulate the old behaviour: if 'force' was the only member of authOptions,
* set it to null so it doesn't overwrite stored. TODO: remove in version 1.0 */
Expand Down
5 changes: 4 additions & 1 deletion src/common/lib/client/realtimechannel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,7 +316,10 @@ class RealtimeChannel extends Channel {
};
}
if (_flags) {
Logger.deprecated('channel.attach() with flags', 'channel.setOptions() with channelOptions.params');
Logger.deprecated(
'The ability to pass an array of channel mode flags as the first argument of `RealtimeChannel.attach()`',
'To set channel mode flags, populate the `modes` property of the channel options object that you pass to `Channels.get()` or `RealtimeChannel.setOptions()`.'
);
/* If flags requested, always do a re-attach. TODO only do this if
* current mode differs from requested mode */
this._requestedFlags = _flags as API.Types.ChannelMode[];
Expand Down
5 changes: 4 additions & 1 deletion src/common/lib/client/rest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,10 @@ class Rest {

if (optionsObj.log) {
Logger.setLog(optionsObj.log.level, optionsObj.log.handler);
Logger.deprecated('the `log` client option', 'the `logLevel` and `logHandler` client options');
Logger.deprecated(
'The `log` client option',
'Equivalent functionality is provided by the `logLevel` and `logHandler` client options. Update your client options code of the form `{ log: { level: logLevel, handler: logHandler } }` to instead be `{ logLevel, logHandler }`.'
);
} else {
Logger.setLog(optionsObj.logLevel, optionsObj.logHandler);
}
Expand Down
18 changes: 12 additions & 6 deletions src/common/lib/util/defaults.ts
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ export function normaliseOptions(options: DeprecatedClientOptions): NormalisedCl
options.queueMessages = options.queueEvents;
}
if (options.headers) {
Logger.deprecatedWithMsg(
Logger.deprecated(
'the `headers` client option',
'' /* there is no replacement; see DeprecatedClientOptions.headers */
);
Expand All @@ -219,12 +219,15 @@ export function normaliseOptions(options: DeprecatedClientOptions): NormalisedCl

/* emit an appropriate deprecation warning */
if (options.environment) {
Logger.deprecatedWithMsg(
'fallbackHostsUseDefault',
'There is no longer a need to set this when the environment option is also set since the library will now generate the correct fallback hosts using the environment option.'
Logger.deprecated(
'The `fallbackHostsUseDefault` client option',
'If you’re using this client option to force the library to make use of fallback hosts even though you’ve set the `environment` client option, then this is no longer necessary: remove your usage of the `fallbackHostsUseDefault` client option and the library will then automatically choose the correct fallback hosts to use for the specified environment.'
);
} else {
Logger.deprecated('fallbackHostsUseDefault', 'fallbackHosts: Ably.Defaults.FALLBACK_HOSTS');
Logger.deprecated(
'The `fallbackHostsUseDefault` client option',
'If you’re using this client option to force the library to make use of fallback hosts even though you’re not using the primary Ably environment, then stop using `fallbackHostsUseDefault`, and update your code to either pass the `environment` client option (in which case the library will automatically choose the correct fallback hosts to use for the specified environment), or to pass the `fallbackHosts` client option to specify a custom list of fallback hosts to use (for example, if you’re using a custom CNAME, in which case Ably will have provided you with an explicit list of fallback hosts).'
);
}

/* use the default fallback hosts as requested */
Expand All @@ -233,7 +236,10 @@ export function normaliseOptions(options: DeprecatedClientOptions): NormalisedCl

/* options.recover as a boolean is deprecated, and therefore is not part of the public typing */
if ((options.recover as any) === true) {
Logger.deprecated('{recover: true}', '{recover: function(lastConnectionDetails, cb) { cb(true); }}');
Logger.deprecated(
'The ability to use a boolean value for the `recover` client option',
'If you wish for the connection to always be recovered, replace `{ recover: true }` with a function that always passes `true` to its callback: `{ recover: function(lastConnectionDetails, cb) { cb(true); } }`'
);
options.recover = function (lastConnectionDetails: unknown, cb: (shouldRecover: boolean) => void) {
cb(true);
};
Expand Down
8 changes: 2 additions & 6 deletions src/common/lib/util/logger.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,12 +104,8 @@ class Logger {
}
};

static deprecated = function (original: string, replacement: string) {
Logger.deprecatedWithMsg(original, "Please use '" + replacement + "' instead.");
};

static deprecatedWithMsg = (funcName: string, msg: string) => {
Logger.deprecationWarning(`'${funcName}' is deprecated and will be removed in a future version. ${msg}`);
static deprecated = (description: string, msg: string) => {
Logger.deprecationWarning(`${description} is deprecated and will be removed in a future version. ${msg}`);
};

static renamedClientOption(oldName: string, newName: string) {
Expand Down
5 changes: 4 additions & 1 deletion src/platform/nodejs/lib/util/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,10 @@ var Crypto = (function () {
var key;
/* Backward compatibility */
if (typeof params === 'function' || typeof params === 'string') {
Logger.deprecated('Crypto.getDefaultParams(key, callback)', 'Crypto.getDefaultParams({key: key})');
Logger.deprecated(
'The ability to pass the encryption key as the first argument of `Crypto.getDefaultParams()`',
'Please update your code so that it instead passes an object whose `key` property contains the key. That is, replace `Crypto.getDefaultParams(key)` with `Crypto.getDefaultParams({ key })`.'
);
if (typeof params === 'function') {
Crypto.generateRandomKey(function (key) {
params(null, Crypto.getDefaultParams({ key: key }));
Expand Down
5 changes: 4 additions & 1 deletion src/platform/web/lib/util/crypto.js
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,10 @@ var CryptoFactory = function (config, bufferUtils) {
var key;
/* Backward compatibility */
if (typeof params === 'function' || typeof params === 'string') {
Logger.deprecated('Crypto.getDefaultParams(key, callback)', 'Crypto.getDefaultParams({key: key})');
Logger.deprecated(
'The ability to pass the encryption key as the first argument of `Crypto.getDefaultParams()`',
'Please update your code so that it instead passes an object whose `key` property contains the key. That is, replace `Crypto.getDefaultParams(key)` with `Crypto.getDefaultParams({ key })`.'
);
if (typeof params === 'function') {
Crypto.generateRandomKey(function (key) {
params(null, Crypto.getDefaultParams({ key: key }));
Expand Down

0 comments on commit 76200a6

Please sign in to comment.