refactor!: update identify method to optionally return results

packages/sdk/browser/src/BrowserClient.ts

@@ -219,11 +219,7 @@ class BrowserClientImpl extends LDClientImpl {
     this._initialContext = context;
   }
 
-  override async identify(context: LDContext, identifyOptions?: LDIdentifyOptions): Promise<void> {
-    return super.identify(context, identifyOptions);
-  }
-
-  override async identifyResult(
+  override async identify(
     context: LDContext,
     identifyOptions?: LDIdentifyOptions,
   ): Promise<LDIdentifyResult> {
@@ -241,7 +237,7 @@ class BrowserClientImpl extends LDClientImpl {
       identifyOptionsWithUpdatedDefaults.sheddable = true;
     }
 
-    const res = await super.identifyResult(context, identifyOptionsWithUpdatedDefaults);
+    const res = await super.identify(context, identifyOptionsWithUpdatedDefaults);
 
     this._goalManager?.startTracking();
     return res;
@@ -291,7 +287,7 @@ class BrowserClientImpl extends LDClientImpl {
 
     this._startPromise = this.promiseWithTimeout(this.initializedPromise!, options?.timeout ?? 5);
 
-    this.identifyResult(this._initialContext!, identifyOptions);
+    this.identify(this._initialContext!, identifyOptions);
     return this._startPromise;
   }
 
@@ -358,7 +354,7 @@ export function makeClient(
     flush: () => impl.flush(),
     setStreaming: (streaming?: boolean) => impl.setStreaming(streaming),
     identify: (pristineContext: LDContext, identifyOptions?: LDIdentifyOptions) =>
-      impl.identifyResult(pristineContext, identifyOptions),
+      impl.identify(pristineContext, identifyOptions),
     getContext: () => impl.getContext(),
     close: () => impl.close(),
     allFlags: () => impl.allFlags(),

packages/shared/sdk-client/__tests__/LDClientImpl.test.ts

@@ -256,7 +256,10 @@ describe('sdk-client object', () => {
   test('identify error invalid context', async () => {
     const carContext: LDContext = { kind: 'car', key: '' };
 
-    await expect(ldc.identify(carContext)).rejects.toThrow(/no key/);
+    await expect(ldc.identify(carContext)).resolves.toEqual({
+      status: 'error',
+      error: new Error('Context was unspecified or had no key'),
+    });
     expect(logger.error).toHaveBeenCalledTimes(1);
     expect(ldc.getContext()).toBeUndefined();
   });
@@ -272,7 +275,10 @@ describe('sdk-client object', () => {
 
     const carContext: LDContext = { kind: 'car', key: 'test-car' };
 
-    await expect(ldc.identify(carContext)).rejects.toThrow('test-error');
+    await expect(ldc.identify(carContext)).resolves.toEqual({
+      status: 'error',
+      error: new Error('test-error'),
+    });
     expect(logger.error).toHaveBeenCalledTimes(2);
     expect(logger.error).toHaveBeenNthCalledWith(1, expect.stringMatching(/^error:.*test-error/));
     expect(logger.error).toHaveBeenNthCalledWith(2, expect.stringContaining('Received error 404'));
@@ -392,7 +398,10 @@ describe('sdk-client object', () => {
     const spyListener = jest.fn();
     ldc.on('dataSourceStatus', spyListener);
     const changePromise = onDataSourceChangePromise(2);
-    await expect(ldc.identify(carContext)).rejects.toThrow('test-error');
+    await expect(ldc.identify(carContext)).resolves.toEqual({
+      status: 'error',
+      error: new Error('test-error'),
+    });
     await changePromise;
 
     expect(spyListener).toHaveBeenCalledTimes(2);

packages/shared/sdk-client/__tests__/LDClientImpl.timeout.test.ts

@@ -65,14 +65,17 @@ describe('sdk-client identify timeout', () => {
 
   test('rejects with default timeout of 5s', async () => {
     jest.advanceTimersByTimeAsync(DEFAULT_IDENTIFY_TIMEOUT * 1000).then();
-    await expect(ldc.identify(carContext)).rejects.toThrow(/identify timed out/);
+    await expect(ldc.identify(carContext)).resolves.toEqual({ status: 'timeout', timeout: 5 });
     expect(logger.error).toHaveBeenCalledWith(expect.stringMatching(/identify timed out/));
   });
 
   test('rejects with custom timeout', async () => {
     const timeout = 15;
     jest.advanceTimersByTimeAsync(timeout * 1000).then();
-    await expect(ldc.identify(carContext, { timeout })).rejects.toThrow(/identify timed out/);
+    await expect(ldc.identify(carContext, { timeout })).resolves.toEqual({
+      status: 'timeout',
+      timeout: 15,
+    });
   });
 
   test('resolves with default timeout', async () => {
@@ -81,7 +84,7 @@ describe('sdk-client identify timeout', () => {
 
     jest.advanceTimersByTimeAsync(DEFAULT_IDENTIFY_TIMEOUT * 1000).then();
 
-    await expect(ldc.identify(carContext)).resolves.toBeUndefined();
+    await expect(ldc.identify(carContext)).resolves.toEqual({ status: 'completed' });
 
     expect(ldc.getContext()).toEqual(expect.objectContaining(toMulti(carContext)));
     expect(ldc.allFlags()).toEqual({
@@ -106,7 +109,7 @@ describe('sdk-client identify timeout', () => {
 
     jest.advanceTimersByTimeAsync(timeout).then();
 
-    await expect(ldc.identify(carContext, { timeout })).resolves.toBeUndefined();
+    await expect(ldc.identify(carContext, { timeout })).resolves.toEqual({ status: 'completed' });
 
     expect(ldc.getContext()).toEqual(expect.objectContaining(toMulti(carContext)));
     expect(ldc.allFlags()).toEqual({
@@ -208,7 +211,7 @@ describe('sdk-client waitForInitialization', () => {
     simulatedEvents = [{ data: JSON.stringify(defaultPutResponse) }];
 
     const waitPromise = ldc.waitForInitialization({ timeout: 10 });
-    const identifyPromise = ldc.identifyResult(carContext);
+    const identifyPromise = ldc.identify(carContext);
 
     jest.advanceTimersByTimeAsync(DEFAULT_IDENTIFY_TIMEOUT).then();
 
@@ -222,7 +225,7 @@ describe('sdk-client waitForInitialization', () => {
     simulatedEvents = [{ data: JSON.stringify(defaultPutResponse) }];
 
     const waitPromise = ldc.waitForInitialization({ timeout: 10 });
-    const identifyPromise = ldc.identifyResult(carContext);
+    const identifyPromise = ldc.identify(carContext);
 
     jest.advanceTimersByTimeAsync(DEFAULT_IDENTIFY_TIMEOUT).then();
 
@@ -240,7 +243,7 @@ describe('sdk-client waitForInitialization', () => {
     simulatedEvents = [];
 
     const waitPromise = ldc.waitForInitialization({ timeout: 10 });
-    const identifyPromise = ldc.identifyResult(carContext);
+    const identifyPromise = ldc.identify(carContext);
 
     jest.advanceTimersByTimeAsync(10 * 1000 + 1).then();
 
@@ -274,7 +277,7 @@ describe('sdk-client waitForInitialization', () => {
     );
 
     const waitPromise = errorLdc.waitForInitialization({ timeout: 10 });
-    const identifyPromise = errorLdc.identifyResult(carContext);
+    const identifyPromise = errorLdc.identify(carContext);
 
     // Advance timers to allow error handler to be set up and error to propagate
     await jest.advanceTimersByTimeAsync(DEFAULT_IDENTIFY_TIMEOUT);

packages/shared/sdk-client/__tests__/LDClientImpl.variation.test.ts

@@ -77,7 +77,7 @@ describe('sdk-client object', () => {
     const p = ldc.identify(context);
     ldc.variation('does-not-exist', 'not-found');
 
-    await expect(p).resolves.toBeUndefined();
+    await expect(p).resolves.toEqual({ status: 'completed' });
     expect(errorListener).toHaveBeenCalledTimes(1);
     const error = errorListener.mock.calls[0][1];
     expect(error.message).toMatch(/unknown feature/i);

packages/shared/sdk-client/src/LDClientImpl.ts

@@ -12,15 +12,13 @@ import {
   LDHeaders,
   LDLogger,
   LDPluginEnvironmentMetadata,
-  LDTimeoutError,
   Platform,
   TypeValidators,
 } from '@launchdarkly/js-sdk-common';
 
 import {
   Hook,
   LDClient,
-  LDClientIdentifyResult,
   LDIdentifyError,
   LDIdentifyResult,
   LDIdentifyShed,
@@ -64,7 +62,7 @@ const { ClientMessages, ErrorKinds } = internal;
 
 const DEFAULT_IDENTIFY_TIMEOUT_SECONDS = 5;
 
-export default class LDClientImpl implements LDClient, LDClientIdentifyResult {
+export default class LDClientImpl implements LDClient {
   private readonly _config: Configuration;
   private readonly _diagnosticsManager?: internal.DiagnosticsManager;
   private _eventProcessor?: internal.EventProcessor;
@@ -280,23 +278,7 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult {
    *
    * 3. A network error is encountered during initialization.
    */
-  async identify(pristineContext: LDContext, identifyOptions?: LDIdentifyOptions): Promise<void> {
-    // In order to manage customization in the derived classes it is important that `identify` MUST be implemented in
-    // terms of `identifyResult`. So that the logic of the identification process can be extended in one place.
-    const result = await this.identifyResult(pristineContext, identifyOptions);
-    if (result.status === 'error') {
-      throw result.error;
-    } else if (result.status === 'timeout') {
-      const timeoutError = new LDTimeoutError(
-        `identify timed out after ${result.timeout} seconds.`,
-      );
-      this.logger.error(timeoutError.message);
-      throw timeoutError;
-    }
-    // If completed or shed, then we are done.
-  }
-
-  async identifyResult(
+  async identify(
     pristineContext: LDContext,
     identifyOptions?: LDIdentifyOptions,
   ): Promise<LDIdentifyResult> {
@@ -397,7 +379,14 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult {
         resolve({ status: 'timeout', timeout: identifyTimeout } as LDIdentifyTimeout);
       }, identifyTimeout * 1000);
     });
-    return Promise.race([callSitePromise, timeoutPromise]);
+
+    return Promise.race([callSitePromise, timeoutPromise]).then((result: LDIdentifyResult) => {
+      if (result.status === 'timeout') {
+        this.logger?.error(`identify timed out after ${identifyTimeout} seconds.`);
+      }
+
+      return result;
+    });
   }
 
   /**

packages/shared/sdk-client/src/api/LDClient.ts

@@ -88,7 +88,8 @@ export interface LDClient {
   getContext(): LDContext | undefined;
 
   /**
-   * Identifies a context to LaunchDarkly.
+   * Identifies a context to LaunchDarkly and returns a promise which resolves to an object containing the result of
+   * the identify operation.
    *
    * Unlike the server-side SDKs, the client-side JavaScript SDKs maintain a current context state,
    * which is set when you call `identify()`.
@@ -106,17 +107,10 @@ export interface LDClient {
    * @param identifyOptions
    *    Optional configuration. Please see {@link LDIdentifyOptions}.
    * @returns
-   *    A Promise which resolves when the flag values for the specified
-   * context are available. It rejects when:
-   *
-   * 1. The context is unspecified or has no key.
-   *
-   * 2. The identify timeout is exceeded. In client SDKs this defaults to 5s.
-   * You can customize this timeout with {@link LDIdentifyOptions | identifyOptions}.
-   *
-   * 3. A network error is encountered during initialization.
+   *    A promise which resolves to an object containing the result of the identify operation.
+   *    The promise returned from this method will not be rejected.
    */
-  identify(context: LDContext, identifyOptions?: LDIdentifyOptions): Promise<void>;
+  identify(context: LDContext, identifyOptions?: LDIdentifyOptions): Promise<LDIdentifyResult>;
 
   /**
    * Determines the json variation of a feature flag.
@@ -373,39 +367,3 @@ export interface LDClient {
     options?: LDWaitForInitializationOptions,
   ): Promise<LDWaitForInitializationResult>;
 }
-
-/**
- * Interface that extends the LDClient interface to include the identifyResult method.
- *
- * This is an independent interface for backwards compatibility. Adding this to the LDClient interface would require
- * a breaking change.
- */
-export interface LDClientIdentifyResult {
-  /**
-   * Identifies a context to LaunchDarkly and returns a promise which resolves to an object containing the result of
-   * the identify operation.
-   *
-   * Unlike the server-side SDKs, the client-side JavaScript SDKs maintain a current context state,
-   * which is set when you call `identify()`.
-   *
-   * Changing the current context also causes all feature flag values to be reloaded. Until that has
-   * finished, calls to {@link variation} will still return flag values for the previous context. You can
-   * await the Promise to determine when the new flag values are available.
-   *
-   * If used with the `sheddable` option set to true, then the identify operation will be sheddable. This means that if
-   * multiple identify operations are done, without waiting for the previous one to complete, then intermediate
-   * operations may be discarded.
-   *
-   * @param context
-   *    The LDContext object.
-   * @param identifyOptions
-   *    Optional configuration. Please see {@link LDIdentifyOptions}.
-   * @returns
-   *    A promise which resolves to an object containing the result of the identify operation.
-   *    The promise returned from this method will not be rejected.
-   */
-  identifyResult(
-    context: LDContext,
-    identifyOptions?: LDIdentifyOptions,
-  ): Promise<LDIdentifyResult>;
-}

packages/shared/sdk-client/src/index.ts

@@ -35,7 +35,6 @@ export type {
   LDIdentifyError,
   LDIdentifyTimeout,
   LDIdentifyShed,
-  LDClientIdentifyResult,
   LDPluginBase,
   LDWaitForInitializationOptions,
   LDWaitForInitializationResult,