Improve errors for incorrect constructor args

This fixes the regression that I introduced in e8c3012, which stopped
the user from getting a useful error message when not passing any args
to the constructor. I’ve also introduced a wider range of error messages
to give the user useful guidance depending on which variant of the
library they’re using.

Resolves #1700.

src/common/lib/client/baseclient.ts

@@ -48,12 +48,6 @@ class BaseClient {
   constructor(options: ClientOptions) {
     this._additionalHTTPRequestImplementations = options.plugins ?? null;
 
-    if (!options) {
-      const msg = 'no options provided';
-      Logger.logAction(Logger.LOG_ERROR, 'BaseClient()', msg);
-      throw new Error(msg);
-    }
-
     Logger.setLog(options.logLevel, options.logHandler);
     Logger.logAction(
       Logger.LOG_MICRO,

src/common/lib/client/baserealtime.ts

@@ -24,8 +24,16 @@ class BaseRealtime extends BaseClient {
   _channels: any;
   connection: Connection;
 
-  constructor(options: ClientOptions | string) {
-    super(Defaults.objectifyOptions(options));
+  /*
+   * The public typings declare that this only accepts an object, but since we want to emit a good error message in the case where a non-TypeScript user does one of these things:
+   *
+   * 1. passes a string (which is quite likely if they’re e.g. migrating from the default variant to the modular variant)
+   * 2. passes no argument at all
+   *
+   * tell the compiler that these cases are possible so that it forces us to handle them.
+   */
+  constructor(options?: ClientOptions | string) {
+    super(Defaults.objectifyOptions(options, false, 'BaseRealtime'));
     Logger.logAction(Logger.LOG_MINOR, 'Realtime()', '');
     this._additionalTransportImplementations = BaseRealtime.transportImplementationsFromPlugins(this.options.plugins);
     this._RealtimePresence = this.options.plugins?.RealtimePresence ?? null;

src/common/lib/client/baserest.ts

@@ -9,7 +9,15 @@ import Defaults from '../util/defaults';
  It always includes the `Rest` plugin.
  */
 export class BaseRest extends BaseClient {
-  constructor(options: ClientOptions | string) {
-    super(Defaults.objectifyOptions(options, { Rest }));
+  /*
+   * The public typings declare that this only accepts an object, but since we want to emit a good error message in the case where a non-TypeScript user does one of these things:
+   *
+   * 1. passes a string (which is quite likely if they’re e.g. migrating from the default variant to the modular variant)
+   * 2. passes no argument at all
+   *
+   * tell the compiler that these cases are possible so that it forces us to handle them.
+   */
+  constructor(options?: ClientOptions | string) {
+    super(Defaults.objectifyOptions(options, false, 'BaseRest', { Rest }));
   }
 }

src/common/lib/client/defaultrealtime.ts

@@ -22,14 +22,15 @@ import Defaults from '../util/defaults';
  `DefaultRealtime` is the class that the non tree-shakable version of the SDK exports as `Realtime`. It ensures that this version of the SDK includes all of the functionality which is optionally available in the tree-shakable version.
  */
 export class DefaultRealtime extends BaseRealtime {
-  constructor(options: ClientOptions | string) {
+  // The public typings declare that this requires an argument to be passed, but since we want to emit a good error message in the case where a non-TypeScript user does not pass an argument, tell the compiler that this is possible so that it forces us to handle it.
+  constructor(options?: ClientOptions | string) {
     const MsgPack = DefaultRealtime._MsgPack;
     if (!MsgPack) {
       throw new Error('Expected DefaultRealtime._MsgPack to have been set');
     }
 
     super(
-      Defaults.objectifyOptions(options, {
+      Defaults.objectifyOptions(options, true, 'Realtime', {
         ...allCommonModularPlugins,
         Crypto: DefaultRealtime.Crypto ?? undefined,
         MsgPack,

src/common/lib/client/defaultrest.ts

@@ -12,14 +12,15 @@ import Defaults from '../util/defaults';
  `DefaultRest` is the class that the non tree-shakable version of the SDK exports as `Rest`. It ensures that this version of the SDK includes all of the functionality which is optionally available in the tree-shakable version.
  */
 export class DefaultRest extends BaseRest {
-  constructor(options: ClientOptions | string) {
+  // The public typings declare that this requires an argument to be passed, but since we want to emit a good error message in the case where a non-TypeScript user does not pass an argument, tell the compiler that this is possible so that it forces us to handle it.
+  constructor(options?: ClientOptions | string) {
     const MsgPack = DefaultRest._MsgPack;
     if (!MsgPack) {
       throw new Error('Expected DefaultRest._MsgPack to have been set');
     }
 
     super(
-      Defaults.objectifyOptions(options, {
+      Defaults.objectifyOptions(options, true, 'Rest', {
         ...allCommonModularPlugins,
         Crypto: DefaultRest.Crypto ?? undefined,
         MsgPack: DefaultRest._MsgPack ?? undefined,

src/common/lib/util/defaults.ts

@@ -44,7 +44,12 @@ type CompleteDefaults = IDefaults & {
   getHosts(options: NormalisedClientOptions): string[];
   checkHost(host: string): void;
   getRealtimeHost(options: ClientOptions, production: boolean, environment: string): string;
-  objectifyOptions(options: ClientOptions | string, modularPluginsToInclude?: ModularPlugins): ClientOptions;
+  objectifyOptions(
+    options: undefined | ClientOptions | string,
+    allowKeyOrToken: boolean,
+    sourceForErrorMessage: string,
+    modularPluginsToInclude?: ModularPlugins,
+  ): ClientOptions;
   normaliseOptions(options: ClientOptions, MsgPack: MsgPack | null): NormalisedClientOptions;
   defaultGetHeaders(options: NormalisedClientOptions, headersOptions?: HeadersOptions): Record<string, string>;
   defaultPostHeaders(options: NormalisedClientOptions, headersOptions?: HeadersOptions): Record<string, string>;
@@ -183,11 +188,42 @@ export function getAgentString(options: ClientOptions): string {
 }
 
 export function objectifyOptions(
-  options: ClientOptions | string,
+  options: undefined | ClientOptions | string,
+  allowKeyOrToken: boolean,
+  sourceForErrorMessage: string,
   modularPluginsToInclude?: ModularPlugins,
 ): ClientOptions {
-  let optionsObj =
-    typeof options === 'string' ? (options.indexOf(':') == -1 ? { token: options } : { key: options }) : options;
+  if (options === undefined) {
+    const msg = allowKeyOrToken
+      ? `${sourceForErrorMessage} must be initialized with either a client options object, an Ably API key, or an Ably Token`
+      : `${sourceForErrorMessage} must be initialized with a client options object`;
+    Logger.logAction(Logger.LOG_ERROR, `${sourceForErrorMessage}()`, msg);
+    throw new Error(msg);
+  }
+
+  let optionsObj: ClientOptions;
+
+  if (typeof options === 'string') {
+    if (options.indexOf(':') == -1) {
+      if (!allowKeyOrToken) {
+        const msg = `${sourceForErrorMessage} cannot be initialized with just an Ably Token; you must provide a client options object with a \`plugins\` property. (Set this Ably Token as the object’s \`token\` property.)`;
+        Logger.logAction(Logger.LOG_ERROR, `${sourceForErrorMessage}()`, msg);
+        throw new Error(msg);
+      }
+
+      optionsObj = { token: options };
+    } else {
+      if (!allowKeyOrToken) {
+        const msg = `${sourceForErrorMessage} cannot be initialized with just an Ably API key; you must provide a client options object with a \`plugins\` property. (Set this Ably API key as the object’s \`key\` property.)`;
+        Logger.logAction(Logger.LOG_ERROR, `${sourceForErrorMessage}()`, msg);
+        throw new Error(msg);
+      }
+
+      optionsObj = { key: options };
+    }
+  } else {
+    optionsObj = options;
+  }
 
   if (modularPluginsToInclude) {
     optionsObj = { ...optionsObj, plugins: { ...modularPluginsToInclude, ...optionsObj.plugins } };

test/browser/modular.test.js

@@ -42,6 +42,40 @@ function registerAblyModularTests(helper) {
       helper.setupApp(done);
     });
 
+    describe('attempting to initialize with no client options', () => {
+      for (const clientClass of [BaseRest, BaseRealtime]) {
+        describe(clientClass.name, () => {
+          it('throws an error', () => {
+            expect(() => new clientClass()).to.throw('must be initialized with a client options object');
+          });
+        });
+      }
+    });
+
+    describe('attempting to initialize with just an API key', () => {
+      for (const clientClass of [BaseRest, BaseRealtime]) {
+        describe(clientClass.name, () => {
+          it('throws an error', () => {
+            expect(() => new clientClass('foo:bar')).to.throw(
+              'cannot be initialized with just an Ably API key; you must provide a client options object with a `plugins` property',
+            );
+          });
+        });
+      }
+    });
+
+    describe('attempting to initialize with just a token', () => {
+      for (const clientClass of [BaseRest, BaseRealtime]) {
+        describe(clientClass.name, () => {
+          it('throws an error', () => {
+            expect(() => new clientClass('foo')).to.throw(
+              'cannot be initialized with just an Ably Token; you must provide a client options object with a `plugins` property',
+            );
+          });
+        });
+      }
+    });
+
     describe('without any plugins', () => {
       for (const clientClass of [BaseRest, BaseRealtime]) {
         describe(clientClass.name, () => {

test/realtime/api.test.js

@@ -8,6 +8,12 @@ define(['ably', 'chai'], function (Ably, chai) {
       expect(typeof Ably.Realtime).to.equal('function');
     });
 
+    it('constructor without any arguments', function () {
+      expect(() => new Ably.Realtime()).to.throw(
+        'must be initialized with either a client options object, an Ably API key, or an Ably Token',
+      );
+    });
+
     it('Crypto', function () {
       expect(typeof Ably.Realtime.Crypto).to.equal('function');
       expect(typeof Ably.Realtime.Crypto.getDefaultParams).to.equal('function');

test/rest/api.test.js

@@ -8,6 +8,12 @@ define(['ably', 'chai'], function (Ably, chai) {
       expect(typeof Ably.Rest).to.equal('function');
     });
 
+    it('constructor without any arguments', function () {
+      expect(() => new Ably.Rest()).to.throw(
+        'must be initialized with either a client options object, an Ably API key, or an Ably Token',
+      );
+    });
+
     it('Crypto', function () {
       expect(typeof Ably.Rest.Crypto).to.equal('function');
       expect(typeof Ably.Rest.Crypto.getDefaultParams).to.equal('function');