Skip to content

Commit fe8bd37

Browse files
authored
feat: add retry logic to FDv2 polling initializer (#1230)
## Summary - The FDv2 polling initializer now retries up to 3 times on recoverable errors (5xx, 400, 408, 429, network errors) with a 1-second delay between attempts, matching FDv1 behavior - Uses a `SHUTDOWN` symbol for clean race discrimination between poll results and shutdown signals - Removes the `oneShot` parameter from `poll()` / `processEvents()` since retry logic now lives at the initializer level <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Medium risk because it changes FDv2 initialization behavior by adding retry/delay loops and altering how polling errors are classified, which can affect startup latency and failure modes. > > **Overview** > Adds retry behavior to the FDv2 polling initializer: it now retries up to 3 times with a 1s delay on recoverable `interrupted` results, converts an exhausted retry sequence into `terminal_error`, and supports shutdown during both an in-flight poll and the retry sleep via a sentinel `SHUTDOWN` symbol. > > Simplifies `fdv2` polling by removing the `oneShot`/initializer-mode branching from `PollingBase`/`processEvents` and updating all call sites/tests accordingly (including `SourceFactoryProvider` ping handling and browser test mocks to return FDv2-style poll responses). > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 1dd4150. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 62d1d08 commit fe8bd37

8 files changed

Lines changed: 252 additions & 131 deletions

File tree

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

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ function mockResponse(value: string, statusCode: number) {
1515
// @ts-ignore
1616
headers: {
1717
// @ts-ignore
18-
get: jest.fn(),
18+
get: jest.fn(() => null),
1919
// @ts-ignore
2020
keys: jest.fn(),
2121
// @ts-ignore
@@ -61,6 +61,34 @@ export function makeRequests(): Requests {
6161
200,
6262
)();
6363
}
64+
if (url.includes('/sdk/poll/eval')) {
65+
return mockFetch(
66+
JSON.stringify({
67+
events: [
68+
{
69+
event: 'server-intent',
70+
data: {
71+
payloads: [{ id: 'mock', target: 1, intentCode: 'xfer-full', reason: 'mock' }],
72+
},
73+
},
74+
{
75+
event: 'put-object',
76+
data: {
77+
kind: 'flag-eval',
78+
key: 'flagA',
79+
version: 1,
80+
object: { value: true, trackEvents: false },
81+
},
82+
},
83+
{
84+
event: 'payload-transferred',
85+
data: { state: 'mock-state', version: 1, id: 'mock' },
86+
},
87+
],
88+
}),
89+
200,
90+
)();
91+
}
6492
return mockFetch('{ "flagA": true }', 200)();
6593
}),
6694
// @ts-ignore

packages/shared/sdk-client/__tests__/datasource/SourceFactoryProvider.test.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -339,7 +339,7 @@ it('ping handler uses the factory selector getter, not a stale reference', () =>
339339
pingHandler.handlePing();
340340

341341
// The ping poll should use the fresh selector, not 'selector-v1'
342-
expect(mockFdv2Poll).toHaveBeenCalledWith(expect.anything(), 'selector-v2', false, ctx.logger);
342+
expect(mockFdv2Poll).toHaveBeenCalledWith(expect.anything(), 'selector-v2', ctx.logger);
343343
});
344344

