diff --git a/packages/sdk/react-native/example/src/welcome.tsx b/packages/sdk/react-native/example/src/welcome.tsx index f5447f65df..028c0dffb6 100644 --- a/packages/sdk/react-native/example/src/welcome.tsx +++ b/packages/sdk/react-native/example/src/welcome.tsx @@ -12,7 +12,7 @@ export default function Welcome() { const onIdentify = () => { ldc - .identify({ kind: 'user', key: userKey }) + .identify({ kind: 'user', key: userKey }, { timeout: 5 }) .catch((e: any) => console.error(`error identifying ${userKey}: ${e}`)); }; diff --git a/packages/sdk/react-native/src/ReactNativeLDClient.test.ts b/packages/sdk/react-native/src/ReactNativeLDClient.test.ts index 439a98c1cb..aafe5b1553 100644 --- a/packages/sdk/react-native/src/ReactNativeLDClient.test.ts +++ b/packages/sdk/react-native/src/ReactNativeLDClient.test.ts @@ -10,6 +10,7 @@ describe('ReactNativeLDClient', () => { }); test('constructing a new client', () => { + expect(ldc.highTimeoutThreshold).toEqual(15); expect(ldc.sdkKey).toEqual('mobile-key'); expect(ldc.config.serviceEndpoints).toEqual({ analyticsEventPath: '/mobile', diff --git a/packages/sdk/react-native/src/ReactNativeLDClient.ts b/packages/sdk/react-native/src/ReactNativeLDClient.ts index 8c60490386..9ccff1f392 100644 --- a/packages/sdk/react-native/src/ReactNativeLDClient.ts +++ b/packages/sdk/react-native/src/ReactNativeLDClient.ts @@ -47,6 +47,7 @@ export default class ReactNativeLDClient extends LDClientImpl { const internalOptions: internal.LDInternalOptions = { analyticsEventPath: `/mobile`, diagnosticEventPath: `/mobile/events/diagnostic`, + highTimeoutThreshold: 15, }; super( diff --git a/packages/shared/common/src/internal/events/LDInternalOptions.ts b/packages/shared/common/src/internal/events/LDInternalOptions.ts index 898197051a..b852990a3e 100644 --- a/packages/shared/common/src/internal/events/LDInternalOptions.ts +++ b/packages/shared/common/src/internal/events/LDInternalOptions.ts @@ -12,4 +12,13 @@ export type LDInternalOptions = { analyticsEventPath?: string; diagnosticEventPath?: string; includeAuthorizationHeader?: boolean; + + /** + * In seconds. Log a warning if identifyTimeout is greater than this value. + * + * Mobile - 15s. + * Browser - 5s. + * Server - 60s. + */ + highTimeoutThreshold?: number; }; diff --git a/packages/shared/common/src/utils/index.ts b/packages/shared/common/src/utils/index.ts index 2c2130fe37..a6f49e0770 100644 --- a/packages/shared/common/src/utils/index.ts +++ b/packages/shared/common/src/utils/index.ts @@ -6,6 +6,7 @@ import fastDeepEqual from './fast-deep-equal'; import { base64UrlEncode, defaultHeaders, httpErrorMessage, LDHeaders, shouldRetry } from './http'; import noop from './noop'; import sleep from './sleep'; +import timedPromise from './timedPromise'; import { VoidFunction } from './VoidFunction'; export { @@ -21,5 +22,6 @@ export { shouldRetry, secondsToMillis, sleep, + timedPromise, VoidFunction, }; diff --git a/packages/shared/common/src/utils/timedPromise.ts b/packages/shared/common/src/utils/timedPromise.ts new file mode 100644 index 0000000000..6aa101d3bc --- /dev/null +++ b/packages/shared/common/src/utils/timedPromise.ts @@ -0,0 +1,19 @@ +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) => + new Promise((_res, reject) => { + setTimeout(() => { + const e = `${taskName} timed out after ${t} seconds.`; + logger?.error(e); + reject(new Error(e)); + }, t * 1000); + }); + +export default timedPromise; diff --git a/packages/shared/mocks/src/streamingProcessor.ts b/packages/shared/mocks/src/streamingProcessor.ts index e8891a89ea..87ba783cd2 100644 --- a/packages/shared/mocks/src/streamingProcessor.ts +++ b/packages/shared/mocks/src/streamingProcessor.ts @@ -13,6 +13,7 @@ export const setupMockStreamingProcessor = ( putResponseJson: any = { data: { flags: {}, segments: {} } }, patchResponseJson?: any, deleteResponseJson?: any, + errorTimeoutSeconds: number = 0, ) => { MockStreamingProcessor.mockImplementation( ( @@ -33,7 +34,7 @@ export const setupMockStreamingProcessor = ( message: 'test-error', }; errorHandler(unauthorized); - }); + }, errorTimeoutSeconds * 1000); } else { // execute put which will resolve the identify promise setTimeout(() => listeners.get('put')?.processJson(putResponseJson)); diff --git a/packages/shared/sdk-client/src/LDClientImpl.timeout.test.ts b/packages/shared/sdk-client/src/LDClientImpl.timeout.test.ts new file mode 100644 index 0000000000..fbb1c53092 --- /dev/null +++ b/packages/shared/sdk-client/src/LDClientImpl.timeout.test.ts @@ -0,0 +1,148 @@ +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'; +import { toMulti } from './utils/addAutoEnv'; + +jest.mock('@launchdarkly/js-sdk-common', () => { + const actual = jest.requireActual('@launchdarkly/js-sdk-common'); + const m = jest.requireActual('@launchdarkly/private-js-mocks'); + return { + ...actual, + ...{ + internal: { + ...actual.internal, + StreamingProcessor: m.MockStreamingProcessor, + }, + }, + }; +}); + +const testSdkKey = 'test-sdk-key'; +const carContext: LDContext = { kind: 'car', key: 'test-car' }; + +let ldc: LDClientImpl; +let defaultPutResponse: Flags; + +describe('sdk-client identify timeout', () => { + beforeAll(() => { + jest.useFakeTimers(); + }); + + beforeEach(() => { + defaultPutResponse = clone(mockResponseJson); + + // simulate streamer error after a long timeout + setupMockStreamingProcessor(true, defaultPutResponse, undefined, undefined, 30); + + ldc = new LDClientImpl(testSdkKey, AutoEnvAttributes.Enabled, basicPlatform, { + logger, + sendEvents: false, + }); + jest + .spyOn(LDClientImpl.prototype as any, 'createStreamUriPath') + .mockReturnValue('/stream/path'); + }); + + afterEach(() => { + jest.resetAllMocks(); + }); + + // streamer is setup to error in beforeEach to cause a timeout + test('rejects with default timeout of 5s', async () => { + jest.advanceTimersByTimeAsync(ldc.identifyTimeout * 1000).then(); + await expect(ldc.identify(carContext)).rejects.toThrow(/identify timed out/); + expect(logger.error).toHaveBeenCalledWith(expect.stringMatching(/identify timed out/)); + }); + + // streamer is setup to error in beforeEach to cause a 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/); + }); + + test('resolves with default timeout', async () => { + setupMockStreamingProcessor(false, defaultPutResponse); + jest.advanceTimersByTimeAsync(ldc.identifyTimeout * 1000).then(); + + await expect(ldc.identify(carContext)).resolves.toBeUndefined(); + + expect(ldc.getContext()).toEqual(expect.objectContaining(toMulti(carContext))); + expect(ldc.allFlags()).toEqual({ + 'dev-test-flag': true, + 'easter-i-tunes-special': false, + 'easter-specials': 'no specials', + fdsafdsafdsafdsa: true, + 'log-level': 'warn', + 'moonshot-demo': true, + test1: 's1', + 'this-is-a-test': true, + }); + }); + + test('resolves with custom timeout', async () => { + const timeout = 15; + setupMockStreamingProcessor(false, defaultPutResponse); + jest.advanceTimersByTimeAsync(timeout).then(); + + await expect(ldc.identify(carContext, { timeout })).resolves.toBeUndefined(); + + expect(ldc.getContext()).toEqual(expect.objectContaining(toMulti(carContext))); + expect(ldc.allFlags()).toEqual({ + 'dev-test-flag': true, + 'easter-i-tunes-special': false, + 'easter-specials': 'no specials', + fdsafdsafdsafdsa: true, + 'log-level': 'warn', + 'moonshot-demo': true, + test1: 's1', + 'this-is-a-test': true, + }); + }); + + test('setting high timeout threshold with internalOptions', async () => { + const highTimeoutThreshold = 20; + setupMockStreamingProcessor(false, defaultPutResponse); + ldc = new LDClientImpl( + testSdkKey, + AutoEnvAttributes.Enabled, + basicPlatform, + { + logger, + sendEvents: false, + }, + { highTimeoutThreshold }, + ); + const customTimeout = 10; + jest.advanceTimersByTimeAsync(customTimeout * 1000).then(); + await ldc.identify(carContext, { timeout: customTimeout }); + + expect(ldc.identifyTimeout).toEqual(10); + expect(logger.warn).not.toHaveBeenCalledWith(expect.stringMatching(/high timeout/)); + }); + + test('warning when timeout is too high', async () => { + const highTimeout = 60; + setupMockStreamingProcessor(false, defaultPutResponse); + jest.advanceTimersByTimeAsync(highTimeout * 1000).then(); + + await ldc.identify(carContext, { timeout: highTimeout }); + + expect(logger.warn).toHaveBeenCalledWith(expect.stringMatching(/high timeout/)); + }); + + test('safe timeout should not warn', async () => { + const { identifyTimeout } = ldc; + const safeTimeout = identifyTimeout; + setupMockStreamingProcessor(false, defaultPutResponse); + jest.advanceTimersByTimeAsync(identifyTimeout * 1000).then(); + + await ldc.identify(carContext, { timeout: safeTimeout }); + + expect(logger.warn).not.toHaveBeenCalledWith(expect.stringMatching(/high timeout/)); + }); +}); diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index 57efc8bd4a..7aa983d3f5 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -14,11 +14,13 @@ import { Platform, ProcessStreamResponse, EventName as StreamEventName, + timedPromise, TypeValidators, } from '@launchdarkly/js-sdk-common'; import { ConnectionMode, LDClient, type LDOptions } from './api'; import LDEmitter, { EventName } from './api/LDEmitter'; +import { LDIdentifyOptions } from './api/LDIdentifyOptions'; import Configuration from './configuration'; import createDiagnosticsManager from './diagnostics/createDiagnosticsManager'; import createEventProcessor from './events/createEventProcessor'; @@ -34,9 +36,12 @@ export default class LDClientImpl implements LDClient { context?: LDContext; diagnosticsManager?: internal.DiagnosticsManager; eventProcessor?: internal.EventProcessor; + identifyTimeout: number = 5; logger: LDLogger; streamer?: internal.StreamingProcessor; + readonly highTimeoutThreshold: number = 15; + private eventFactoryDefault = new EventFactory(false); private eventFactoryWithReasons = new EventFactory(true); private emitter: LDEmitter; @@ -100,7 +105,7 @@ export default class LDClientImpl implements LDClient { if (this.context) { // identify will start streamer - return this.identify(this.context); + return this.identify(this.context, { timeout: this.identifyTimeout }); } break; default: @@ -233,13 +238,13 @@ export default class LDClientImpl implements LDClient { */ protected createStreamUriPath(_context: LDContext): string { throw new Error( - 'createStreamUriPath not implemented. Client sdks must implement createStreamUriPath for streamer to work', + 'createStreamUriPath not implemented. Client sdks must implement createStreamUriPath for streamer to work.', ); } - private createIdentifyPromise() { + private createIdentifyPromise(timeout: number) { let res: any; - const p = new Promise((resolve, reject) => { + const slow = new Promise((resolve, reject) => { res = resolve; if (this.identifyChangeListener) { @@ -252,7 +257,7 @@ export default class LDClientImpl implements LDClient { this.identifyChangeListener = (c: LDContext, changedKeys: string[]) => { this.logger.debug(`change: context: ${JSON.stringify(c)}, flags: ${changedKeys}`); }; - this.identifyErrorListener = (c: LDContext, err: any) => { + this.identifyErrorListener = (c: LDContext, err: Error) => { this.logger.debug(`error: ${err}, context: ${JSON.stringify(c)}`); reject(err); }; @@ -261,7 +266,10 @@ export default class LDClientImpl implements LDClient { this.emitter.on('error', this.identifyErrorListener); }); - return { identifyPromise: p, identifyResolve: res }; + const timed = timedPromise(timeout, 'identify', this.logger); + const raced = Promise.race([timed, slow]); + + return { identifyPromise: raced, identifyResolve: res }; } private async getFlagsFromStorage(canonicalKey: string): Promise { @@ -269,7 +277,23 @@ export default class LDClientImpl implements LDClient { return f ? JSON.parse(f) : undefined; } - async identify(pristineContext: LDContext): Promise { + /** + * Identifies a context to LaunchDarkly. See {@link LDClient.identify}. + * + * @param pristineContext The LDContext object to be identified. + * @param identifyOptions Optional configuration. See {@link LDIdentifyOptions}. + * + * @returns {Promise}. + */ + async identify(pristineContext: LDContext, identifyOptions?: LDIdentifyOptions): Promise { + if (identifyOptions?.timeout) { + this.identifyTimeout = identifyOptions.timeout; + } + + if (this.identifyTimeout > this.highTimeoutThreshold) { + this.logger.warn('identify called with high timeout parameter.'); + } + let context = await ensureKey(pristineContext, this.platform); if (this.autoEnvAttributes === AutoEnvAttributes.Enabled) { @@ -285,7 +309,7 @@ export default class LDClientImpl implements LDClient { } this.eventProcessor?.sendEvent(this.eventFactoryDefault.identifyEvent(checkedContext)); - const { identifyPromise, identifyResolve } = this.createIdentifyPromise(); + const { identifyPromise, identifyResolve } = this.createIdentifyPromise(this.identifyTimeout); this.logger.debug(`Identifying ${JSON.stringify(context)}`); const flagsStorage = await this.getFlagsFromStorage(checkedContext.canonicalKey); diff --git a/packages/shared/sdk-client/src/api/LDClient.ts b/packages/shared/sdk-client/src/api/LDClient.ts index 6e31aab3b2..98b3bd924e 100644 --- a/packages/shared/sdk-client/src/api/LDClient.ts +++ b/packages/shared/sdk-client/src/api/LDClient.ts @@ -8,6 +8,7 @@ import { } from '@launchdarkly/js-sdk-common'; import ConnectionMode from './ConnectionMode'; +import { LDIdentifyOptions } from './LDIdentifyOptions'; /** * The basic interface for the LaunchDarkly client. Platform-specific SDKs may add some methods of their own. @@ -107,11 +108,13 @@ export interface LDClient { * await the Promise to determine when the new flag values are available. * * @param context - * The LDContext object. + * The LDContext object. + * @param identifyOptions + * Optional configuration. Please see {@link LDIdentifyOptions}. * @returns * A Promise which resolves when the flag values for the specified context are available. */ - identify(context: LDContext): Promise; + identify(context: LDContext, identifyOptions?: LDIdentifyOptions): Promise; /** * Determines the json variation of a feature flag. diff --git a/packages/shared/sdk-client/src/api/LDIdentifyOptions.ts b/packages/shared/sdk-client/src/api/LDIdentifyOptions.ts new file mode 100644 index 0000000000..355c0e93ec --- /dev/null +++ b/packages/shared/sdk-client/src/api/LDIdentifyOptions.ts @@ -0,0 +1,11 @@ +export interface LDIdentifyOptions { + /** + * In seconds. Determines when the identify promise resolves if no flags have been + * returned from the network. If you use a large timeout and await it, then + * any network delays will cause your application to wait a long time before + * continuing execution. + * + * Defaults to 5 seconds. + */ + timeout: number; +}