Skip to content

Commit cd91013

Browse files
authored
feat: add initial polling retries to BrowserDataManager (#1030)
**Requirements** - [x] I have added test coverage for new or changed functionality - [x] I have followed the repository's [pull request submission guidelines](../blob/main/CONTRIBUTING.md#submitting-pull-requests) - [x] I have validated my changes against all supported platform versions **Additional context** - I did not implement a configuration to set the retry interval (pending comments on whether or not we want to reuse the backoff logic) - I set the default retry limit to 3 - I made it so that if users set retry to less than 0 then we will log a warning and continue with limit set to default (3) <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Adds 3-attempt (1s interval) retry to initial polling in `BrowserDataManager`, with tests for success and max-retry failure. > > - **Browser SDK** > - **`packages/sdk/browser/src/BrowserDataManager.ts`**: > - Add `_requestPayload` helper implementing retry for initial poll (max 3 retries, 1s delay) using `shouldRetry`, `sleep`, and `httpErrorMessage`. > - Refactor `_finishIdentifyFromPoll` to use `_requestPayload`; maintain error reporting via `DataSourceStatusManager`. > - Import new utilities from `@launchdarkly/js-client-sdk-common`. > - **Tests (`packages/sdk/browser/__tests__/BrowserDataManager.test.ts`)**: > - Add tests verifying retries until success and rejection after max retries using fake timers and mocked `fetch`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 6affd78. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 0486ac8 commit cd91013

File tree

2 files changed

+121
-14
lines changed

2 files changed

+121
-14
lines changed

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

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -527,4 +527,71 @@ describe('given a BrowserDataManager with mocked dependencies', () => {
527527
'[BrowserDataManager] Identify called after data manager was closed.',
528528
);
529529
});
530+
531+
it('retries initial polling until it succeeds', async () => {
532+
jest.useFakeTimers();
533+
const context = Context.fromLDContext({ kind: 'user', key: 'test-user' });
534+
flagManager.loadCached.mockResolvedValue(false);
535+
536+
// Mock fetch to fail twice with 500 error, then succeed
537+
let callCount = 0;
538+
const mockedFetch = jest.fn().mockImplementation(() => {
539+
callCount += 1;
540+
if (callCount <= 2) {
541+
return mockResponse('', 500);
542+
}
543+
return mockResponse('{"flagA": true}', 200);
544+
});
545+
546+
platform.requests.fetch = mockedFetch as typeof platform.requests.fetch;
547+
548+
const identifyResolve = jest.fn();
549+
const identifyReject = jest.fn();
550+
551+
const identifyPromise = dataManager.identify(identifyResolve, identifyReject, context);
552+
553+
// Fast-forward through the retry delays (2 retries * 1000ms each)
554+
await jest.advanceTimersByTimeAsync(2000);
555+
556+
await identifyPromise;
557+
558+
expect(mockedFetch).toHaveBeenCalledTimes(3);
559+
expect(identifyResolve).toHaveBeenCalled();
560+
expect(identifyReject).not.toHaveBeenCalled();
561+
expect(flagManager.init).toHaveBeenCalledWith(
562+
expect.anything(),
563+
expect.objectContaining({ flagA: { flag: true, version: undefined } }),
564+
);
565+
566+
jest.useRealTimers();
567+
});
568+
569+
it('throws an error when initial polling reaches max retry limit', async () => {
570+
jest.useFakeTimers();
571+
const context = Context.fromLDContext({ kind: 'user', key: 'test-user' });
572+
flagManager.loadCached.mockResolvedValue(false);
573+
574+
// Mock fetch to always fail with 500 error
575+
const mockedFetch = jest.fn().mockImplementation(() => mockResponse('', 500));
576+
577+
platform.requests.fetch = mockedFetch as typeof platform.requests.fetch;
578+
579+
const identifyResolve = jest.fn();
580+
const identifyReject = jest.fn();
581+
582+
const identifyPromise = dataManager.identify(identifyResolve, identifyReject, context);
583+
584+
// Fast-forward through the retry delays (4 retries * 1000ms each)
585+
await jest.advanceTimersByTimeAsync(4000);
586+
587+
await identifyPromise;
588+
589+
// Should attempt initial request + 3 retries = 4 total attempts
590+
expect(mockedFetch).toHaveBeenCalledTimes(4);
591+
expect(identifyResolve).not.toHaveBeenCalled();
592+
expect(identifyReject).toHaveBeenCalled();
593+
expect(identifyReject).toHaveBeenCalledWith(expect.objectContaining({ status: 500 }));
594+
595+
jest.useRealTimers();
596+
});
530597
});