345345
it('ping handler uses per-entry endpoint-overridden requestor', () => {
@@ -361,5 +361,5 @@ it('ping handler uses per-entry endpoint-overridden requestor', () => {
361361

362362
// The ping poll should use the overridden requestor, not ctx.requestor
363363
const overriddenRequestor = mockMakeFDv2Requestor.mock.results[0].value;
364-
expect(mockFdv2Poll).toHaveBeenCalledWith(overriddenRequestor, undefined, false, ctx.logger);
364+
expect(mockFdv2Poll).toHaveBeenCalledWith(overriddenRequestor, undefined, ctx.logger);
365365
});

packages/shared/sdk-client/__tests__/datasource/fdv2/PollingBase.test.ts

Lines changed: 22 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ describe('given a successful FDv2 response', () => {
2525
body,
2626
});
2727

28-
const result = await poll(requestor, undefined, false, logger);
28+
const result = await poll(requestor, undefined, logger);
2929

3030
expect(result.type).toBe('changeSet');
3131
if (result.type === 'changeSet') {
@@ -48,7 +48,7 @@ describe('given a successful FDv2 response', () => {
4848
body,
4949
});
5050

51-
await poll(requestor, 'my-basis', false, logger);
51+
await poll(requestor, 'my-basis', logger);
5252

5353
expect(requestor.poll).toHaveBeenCalledWith('my-basis');
5454
});
@@ -62,7 +62,7 @@ describe('given a 304 Not Modified response', () => {
6262
body: null,
6363
});
6464

65-
const result = await poll(requestor, 'some-basis', false, logger);
65+
const result = await poll(requestor, 'some-basis', logger);
6666

6767
expect(result.type).toBe('changeSet');
6868
if (result.type === 'changeSet') {
@@ -76,7 +76,7 @@ describe('given a network error', () => {
7676
it('returns interrupted for synchronizer mode', async () => {
7777
const requestor = makeErrorRequestor(new Error('connection reset'));
7878

79-
const result = await poll(requestor, undefined, false, logger);
79+
const result = await poll(requestor, undefined, logger);
8080

8181
expect(result.type).toBe('status');
8282
if (result.type === 'status') {
@@ -85,18 +85,6 @@ describe('given a network error', () => {
8585
expect(result.errorInfo?.message).toBe('connection reset');
8686
}
8787
});
88-
89-
it('returns terminal error for initializer mode', async () => {
90-
const requestor = makeErrorRequestor(new Error('connection reset'));
91-
92-
const result = await poll(requestor, undefined, true, logger);
93-
94-
expect(result.type).toBe('status');
95-
if (result.type === 'status') {
96-
expect(result.state).toBe('terminal_error');
97-
expect(result.errorInfo?.kind).toBe(DataSourceErrorKind.NetworkError);
98-
}
99-
});
10088
});
10189

10290
describe('given an unrecoverable HTTP error', () => {
@@ -107,7 +95,7 @@ describe('given an unrecoverable HTTP error', () => {
10795
body: null,
10896
});
10997

110-
const result = await poll(requestor, undefined, false, logger);
98+
const result = await poll(requestor, undefined, logger);
11199

112100
expect(result.type).toBe('status');
113101
if (result.type === 'status') {
@@ -123,7 +111,7 @@ describe('given an unrecoverable HTTP error', () => {
123111
body: null,
124112
});
125113

126-
const result = await poll(requestor, undefined, false, logger);
114+
const result = await poll(requestor, undefined, logger);
127115

128116
if (result.type === 'status') {
129117
expect(result.state).toBe('terminal_error');
@@ -139,7 +127,7 @@ describe('given a recoverable HTTP error', () => {
139127
body: null,
140128
});
141129

142-
const result = await poll(requestor, undefined, false, logger);
130+
const result = await poll(requestor, undefined, logger);
143131

144132
expect(result.type).toBe('status');
145133
if (result.type === 'status') {
@@ -155,26 +143,12 @@ describe('given a recoverable HTTP error', () => {
155143
body: null,
156144
});
157145

158-
const result = await poll(requestor, undefined, false, logger);
146+
const result = await poll(requestor, undefined, logger);
159147

160148
if (result.type === 'status') {
161149
expect(result.state).toBe('interrupted');
162150
}
163151
});
164-
165-
it('returns terminal error for initializer mode on 500', async () => {
166-
const requestor = makeRequestor({
167-
status: 500,
168-
headers: makeHeaders(),
169-
body: null,
170-
});
171-
172-
const result = await poll(requestor, undefined, true, logger);
173-
174-
if (result.type === 'status') {
175-
expect(result.state).toBe('terminal_error');
176-
}
177-
});
178152
});
179153

180154
describe('given x-ld-fd-fallback header', () => {
@@ -186,7 +160,7 @@ describe('given x-ld-fd-fallback header', () => {
186160
body,
187161
});
188162

189-
const result = await poll(requestor, undefined, false, logger);
163+
const result = await poll(requestor, undefined, logger);
190164

191165
expect(result.fdv1Fallback).toBe(true);
192166
});
@@ -199,7 +173,7 @@ describe('given x-ld-fd-fallback header', () => {
199173
body,
200174
});
201175

202-
const result = await poll(requestor, undefined, false, logger);
176+
const result = await poll(requestor, undefined, logger);
203177

204178
expect(result.fdv1Fallback).toBe(false);
205179
});
@@ -211,7 +185,7 @@ describe('given x-ld-fd-fallback header', () => {
211185
body: null,
212186
});
213187

214-
const result = await poll(requestor, undefined, false, logger);
188+
const result = await poll(requestor, undefined, logger);
215189

216190
expect(result.fdv1Fallback).toBe(true);
217191
});
@@ -226,7 +200,7 @@ describe('given x-ld-envid header', () => {
226200
body,
227201
});
228202

229-
const result = await poll(requestor, undefined, false, logger);
203+
const result = await poll(requestor, undefined, logger);
230204

231205
if (result.type === 'changeSet') {
232206
expect(result.environmentId).toBe('env-abc-123');
@@ -240,7 +214,7 @@ describe('given x-ld-envid header', () => {
240214
body: null,
241215
});
242216

243-
const result = await poll(requestor, undefined, false, logger);
217+
const result = await poll(requestor, undefined, logger);
244218

245219
if (result.type === 'changeSet') {
246220
expect(result.environmentId).toBe('env-abc-123');
@@ -256,28 +230,13 @@ describe('given malformed JSON response', () => {
256230
body: '{invalid json',
257231
});
258232

259-
const result = await poll(requestor, undefined, false, logger);
233+
const result = await poll(requestor, undefined, logger);
260234

261235
if (result.type === 'status') {
262236
expect(result.state).toBe('interrupted');
263237
expect(result.errorInfo?.kind).toBe(DataSourceErrorKind.InvalidData);
264238
}
265239
});
266-
267-
it('returns terminal error for initializer mode', async () => {
268-
const requestor = makeRequestor({
269-
status: 200,
270-
headers: makeHeaders(),
271-
body: '{invalid json',
272-
});
273-
274-
const result = await poll(requestor, undefined, true, logger);
275-
276-
if (result.type === 'status') {
277-
expect(result.state).toBe('terminal_error');
278-
expect(result.errorInfo?.kind).toBe(DataSourceErrorKind.InvalidData);
279-
}
280-
});
281240
});
282241

