diff --git a/packages/shared/common/src/utils/timedPromise.ts b/packages/shared/common/src/utils/timedPromise.ts index 6aa101d3bc..420bf95516 100644 --- a/packages/shared/common/src/utils/timedPromise.ts +++ b/packages/shared/common/src/utils/timedPromise.ts @@ -1,17 +1,13 @@ -import { LDLogger } from '../api'; - /** * Returns a promise which errors after t seconds. * * @param t Timeout in seconds. * @param taskName Name of task being timed for logging and error reporting. - * @param logger {@link LDLogger} object. */ -const timedPromise = (t: number, taskName: string, logger?: LDLogger) => +const timedPromise = (t: number, taskName: string) => new Promise((_res, reject) => { setTimeout(() => { const e = `${taskName} timed out after ${t} seconds.`; - logger?.error(e); reject(new Error(e)); }, t * 1000); }); diff --git a/packages/shared/mocks/src/streamingProcessor.ts b/packages/shared/mocks/src/streamingProcessor.ts index 87ba783cd2..260db1654e 100644 --- a/packages/shared/mocks/src/streamingProcessor.ts +++ b/packages/shared/mocks/src/streamingProcessor.ts @@ -28,11 +28,9 @@ export const setupMockStreamingProcessor = ( start: jest.fn(async () => { if (shouldError) { setTimeout(() => { - const unauthorized: LDStreamingError = { - code: 401, - name: 'LaunchDarklyStreamingError', - message: 'test-error', - }; + const unauthorized = new Error('test-error') as LDStreamingError; + // @ts-ignore + unauthorized.code = 401; errorHandler(unauthorized); }, errorTimeoutSeconds * 1000); } else { diff --git a/packages/shared/sdk-client/src/LDClientImpl.test.ts b/packages/shared/sdk-client/src/LDClientImpl.test.ts index 9c6416892c..e803d86122 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.test.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.test.ts @@ -115,37 +115,6 @@ describe('sdk-client object', () => { }); }); - test('variation', async () => { - await ldc.identify(context); - const devTestFlag = ldc.variation('dev-test-flag'); - - expect(devTestFlag).toBe(true); - }); - - test('variationDetail flag not found', async () => { - await ldc.identify(context); - const flag = ldc.variationDetail('does-not-exist', 'not-found'); - - expect(flag).toEqual({ - reason: { errorKind: 'FLAG_NOT_FOUND', kind: 'ERROR' }, - value: 'not-found', - variationIndex: null, - }); - }); - - test('variationDetail deleted flag not found', async () => { - await ldc.identify(context); - // @ts-ignore - ldc.flags['dev-test-flag'].deleted = true; - const flag = ldc.variationDetail('dev-test-flag', 'deleted'); - - expect(flag).toEqual({ - reason: { errorKind: 'FLAG_NOT_FOUND', kind: 'ERROR' }, - value: 'deleted', - variationIndex: null, - }); - }); - test('identify success', async () => { defaultPutResponse['dev-test-flag'].value = false; const carContext: LDContext = { kind: 'car', key: 'test-car' }; @@ -240,11 +209,9 @@ describe('sdk-client object', () => { setupMockStreamingProcessor(true); const carContext: LDContext = { kind: 'car', key: 'test-car' }; - await expect(ldc.identify(carContext)).rejects.toMatchObject({ - code: 401, - message: 'test-error', - }); + await expect(ldc.identify(carContext)).rejects.toThrow('test-error'); expect(logger.error).toHaveBeenCalledTimes(1); + expect(logger.error).toHaveBeenCalledWith(expect.stringMatching(/^error:.*test-error/)); expect(ldc.getContext()).toBeUndefined(); }); diff --git a/packages/shared/sdk-client/src/LDClientImpl.timeout.test.ts b/packages/shared/sdk-client/src/LDClientImpl.timeout.test.ts index fbb1c53092..fe3e2c3c5c 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.timeout.test.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.timeout.test.ts @@ -61,7 +61,6 @@ describe('sdk-client identify timeout', () => { 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/); }); diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index 7aa983d3f5..8cdf53dc38 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -46,8 +46,6 @@ export default class LDClientImpl implements LDClient { private eventFactoryWithReasons = new EventFactory(true); private emitter: LDEmitter; private flags: Flags = {}; - private identifyChangeListener?: (c: LDContext, changedKeys: string[]) => void; - private identifyErrorListener?: (c: LDContext, err: any) => void; private readonly clientContext: ClientContext; @@ -81,6 +79,12 @@ export default class LDClientImpl implements LDClient { !this.isOffline(), ); this.emitter = new LDEmitter(); + this.emitter.on('change', (c: LDContext, changedKeys: string[]) => { + this.logger.debug(`change: context: ${JSON.stringify(c)}, flags: ${changedKeys}`); + }); + this.emitter.on('error', (c: LDContext, err: any) => { + this.logger.error(`error: ${err}, context: ${JSON.stringify(c)}`); + }); } /** @@ -244,32 +248,22 @@ export default class LDClientImpl implements LDClient { private createIdentifyPromise(timeout: number) { let res: any; + let rej: any; + const slow = new Promise((resolve, reject) => { res = resolve; + rej = reject; + }); - if (this.identifyChangeListener) { - this.emitter.off('change', this.identifyChangeListener); - } - if (this.identifyErrorListener) { - this.emitter.off('error', this.identifyErrorListener); + const timed = timedPromise(timeout, 'identify'); + const raced = Promise.race([timed, slow]).catch((e) => { + if (e.message.includes('timed out')) { + this.logger.error(`identify error: ${e}`); } - - this.identifyChangeListener = (c: LDContext, changedKeys: string[]) => { - this.logger.debug(`change: context: ${JSON.stringify(c)}, flags: ${changedKeys}`); - }; - this.identifyErrorListener = (c: LDContext, err: Error) => { - this.logger.debug(`error: ${err}, context: ${JSON.stringify(c)}`); - reject(err); - }; - - this.emitter.on('change', this.identifyChangeListener); - this.emitter.on('error', this.identifyErrorListener); + throw e; }); - const timed = timedPromise(timeout, 'identify', this.logger); - const raced = Promise.race([timed, slow]); - - return { identifyPromise: raced, identifyResolve: res }; + return { identifyPromise: raced, identifyResolve: res, identifyReject: rej }; } private async getFlagsFromStorage(canonicalKey: string): Promise { @@ -282,8 +276,15 @@ export default class LDClientImpl implements LDClient { * * @param pristineContext The LDContext object to be identified. * @param identifyOptions Optional configuration. 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. * - * @returns {Promise}. + * 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. */ async identify(pristineContext: LDContext, identifyOptions?: LDIdentifyOptions): Promise { if (identifyOptions?.timeout) { @@ -303,13 +304,14 @@ export default class LDClientImpl implements LDClient { const checkedContext = Context.fromLDContext(context); if (!checkedContext.valid) { const error = new Error('Context was unspecified or had no key'); - this.logger.error(error); this.emitter.emit('error', context, error); return Promise.reject(error); } this.eventProcessor?.sendEvent(this.eventFactoryDefault.identifyEvent(checkedContext)); - const { identifyPromise, identifyResolve } = this.createIdentifyPromise(this.identifyTimeout); + const { identifyPromise, identifyResolve, identifyReject } = this.createIdentifyPromise( + this.identifyTimeout, + ); this.logger.debug(`Identifying ${JSON.stringify(context)}`); const flagsStorage = await this.getFlagsFromStorage(checkedContext.canonicalKey); @@ -340,7 +342,7 @@ export default class LDClientImpl implements LDClient { this.createStreamListeners(context, checkedContext.canonicalKey, identifyResolve), this.diagnosticsManager, (e) => { - this.logger.error(e); + identifyReject(e); this.emitter.emit('error', context, e); }, ); @@ -387,12 +389,12 @@ export default class LDClientImpl implements LDClient { track(key: string, data?: any, metricValue?: number): void { if (!this.context) { - this.logger?.warn(ClientMessages.missingContextKeyNoEvent); + this.logger.warn(ClientMessages.missingContextKeyNoEvent); return; } const checkedContext = Context.fromLDContext(this.context); if (!checkedContext.valid) { - this.logger?.warn(ClientMessages.missingContextKeyNoEvent); + this.logger.warn(ClientMessages.missingContextKeyNoEvent); return; } @@ -408,7 +410,7 @@ export default class LDClientImpl implements LDClient { typeChecker?: (value: any) => [boolean, string], ): LDFlagValue { if (!this.context) { - this.logger?.debug(ClientMessages.missingContextKeyNoEvent); + this.logger.debug(ClientMessages.missingContextKeyNoEvent); return createErrorEvaluationDetail(ErrorKinds.UserNotSpecified, defaultValue); } @@ -420,7 +422,6 @@ export default class LDClientImpl implements LDClient { const error = new LDClientError( `Unknown feature flag "${flagKey}"; returning default value ${defVal}.`, ); - this.logger.error(error); this.emitter.emit('error', this.context, error); this.eventProcessor?.sendEvent( this.eventFactoryDefault.unknownFlagEvent(flagKey, defVal, evalContext), @@ -446,7 +447,6 @@ export default class LDClientImpl implements LDClient { const error = new LDClientError( `Wrong type "${type}" for feature flag "${flagKey}"; returning default value`, ); - this.logger.error(error); this.emitter.emit('error', this.context, error); return createErrorEvaluationDetail(ErrorKinds.WrongType, defaultValue); } diff --git a/packages/shared/sdk-client/src/LDClientImpl.variation.test.ts b/packages/shared/sdk-client/src/LDClientImpl.variation.test.ts new file mode 100644 index 0000000000..06ca449c0d --- /dev/null +++ b/packages/shared/sdk-client/src/LDClientImpl.variation.test.ts @@ -0,0 +1,93 @@ +import { AutoEnvAttributes, clone, LDContext } from '@launchdarkly/js-sdk-common'; +import { basicPlatform, logger, setupMockStreamingProcessor } from '@launchdarkly/private-js-mocks'; + +import * as mockResponseJson from './evaluation/mockResponse.json'; +import LDClientImpl from './LDClientImpl'; +import { Flags } from './types'; + +jest.mock('@launchdarkly/js-sdk-common', () => { + const actual = jest.requireActual('@launchdarkly/js-sdk-common'); + const actualMock = jest.requireActual('@launchdarkly/private-js-mocks'); + return { + ...actual, + ...{ + internal: { + ...actual.internal, + StreamingProcessor: actualMock.MockStreamingProcessor, + }, + }, + }; +}); + +const testSdkKey = 'test-sdk-key'; +const context: LDContext = { kind: 'org', key: 'Testy Pizza' }; + +let ldc: LDClientImpl; +let defaultPutResponse: Flags; + +describe('sdk-client object', () => { + beforeEach(() => { + defaultPutResponse = clone(mockResponseJson); + setupMockStreamingProcessor(false, defaultPutResponse); + ldc = new LDClientImpl(testSdkKey, AutoEnvAttributes.Enabled, basicPlatform, { + logger, + sendEvents: false, + }); + jest + .spyOn(LDClientImpl.prototype as any, 'createStreamUriPath') + .mockReturnValue('/stream/path'); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + test('variation', async () => { + await ldc.identify(context); + const devTestFlag = ldc.variation('dev-test-flag'); + + expect(devTestFlag).toBe(true); + }); + + test('variation flag not found', async () => { + // set context manually to pass validation + ldc.context = { kind: 'user', key: 'test-user' }; + const errorListener = jest.fn().mockName('errorListener'); + ldc.on('error', errorListener); + + const p = ldc.identify(context); + setTimeout(() => { + // call variation in the next tick to give ldc a chance to hook up event emitter + ldc.variation('does-not-exist', 'not-found'); + }); + + await expect(p).resolves.toBeUndefined(); + const error = errorListener.mock.calls[0][1]; + expect(errorListener).toHaveBeenCalledTimes(1); + expect(error.message).toMatch(/unknown feature/i); + }); + + test('variationDetail flag not found', async () => { + await ldc.identify(context); + const flag = ldc.variationDetail('does-not-exist', 'not-found'); + + expect(flag).toEqual({ + reason: { errorKind: 'FLAG_NOT_FOUND', kind: 'ERROR' }, + value: 'not-found', + variationIndex: null, + }); + }); + + test('variationDetail deleted flag not found', async () => { + await ldc.identify(context); + // @ts-ignore + ldc.flags['dev-test-flag'].deleted = true; + const flag = ldc.variationDetail('dev-test-flag', 'deleted'); + + expect(flag).toEqual({ + reason: { errorKind: 'FLAG_NOT_FOUND', kind: 'ERROR' }, + value: 'deleted', + variationIndex: null, + }); + }); +}); diff --git a/packages/shared/sdk-client/src/api/LDClient.ts b/packages/shared/sdk-client/src/api/LDClient.ts index 98b3bd924e..1eb7cd3fb8 100644 --- a/packages/shared/sdk-client/src/api/LDClient.ts +++ b/packages/shared/sdk-client/src/api/LDClient.ts @@ -112,7 +112,15 @@ 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. + * 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. */ identify(context: LDContext, identifyOptions?: LDIdentifyOptions): Promise;