packages/sdk/browser/src/BrowserDataManager.ts

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,15 @@ import {
66
DataSourcePaths,
77
DataSourceState,
88
FlagManager,
9+
httpErrorMessage,
910
internal,
1011
LDEmitter,
1112
LDHeaders,
1213
LDIdentifyOptions,
1314
makeRequestor,
1415
Platform,
16+
shouldRetry,
17+
sleep,
1518
} from '@launchdarkly/js-client-sdk-common';
1619

1720
import { readFlagsFromBootstrap } from './bootstrap';
@@ -102,6 +105,56 @@ export default class BrowserDataManager extends BaseDataManager {
102105
this._updateStreamingState();
103106
}
104107

108+
/**
109+
* A helper function for the initial poll request. This is mainly here to facilitate
110+
* the retry logic.
111+
*
112+
* @param context - LDContext to request payload for.
113+
* @returns Payload as a string.
114+
*/
115+
private async _requestPayload(context: Context): Promise<string> {
116+
const plainContextString = JSON.stringify(Context.toLDContext(context));
117+
const pollingRequestor = makeRequestor(
118+
plainContextString,
119+
this.config.serviceEndpoints,
120+
this.getPollingPaths(),
121+
this.platform.requests,
122+
this.platform.encoding!,
123+
this.baseHeaders,
124+
[],
125+
this.config.withReasons,
126+
this.config.useReport,
127+
this._secureModeHash,
128+
);
129+
130+
// NOTE: We are currently hardcoding in 3 retries for the initial
131+
// poll. We can make this configurable in the future.
132+
const maxRetries = 3;
133+
134+
let lastError: any;
135+
136+
for (let attempt = 0; attempt <= maxRetries; attempt += 1) {
137+
try {
138+
// eslint-disable-next-line no-await-in-loop
139+
return await pollingRequestor.requestPayload();
140+
} catch (e: any) {
141+
if (!shouldRetry(e)) {
142+
throw e;
143+
}
144+
lastError = e;
145+
// NOTE: current we are hardcoding the retry interval to 1 second.
146+
// We can make this configurable in the future.
147+
if (attempt < maxRetries) {
148+
this._debugLog(httpErrorMessage(e, 'initial poll request', 'will retry'));
149+
// eslint-disable-next-line no-await-in-loop
150+
await sleep(1000);
151+
}
152+
}
153+
}
154+
155+
throw lastError;
156+
}
157+
105158
private async _finishIdentifyFromPoll(
106159
context: Context,
107160
identifyResolve: () => void,
@@ -110,21 +163,8 @@ export default class BrowserDataManager extends BaseDataManager {
110163
try {
111164
this.dataSourceStatusManager.requestStateUpdate(DataSourceState.Initializing);
112165

113-
const plainContextString = JSON.stringify(Context.toLDContext(context));
114-
const pollingRequestor = makeRequestor(
115-
plainContextString,
116-
this.config.serviceEndpoints,
117-
this.getPollingPaths(),
118-
this.platform.requests,
119-
this.platform.encoding!,
120-
this.baseHeaders,
121-
[],
122-
this.config.withReasons,
123-
this.config.useReport,
124-
this._secureModeHash,
125-
);
166+
const payload = await this._requestPayload(context);
126167

127-
const payload = await pollingRequestor.requestPayload();
128168
try {
129169
const listeners = this.createStreamListeners(context, identifyResolve);
130170
const putListener = listeners.get('put');

0 commit comments

Comments
 (0)