diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index 496e75e24..88e6822c6 100644 --- a/packages/sdk/browser/src/BrowserClient.ts +++ b/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 { - return super.identify(context, identifyOptions); - } - - override async identifyResult( + override async identify( context: LDContext, identifyOptions?: LDIdentifyOptions, ): Promise { @@ -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(), diff --git a/packages/shared/sdk-client/__tests__/LDClientImpl.test.ts b/packages/shared/sdk-client/__tests__/LDClientImpl.test.ts index 2241c81c0..ab538d714 100644 --- a/packages/shared/sdk-client/__tests__/LDClientImpl.test.ts +++ b/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); diff --git a/packages/shared/sdk-client/__tests__/LDClientImpl.timeout.test.ts b/packages/shared/sdk-client/__tests__/LDClientImpl.timeout.test.ts index 9572d0526..3fb606903 100644 --- a/packages/shared/sdk-client/__tests__/LDClientImpl.timeout.test.ts +++ b/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); diff --git a/packages/shared/sdk-client/__tests__/LDClientImpl.variation.test.ts b/packages/shared/sdk-client/__tests__/LDClientImpl.variation.test.ts index d6fbd74b9..8365b0fc0 100644 --- a/packages/shared/sdk-client/__tests__/LDClientImpl.variation.test.ts +++ b/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); diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index 71bca2293..0e9e66e75 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -12,7 +12,6 @@ import { LDHeaders, LDLogger, LDPluginEnvironmentMetadata, - LDTimeoutError, Platform, TypeValidators, } from '@launchdarkly/js-sdk-common'; @@ -20,7 +19,6 @@ import { 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 { - // 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 { @@ -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; + }); } /** diff --git a/packages/shared/sdk-client/src/api/LDClient.ts b/packages/shared/sdk-client/src/api/LDClient.ts index 0cae2a9dd..601fe6b6b 100644 --- a/packages/shared/sdk-client/src/api/LDClient.ts +++ b/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; + identify(context: LDContext, identifyOptions?: LDIdentifyOptions): Promise; /** * Determines the json variation of a feature flag. @@ -373,39 +367,3 @@ export interface LDClient { options?: LDWaitForInitializationOptions, ): Promise; } - -/** - * 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; -} diff --git a/packages/shared/sdk-client/src/index.ts b/packages/shared/sdk-client/src/index.ts index 3750eb0b9..4238c14c1 100644 --- a/packages/shared/sdk-client/src/index.ts +++ b/packages/shared/sdk-client/src/index.ts @@ -35,7 +35,6 @@ export type { LDIdentifyError, LDIdentifyTimeout, LDIdentifyShed, - LDClientIdentifyResult, LDPluginBase, LDWaitForInitializationOptions, LDWaitForInitializationResult,