Skip to content

Commit acf1ac7

Browse files
committed
chore: addressing PR comments
1 parent 817200b commit acf1ac7

File tree

4 files changed

+120
-44
lines changed

4 files changed

+120
-44
lines changed

packages/sdk/browser/__tests__/BrowserClient.test.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -382,12 +382,12 @@ describe('given a mock platform for a BrowserClient', () => {
382382
platform,
383383
);
384384

385-
const waitPromise = client.waitForInitialization(10);
385+
const waitPromise = client.waitForInitialization({ timeout: 10 });
386386
const identifyPromise = client.identify({ key: 'user-key', kind: 'user' });
387387

388388
await Promise.all([waitPromise, identifyPromise]);
389389

390-
await expect(waitPromise).resolves.toBeUndefined();
390+
await expect(waitPromise).resolves.toEqual({ status: 'complete' });
391391
await expect(identifyPromise).resolves.toEqual({ status: 'completed' });
392392
});
393393

@@ -417,10 +417,10 @@ describe('given a mock platform for a BrowserClient', () => {
417417
client.identify({ key: 'user-key', kind: 'user' });
418418

419419
// Call waitForInitialization with a short timeout (0.1 seconds)
420-
const waitPromise = client.waitForInitialization(0.1);
420+
const waitPromise = client.waitForInitialization({ timeout: 0.1 });
421421

422422
// Verify that waitForInitialization rejects with a timeout error
423-
await expect(waitPromise).rejects.toThrow();
423+
await expect(waitPromise).resolves.toEqual({ status: 'timeout' });
424424

425425
// Clean up: resolve the fetch to avoid hanging promises and restore fake timers
426426
resolveFetch!({});

packages/sdk/browser/src/BrowserClient.ts

Lines changed: 37 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import {
1616
LDHeaders,
1717
LDIdentifyResult,
1818
LDPluginEnvironmentMetadata,
19-
LDTimeoutError,
2019
Platform,
2120
} from '@launchdarkly/js-client-sdk-common';
2221

@@ -26,16 +25,23 @@ import { BrowserIdentifyOptions as LDIdentifyOptions } from './BrowserIdentifyOp
2625
import { registerStateDetection } from './BrowserStateDetector';
2726
import GoalManager from './goals/GoalManager';
2827
import { Goal, isClick } from './goals/Goals';
29-
import { LDClient } from './LDClient';
28+
import {
29+
LDClient,
30+
LDWaitForInitializationComplete,
31+
LDWaitForInitializationFailed,
32+
LDWaitForInitializationOptions,
33+
LDWaitForInitializationResult,
34+
LDWaitForInitializationTimeout,
35+
} from './LDClient';
3036
import { LDPlugin } from './LDPlugin';
3137
import validateBrowserOptions, { BrowserOptions, filterToBaseOptionsWithDefaults } from './options';
3238
import BrowserPlatform from './platform/BrowserPlatform';
3339

3440
class BrowserClientImpl extends LDClientImpl {
3541
private readonly _goalManager?: GoalManager;
3642
private readonly _plugins?: LDPlugin[];
37-
private _initializedPromise?: Promise<void>;
38-
private _initResolve?: () => void;
43+
private _initializedPromise?: Promise<LDWaitForInitializationResult>;
44+
private _initResolve?: (result: LDWaitForInitializationResult) => void;
3945
private _isInitialized: boolean = false;
4046

4147
constructor(
@@ -219,13 +225,17 @@ class BrowserClientImpl extends LDClientImpl {
219225
const res = await super.identifyResult(context, identifyOptionsWithUpdatedDefaults);
220226
if (res.status === 'completed') {
221227
this._isInitialized = true;
222-
this._initResolve?.();
228+
this._initResolve?.({ status: 'complete' });
223229
}
224230
this._goalManager?.startTracking();
225231
return res;
226232
}
227233

228-
waitForInitialization(timeout: number = 5): Promise<void> {
234+
waitForInitialization(
235+
options?: LDWaitForInitializationOptions,
236+
): Promise<LDWaitForInitializationResult> {
237+
const timeout = options?.timeout ?? 5;
238+
229239
// An initialization promise is only created if someone is going to use that promise.
230240
// If we always created an initialization promise, and there was no call waitForInitialization
231241
// by the time the promise was rejected, then that would result in an unhandled promise
@@ -238,7 +248,9 @@ class BrowserClientImpl extends LDClientImpl {
238248
}
239249

240250
if (this._isInitialized) {
241-
return Promise.resolve();
251+
return Promise.resolve({
252+
status: 'complete',
253+
});
242254
}
243255

244256
if (!this._initializedPromise) {
@@ -258,16 +270,25 @@ class BrowserClientImpl extends LDClientImpl {
258270
* @param logger A logger to log when the timeout expires.
259271
* @returns
260272
*/
261-
private _promiseWithTimeout(basePromise: Promise<void>, timeout: number): Promise<void> {
273+
private _promiseWithTimeout(
274+
basePromise: Promise<LDWaitForInitializationResult>,
275+
timeout: number,
276+
): Promise<LDWaitForInitializationResult> {
262277
const cancelableTimeout = cancelableTimedPromise(timeout, 'waitForInitialization');
263278
return Promise.race([
264-
basePromise.then(() => cancelableTimeout.cancel()),
265-
cancelableTimeout.promise,
279+
basePromise.then((res: LDWaitForInitializationResult) => {
280+
cancelableTimeout.cancel();
281+
return res;
282+
}),
283+
cancelableTimeout.promise
284+
// If the promise resolves without error, then the initialization completed successfully.
285+
// NOTE: this should never return as the resolution would only be triggered by the basePromise
286+
// being resolved.
287+
.then(() => ({ status: 'complete' }) as LDWaitForInitializationComplete)
288+
.catch(() => ({ status: 'timeout' }) as LDWaitForInitializationTimeout),
266289
]).catch((reason) => {
267-
if (reason instanceof LDTimeoutError) {
268-
this.logger?.error(reason.message);
269-
}
270-
throw reason;
290+
this.logger?.error(reason.message);
291+
return { status: 'failed', error: reason as Error } as LDWaitForInitializationFailed;
271292
});
272293
}
273294

@@ -337,7 +358,8 @@ export function makeClient(
337358
close: () => impl.close(),
338359
allFlags: () => impl.allFlags(),
339360
addHook: (hook: Hook) => impl.addHook(hook),
340-
waitForInitialization: (timeout: number) => impl.waitForInitialization(timeout),
361+
waitForInitialization: (waitOptions?: LDWaitForInitializationOptions) =>
362+
impl.waitForInitialization(waitOptions),
341363
logger: impl.logger,
342364
};
343365

packages/sdk/browser/src/LDClient.ts

Lines changed: 78 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,63 @@ import {
66

77
import { BrowserIdentifyOptions as LDIdentifyOptions } from './BrowserIdentifyOptions';
88

9+
/**
10+
* @ignore
11+
* Currently these options and the waitForInitialization method signiture will mirror the one
12+
* that is defined in the server common. We will be consolidating this mehod so that it will
13+
* be common to all sdks in the future.
14+
*/
15+
/**
16+
* Options for the waitForInitialization method.
17+
*/
18+
export interface LDWaitForInitializationOptions {
19+
/**
20+
* The timeout duration in seconds to wait for initialization before resolving the promise.
21+
* If exceeded, the promise will resolve to a {@link LDWaitForInitializationTimeout} object.
22+
*
23+
* If no options are specified on the `waitForInitialization`, the default timeout of 5 seconds will be used.
24+
*
25+
* Using a high timeout, or no timeout, is not recommended because it could result in a long
26+
* delay when conditions prevent successful initialization.
27+
*
28+
* A value of 0 will cause the promise to resolve without waiting. In that scenario it would be
29+
* more effective to not call `waitForInitialization`.
30+
*
31+
* @default 5 seconds
32+
*/
33+
timeout: number;
34+
}
35+
36+
/**
37+
* The waitForInitialization operation failed.
38+
*/
39+
export interface LDWaitForInitializationFailed {
40+
status: 'failed';
41+
error: Error;
42+
}
43+
44+
/**
45+
* The waitForInitialization operation timed out.
46+
*/
47+
export interface LDWaitForInitializationTimeout {
48+
status: 'timeout';
49+
}
50+
51+
/**
52+
* The waitForInitialization operation completed successfully.
53+
*/
54+
export interface LDWaitForInitializationComplete {
55+
status: 'complete';
56+
}
57+
58+
/**
59+
* The result of the waitForInitialization operation.
60+
*/
61+
export type LDWaitForInitializationResult =
62+
| LDWaitForInitializationFailed
63+
| LDWaitForInitializationTimeout
64+
| LDWaitForInitializationComplete;
65+
966
/**
1067
*
1168
* The LaunchDarkly SDK client object.
@@ -70,38 +127,35 @@ export type LDClient = Omit<
70127
/**
71128
* Returns a Promise that tracks the client's initialization state.
72129
*
73-
* The Promise will be resolved if the client successfully initializes, or rejected if client
74-
* initialization takes longer than the set timeout.
130+
* The Promise will be resolved to a {@link LDWaitForInitializationResult} object containing the
131+
* status of the waitForInitialization operation.
75132
*
133+
* @example
134+
* This example shows use of async/await syntax for specifying handlers:
76135
* ```
77-
* // using async/await
78-
* try {
79-
* await client.waitForInitialization(5);
80-
* doSomethingWithSuccessfullyInitializedClient();
81-
* } catch (err) {
82-
* doSomethingForFailedStartup(err);
136+
* const result = await client.waitForInitialization({ timeout: 5 });
137+
*
138+
* if (result.status === 'complete') {
139+
* doSomethingWithSuccessfullyInitializedClient();
140+
* } else if (result.status === 'failed') {
141+
* doSomethingForFailedStartup(result.error);
142+
* } else if (result.status === 'timeout') {
143+
* doSomethingForTimedOutStartup();
83144
* }
84145
* ```
85146
*
86-
* It is important that you handle the rejection case; otherwise it will become an unhandled Promise
87-
* rejection, which is a serious error on some platforms. The Promise is not created unless you
88-
* request it, so if you never call `waitForInitialization()` then you do not have to worry about
89-
* unhandled rejections.
90-
*
91-
* Note that you can also use event listeners ({@link on}) for the same purpose: the event `"initialized"`
147+
* @remarks
148+
* You can also use event listeners ({@link on}) for the same purpose: the event `"initialized"`
92149
* indicates success, and `"error"` indicates an error.
93150
*
94-
* @param timeout
95-
* The amount of time, in seconds, to wait for initialization before rejecting the promise.
96-
* Using a large timeout is not recommended. If you use a large timeout and await it, then
97-
* any network delays will cause your application to wait a long time before
98-
* continuing execution.
99-
*
100-
* @default 5 seconds
151+
* @param options
152+
* Optional configuration. Please see {@link LDWaitForInitializationOptions}.
101153
*
102154
* @returns
103-
* A Promise that will be resolved if the client initializes successfully, or rejected if it
104-
* fails or the specified timeout elapses.
155+
* A Promise that will be resolved to a {@link LDWaitForInitializationResult} object containing the
156+
* status of the waitForInitialization operation.
105157
*/
106-
waitForInitialization(timeout?: number): Promise<void>;
158+
waitForInitialization(
159+
options?: LDWaitForInitializationOptions,
160+
): Promise<LDWaitForInitializationResult>;
107161
};

packages/sdk/browser/src/compat/LDClientCompat.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import { LDClient as LDCLientBrowser } from '../LDClient';
1414
*/
1515
export interface LDClient extends Omit<
1616
LDCLientBrowser,
17-
'close' | 'flush' | 'identify' | 'identifyResult'
17+
'close' | 'flush' | 'identify' | 'identifyResult' | 'waitForInitialization'
1818
> {
1919
/**
2020
* Identifies a context to LaunchDarkly.

0 commit comments

Comments
 (0)