From 76200a6efc4da981e8dd3a19f99d0a10daabc153 Mon Sep 17 00:00:00 2001 From: Lawrence Forooghian Date: Mon, 11 Mar 2024 09:56:55 -0300 Subject: [PATCH] Improve deprecation log messages 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. --- src/common/lib/client/auth.ts | 7 +++---- src/common/lib/client/realtimechannel.ts | 5 ++++- src/common/lib/client/rest.ts | 5 ++++- src/common/lib/util/defaults.ts | 18 ++++++++++++------ src/common/lib/util/logger.ts | 8 ++------ src/platform/nodejs/lib/util/crypto.js | 5 ++++- src/platform/web/lib/util/crypto.js | 5 ++++- 7 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/common/lib/client/auth.ts b/src/common/lib/client/auth.ts index 6feada99ab..feef6678b5 100644 --- a/src/common/lib/client/auth.ts +++ b/src/common/lib/client/auth.ts @@ -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 */ diff --git a/src/common/lib/client/realtimechannel.ts b/src/common/lib/client/realtimechannel.ts index 802a133072..c2b1db6f39 100644 --- a/src/common/lib/client/realtimechannel.ts +++ b/src/common/lib/client/realtimechannel.ts @@ -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[]; diff --git a/src/common/lib/client/rest.ts b/src/common/lib/client/rest.ts index 5eea47c705..a4dae85f2c 100644 --- a/src/common/lib/client/rest.ts +++ b/src/common/lib/client/rest.ts @@ -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); } diff --git a/src/common/lib/util/defaults.ts b/src/common/lib/util/defaults.ts index 842e1964b7..86f292a558 100644 --- a/src/common/lib/util/defaults.ts +++ b/src/common/lib/util/defaults.ts @@ -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 */ ); @@ -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 */ @@ -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); }; diff --git a/src/common/lib/util/logger.ts b/src/common/lib/util/logger.ts index ca857d4edd..f81a8c076a 100644 --- a/src/common/lib/util/logger.ts +++ b/src/common/lib/util/logger.ts @@ -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) { diff --git a/src/platform/nodejs/lib/util/crypto.js b/src/platform/nodejs/lib/util/crypto.js index ef03ddea14..18b82424b5 100644 --- a/src/platform/nodejs/lib/util/crypto.js +++ b/src/platform/nodejs/lib/util/crypto.js @@ -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 })); diff --git a/src/platform/web/lib/util/crypto.js b/src/platform/web/lib/util/crypto.js index 7f6d192ef4..abd24da9ff 100644 --- a/src/platform/web/lib/util/crypto.js +++ b/src/platform/web/lib/util/crypto.js @@ -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 }));