Skip to content

Commit 5d73dfe

Browse files
yusintoyusinto
authored
feat: Add identify timeout to client-sdk. (#420)
1 parent 46f7e0f commit 5d73dfe

File tree

11 files changed

+231
-12
lines changed

11 files changed

+231
-12
lines changed

‎packages/sdk/react-native/example/src/welcome.tsx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ export default function Welcome() {
1212

1313
const onIdentify = () => {
1414
ldc
15-
.identify({ kind: 'user', key: userKey })
15+
.identify({ kind: 'user', key: userKey }, { timeout: 5 })
1616
.catch((e: any) => console.error(`error identifying ${userKey}: ${e}`));
1717
};
1818

‎packages/sdk/react-native/src/ReactNativeLDClient.test.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ describe('ReactNativeLDClient', () => {
1010
});
1111

1212
test('constructing a new client', () => {
13+
expect(ldc.highTimeoutThreshold).toEqual(15);
1314
expect(ldc.sdkKey).toEqual('mobile-key');
1415
expect(ldc.config.serviceEndpoints).toEqual({
1516
analyticsEventPath: '/mobile',

‎packages/sdk/react-native/src/ReactNativeLDClient.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ export default class ReactNativeLDClient extends LDClientImpl {
4747
const internalOptions: internal.LDInternalOptions = {
4848
analyticsEventPath: `/mobile`,
4949
diagnosticEventPath: `/mobile/events/diagnostic`,
50+
highTimeoutThreshold: 15,
5051
};
5152

5253
super(

‎packages/shared/common/src/internal/events/LDInternalOptions.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,4 +12,13 @@ export type LDInternalOptions = {
1212
analyticsEventPath?: string;
1313
diagnosticEventPath?: string;
1414
includeAuthorizationHeader?: boolean;
15+
16+
/**
17+
* In seconds. Log a warning if identifyTimeout is greater than this value.
18+
*
19+
* Mobile - 15s.
20+
* Browser - 5s.
21+
* Server - 60s.
22+
*/
23+
highTimeoutThreshold?: number;
1524
};

‎packages/shared/common/src/utils/index.ts‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import fastDeepEqual from './fast-deep-equal';
66
import { base64UrlEncode, defaultHeaders, httpErrorMessage, LDHeaders, shouldRetry } from './http';
77
import noop from './noop';
88
import sleep from './sleep';
9+
import timedPromise from './timedPromise';
910
import { VoidFunction } from './VoidFunction';
1011

1112
export {
@@ -21,5 +22,6 @@ export {
2122
shouldRetry,
2223
secondsToMillis,
2324
sleep,
25+
timedPromise,
2426
VoidFunction,
2527
};

‎packages/shared/common/src/utils/timedPromise.ts‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
import { LDLogger } from '../api';
2+
3+
/**
4+
* Returns a promise which errors after t seconds.
5+
*
6+
* @param t Timeout in seconds.
7+
* @param taskName Name of task being timed for logging and error reporting.
8+
* @param logger {@link LDLogger} object.
9+
*/
10+
const timedPromise = (t: number, taskName: string, logger?: LDLogger) =>
11+
new Promise<void>((_res, reject) => {
12+
setTimeout(() => {
13+
const e = `${taskName} timed out after ${t} seconds.`;
14+
logger?.error(e);
15+
reject(new Error(e));
16+
}, t * 1000);
17+
});
18+
19+
export default timedPromise;

‎packages/shared/mocks/src/streamingProcessor.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export const setupMockStreamingProcessor = (
1313
putResponseJson: any = { data: { flags: {}, segments: {} } },
1414
patchResponseJson?: any,
1515
deleteResponseJson?: any,
16+
errorTimeoutSeconds: number = 0,
1617
) => {
1718
MockStreamingProcessor.mockImplementation(
1819
(
@@ -33,7 +34,7 @@ export const setupMockStreamingProcessor = (
3334
message: 'test-error',
3435
};
3536
errorHandler(unauthorized);
36-
});
37+
}, errorTimeoutSeconds * 1000);
3738
} else {
3839
// execute put which will resolve the identify promise
3940
setTimeout(() => listeners.get('put')?.processJson(putResponseJson));

‎packages/shared/sdk-client/src/LDClientImpl.timeout.test.ts‎

Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
import { AutoEnvAttributes, clone, LDContext } from '@launchdarkly/js-sdk-common';
2+
import { basicPlatform, logger, setupMockStreamingProcessor } from '@launchdarkly/private-js-mocks';
3+
4+
import * as mockResponseJson from './evaluation/mockResponse.json';
5+
import LDClientImpl from './LDClientImpl';
6+
import { Flags } from './types';
7+
import { toMulti } from './utils/addAutoEnv';
8+
9+
jest.mock('@launchdarkly/js-sdk-common', () => {
10+
const actual = jest.requireActual('@launchdarkly/js-sdk-common');
11+
const m = jest.requireActual('@launchdarkly/private-js-mocks');
12+
return {
13+
...actual,
14+
...{
15+
internal: {
16+
...actual.internal,
17+
StreamingProcessor: m.MockStreamingProcessor,
18+
},
19+
},
20+
};
21+
});
22+
23+
const testSdkKey = 'test-sdk-key';
24+
const carContext: LDContext = { kind: 'car', key: 'test-car' };
25+
26+
let ldc: LDClientImpl;
27+
let defaultPutResponse: Flags;
28+
29+
describe('sdk-client identify timeout', () => {
30+
beforeAll(() => {
31+
jest.useFakeTimers();
32+
});
33+
34+
beforeEach(() => {
35+
defaultPutResponse = clone<Flags>(mockResponseJson);
36+
37+
// simulate streamer error after a long timeout
38+
setupMockStreamingProcessor(true, defaultPutResponse, undefined, undefined, 30);
39+
40+
ldc = new LDClientImpl(testSdkKey, AutoEnvAttributes.Enabled, basicPlatform, {
41+
logger,
42+
sendEvents: false,
43+
});
44+
jest
45+
.spyOn(LDClientImpl.prototype as any, 'createStreamUriPath')
46+
.mockReturnValue('/stream/path');
47+
});
48+
49+
afterEach(() => {
50+
jest.resetAllMocks();
51+
});
52+
53+
// streamer is setup to error in beforeEach to cause a timeout
54+
test('rejects with default timeout of 5s', async () => {
55+
jest.advanceTimersByTimeAsync(ldc.identifyTimeout * 1000).then();
56+
await expect(ldc.identify(carContext)).rejects.toThrow(/identify timed out/);
57+
expect(logger.error).toHaveBeenCalledWith(expect.stringMatching(/identify timed out/));
58+
});
59+
60+
// streamer is setup to error in beforeEach to cause a timeout
61+
test('rejects with custom timeout', async () => {
62+
const timeout = 15;
63+
jest.advanceTimersByTimeAsync(timeout * 1000).then();
64+
65+
await expect(ldc.identify(carContext, { timeout })).rejects.toThrow(/identify timed out/);
66+
});
67+
68+
test('resolves with default timeout', async () => {
69+
setupMockStreamingProcessor(false, defaultPutResponse);
70+
jest.advanceTimersByTimeAsync(ldc.identifyTimeout * 1000).then();
71+
72+
await expect(ldc.identify(carContext)).resolves.toBeUndefined();
73+
74+
expect(ldc.getContext()).toEqual(expect.objectContaining(toMulti(carContext)));
75+
expect(ldc.allFlags()).toEqual({
76+
'dev-test-flag': true,
77+
'easter-i-tunes-special': false,
78+
'easter-specials': 'no specials',
79+
fdsafdsafdsafdsa: true,
80+
'log-level': 'warn',
81+
'moonshot-demo': true,
82+
test1: 's1',
83+
'this-is-a-test': true,
84+
});
85+
});
86+
87+
test('resolves with custom timeout', async () => {
88+
const timeout = 15;
89+
setupMockStreamingProcessor(false, defaultPutResponse);
90+
jest.advanceTimersByTimeAsync(timeout).then();
91+
92+
await expect(ldc.identify(carContext, { timeout })).resolves.toBeUndefined();
93+
94+
expect(ldc.getContext()).toEqual(expect.objectContaining(toMulti(carContext)));
95+
expect(ldc.allFlags()).toEqual({
96+
'dev-test-flag': true,
97+
'easter-i-tunes-special': false,
98+
'easter-specials': 'no specials',
99+
fdsafdsafdsafdsa: true,
100+
'log-level': 'warn',
101+
'moonshot-demo': true,
102+
test1: 's1',
103+
'this-is-a-test': true,
104+
});
105+
});
106+
107+
test('setting high timeout threshold with internalOptions', async () => {
108+
const highTimeoutThreshold = 20;
109+
setupMockStreamingProcessor(false, defaultPutResponse);
110+
ldc = new LDClientImpl(
111+
testSdkKey,
112+
AutoEnvAttributes.Enabled,
113+
basicPlatform,
114+
{
115+
logger,
116+
sendEvents: false,
117+
},
118+
{ highTimeoutThreshold },
119+
);
120+
const customTimeout = 10;
121+
jest.advanceTimersByTimeAsync(customTimeout * 1000).then();
122+
await ldc.identify(carContext, { timeout: customTimeout });
123+
124+
expect(ldc.identifyTimeout).toEqual(10);
125+
expect(logger.warn).not.toHaveBeenCalledWith(expect.stringMatching(/high timeout/));
126+
});
127+
128+
test('warning when timeout is too high', async () => {
129+
const highTimeout = 60;
130+
setupMockStreamingProcessor(false, defaultPutResponse);
131+
jest.advanceTimersByTimeAsync(highTimeout * 1000).then();
132+
133+
await ldc.identify(carContext, { timeout: highTimeout });
134+
135+
expect(logger.warn).toHaveBeenCalledWith(expect.stringMatching(/high timeout/));
136+
});
137+
138+
test('safe timeout should not warn', async () => {
139+
const { identifyTimeout } = ldc;
140+
const safeTimeout = identifyTimeout;
141+
setupMockStreamingProcessor(false, defaultPutResponse);
142+
jest.advanceTimersByTimeAsync(identifyTimeout * 1000).then();
143+
144+
await ldc.identify(carContext, { timeout: safeTimeout });
145+
146+
expect(logger.warn).not.toHaveBeenCalledWith(expect.stringMatching(/high timeout/));
147+
});
148+
});

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

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@ import {
1414
Platform,
1515
ProcessStreamResponse,
1616
EventName as StreamEventName,
17+
timedPromise,
1718
TypeValidators,
1819
} from '@launchdarkly/js-sdk-common';
1920

2021
import { ConnectionMode, LDClient, type LDOptions } from './api';
2122
import LDEmitter, { EventName } from './api/LDEmitter';
23+
import { LDIdentifyOptions } from './api/LDIdentifyOptions';
2224
import Configuration from './configuration';
2325
import createDiagnosticsManager from './diagnostics/createDiagnosticsManager';
2426
import createEventProcessor from './events/createEventProcessor';
@@ -34,9 +36,12 @@ export default class LDClientImpl implements LDClient {
3436
context?: LDContext;
3537
diagnosticsManager?: internal.DiagnosticsManager;
3638
eventProcessor?: internal.EventProcessor;
39+
identifyTimeout: number = 5;
3740
logger: LDLogger;
3841
streamer?: internal.StreamingProcessor;
3942

43+
readonly highTimeoutThreshold: number = 15;
44+
4045
private eventFactoryDefault = new EventFactory(false);
4146
private eventFactoryWithReasons = new EventFactory(true);
4247
private emitter: LDEmitter;
@@ -100,7 +105,7 @@ export default class LDClientImpl implements LDClient {
100105

101106
if (this.context) {
102107
// identify will start streamer
103-
return this.identify(this.context);
108+
return this.identify(this.context, { timeout: this.identifyTimeout });
104109
}
105110
break;
106111
default:
@@ -233,13 +238,13 @@ export default class LDClientImpl implements LDClient {
233238
*/
234239
protected createStreamUriPath(_context: LDContext): string {
235240
throw new Error(
236-
'createStreamUriPath not implemented. Client sdks must implement createStreamUriPath for streamer to work',
241+
'createStreamUriPath not implemented. Client sdks must implement createStreamUriPath for streamer to work.',
237242
);
238243
}
239244

240-
private createIdentifyPromise() {
245+
private createIdentifyPromise(timeout: number) {
241246
let res: any;
242-
const p = new Promise<void>((resolve, reject) => {
247+
const slow = new Promise<void>((resolve, reject) => {
243248
res = resolve;
244249

245250
if (this.identifyChangeListener) {
@@ -252,7 +257,7 @@ export default class LDClientImpl implements LDClient {
252257
this.identifyChangeListener = (c: LDContext, changedKeys: string[]) => {
253258
this.logger.debug(`change: context: ${JSON.stringify(c)}, flags: ${changedKeys}`);
254259
};
255-
this.identifyErrorListener = (c: LDContext, err: any) => {
260+
this.identifyErrorListener = (c: LDContext, err: Error) => {
256261
this.logger.debug(`error: ${err}, context: ${JSON.stringify(c)}`);
257262
reject(err);
258263
};
@@ -261,15 +266,34 @@ export default class LDClientImpl implements LDClient {
261266
this.emitter.on('error', this.identifyErrorListener);
262267
});
263268

264-
return { identifyPromise: p, identifyResolve: res };
269+
const timed = timedPromise(timeout, 'identify', this.logger);
270+
const raced = Promise.race([timed, slow]);
271+
272+
return { identifyPromise: raced, identifyResolve: res };
265273
}
266274

267275
private async getFlagsFromStorage(canonicalKey: string): Promise<Flags | undefined> {
268276
const f = await this.platform.storage?.get(canonicalKey);
269277
return f ? JSON.parse(f) : undefined;
270278
}
271279

272-
async identify(pristineContext: LDContext): Promise<void> {
280+
/**
281+
* Identifies a context to LaunchDarkly. See {@link LDClient.identify}.
282+
*
283+
* @param pristineContext The LDContext object to be identified.
284+
* @param identifyOptions Optional configuration. See {@link LDIdentifyOptions}.
285+
*
286+
* @returns {Promise<void>}.
287+
*/
288+
async identify(pristineContext: LDContext, identifyOptions?: LDIdentifyOptions): Promise<void> {
289+
if (identifyOptions?.timeout) {
290+
this.identifyTimeout = identifyOptions.timeout;
291+
}
292+
293+
if (this.identifyTimeout > this.highTimeoutThreshold) {
294+
this.logger.warn('identify called with high timeout parameter.');
295+
}
296+
273297
let context = await ensureKey(pristineContext, this.platform);
274298

275299
if (this.autoEnvAttributes === AutoEnvAttributes.Enabled) {
@@ -285,7 +309,7 @@ export default class LDClientImpl implements LDClient {
285309
}
286310

287311
this.eventProcessor?.sendEvent(this.eventFactoryDefault.identifyEvent(checkedContext));
288-
const { identifyPromise, identifyResolve } = this.createIdentifyPromise();
312+
const { identifyPromise, identifyResolve } = this.createIdentifyPromise(this.identifyTimeout);
289313
this.logger.debug(`Identifying ${JSON.stringify(context)}`);
290314

291315
const flagsStorage = await this.getFlagsFromStorage(checkedContext.canonicalKey);

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import {
88
} from '@launchdarkly/js-sdk-common';
99

1010
import ConnectionMode from './ConnectionMode';
11+
import { LDIdentifyOptions } from './LDIdentifyOptions';
1112

1213
/**
1314
* The basic interface for the LaunchDarkly client. Platform-specific SDKs may add some methods of their own.
@@ -107,11 +108,13 @@ export interface LDClient {
107108
* await the Promise to determine when the new flag values are available.
108109
*
109110
* @param context
110-
* The LDContext object.
111+
* The LDContext object.
112+
* @param identifyOptions
113+
* Optional configuration. Please see {@link LDIdentifyOptions}.
111114
* @returns
112115
* A Promise which resolves when the flag values for the specified context are available.
113116
*/
114-
identify(context: LDContext): Promise<void>;
117+
identify(context: LDContext, identifyOptions?: LDIdentifyOptions): Promise<void>;
115118

116119
/**
117120
* Determines the json variation of a feature flag.

0 commit comments

Comments
 (0)