283242
describe('given valid JSON without an events array', () => {
@@ -288,7 +247,7 @@ describe('given valid JSON without an events array', () => {
288247
body: '{"notEvents": true}',
289248
});
290249

291-
const result = await poll(requestor, undefined, false, logger);
250+
const result = await poll(requestor, undefined, logger);
292251

293252
expect(result.type).toBe('status');
294253
if (result.type === 'status') {
@@ -297,22 +256,6 @@ describe('given valid JSON without an events array', () => {
297256
expect(result.errorInfo?.message).toContain('missing or invalid events array');
298257
}
299258
});
300-
301-
it('returns terminal error for initializer mode', async () => {
302-
const requestor = makeRequestor({
303-
status: 200,
304-
headers: makeHeaders(),
305-
body: '{"events": "not-an-array"}',
306-
});
307-
308-
const result = await poll(requestor, undefined, true, logger);
309-
310-
expect(result.type).toBe('status');
311-
if (result.type === 'status') {
312-
expect(result.state).toBe('terminal_error');
313-
expect(result.errorInfo?.kind).toBe(DataSourceErrorKind.InvalidData);
314-
}
315-
});
316259
});
317260

318261
describe('given an empty response body', () => {
@@ -323,7 +266,7 @@ describe('given an empty response body', () => {
323266
body: null,
324267
});
325268

326-
const result = await poll(requestor, undefined, false, logger);
269+
const result = await poll(requestor, undefined, logger);
327270

328271
expect(result.type).toBe('status');
329272
if (result.type === 'status') {
@@ -352,7 +295,7 @@ describe('given a goodbye event in the response', () => {
352295
body,
353296
});
354297

355-
const result = await poll(requestor, undefined, false, logger);
298+
const result = await poll(requestor, undefined, logger);
356299

357300
expect(result.type).toBe('status');
358301
if (result.type === 'status') {
@@ -382,7 +325,7 @@ describe('given a server error event in the response', () => {
382325
body,
383326
});
384327

385-
const result = await poll(requestor, undefined, false, logger);
328+
const result = await poll(requestor, undefined, logger);
386329

387330
expect(result.type).toBe('status');
388331
if (result.type === 'status') {
@@ -408,7 +351,7 @@ describe('given a response with no payload-transferred event', () => {
408351
body,
409352
});
410353

411-
const result = await poll(requestor, undefined, false, logger);
354+
const result = await poll(requestor, undefined, logger);
412355

413356
expect(result.type).toBe('status');
414357
if (result.type === 'status') {
@@ -433,7 +376,7 @@ describe('given an intent with code none', () => {
433376
body,
434377
});
435378

436-
const result = await poll(requestor, undefined, false, logger);
379+
const result = await poll(requestor, undefined, logger);
437380

438381
expect(result.type).toBe('changeSet');
439382
if (result.type === 'changeSet') {
@@ -473,7 +416,7 @@ describe('given a partial (changes) transfer', () => {
473416
body,
474417
});
475418

476-
const result = await poll(requestor, 'old-state', false, logger);
419+
const result = await poll(requestor, 'old-state', logger);
477420

478421
expect(result.type).toBe('changeSet');
479422
if (result.type === 'changeSet') {
@@ -515,7 +458,7 @@ describe('given a delete-object event', () => {
515458
body,
516459
});
517460

518-
const result = await poll(requestor, 'old-state', false, logger);
461+
const result = await poll(requestor, 'old-state', logger);
519462

520463
expect(result.type).toBe('changeSet');
521464
if (result.type === 'changeSet') {

0 commit comments

Comments
 (0)