Skip to content

Commit bbdd6c6

Browse files
authored
fix: Fix the calculation of the basis parameter for FDv2 streaming. (Does not affect FDv1). (#1165)
The streaming synchronizer was pre-calculating the URL so it didn't include the selector from the initial poll. <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Touches FDv2 streaming connection setup by changing how the EventSource URL is built and encoded; a mistake here could break streaming connections or produce an incorrectly encoded `basis` parameter. > > **Overview** > Fixes FDv2 streaming URL construction so the `basis` query parameter is computed at `start()` time via an optional `selectorGetter`, ensuring the stream request can include the selector derived from the initial poll. > > Adds test coverage for including/excluding `basis` based on `selectorGetter` presence/return value, and updates streaming test helpers to pass the new option through to `createStreamingBase`. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit 063d6bc. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
1 parent 929a385 commit bbdd6c6

3 files changed

Lines changed: 65 additions & 6 deletions

File tree

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,6 +430,58 @@ it('produces results in order', async () => {
430430
base.close();
431431
});
432432

433+
it('includes basis parameter in stream URI when selectorGetter returns a value', () => {
434+
const mockEventSource = createMockEventSource();
435+
const mockRequests = createMockRequests(mockEventSource);
436+
437+
const base = createBase(mockRequests, logger, {
438+
streamUriPath: '/sdk/stream/eval/ctx',
439+
parameters: [{ key: 'auth', value: 'my-key' }],
440+
selectorGetter: () => '(p:test:1)',
441+
});
442+
base.start();
443+
444+
const uri = mockRequests.createEventSource.mock.calls[0][0];
445+
expect(uri).toContain(`basis=${encodeURIComponent('(p:test:1)')}`);
446+
expect(uri).toContain('auth=my-key');
447+
448+
base.close();
449+
});
450+
451+
it('does not include basis parameter when selectorGetter returns undefined', () => {
452+
const mockEventSource = createMockEventSource();
453+
const mockRequests = createMockRequests(mockEventSource);
454+
455+
const base = createBase(mockRequests, logger, {
456+
streamUriPath: '/sdk/stream/eval/ctx',
457+
parameters: [{ key: 'auth', value: 'my-key' }],
458+
selectorGetter: () => undefined,
459+
});
460+
base.start();
461+
462+
const uri = mockRequests.createEventSource.mock.calls[0][0];
463+
expect(uri).not.toContain('basis');
464+
expect(uri).toContain('auth=my-key');
465+
466+
base.close();
467+
});
468+
469+
it('does not include basis parameter when no selectorGetter is provided', () => {
470+
const mockEventSource = createMockEventSource();
471+
const mockRequests = createMockRequests(mockEventSource);
472+
473+
const base = createBase(mockRequests, logger, {
474+
streamUriPath: '/sdk/stream/eval/ctx',
475+
parameters: [{ key: 'auth', value: 'my-key' }],
476+
});
477+
base.start();
478+
479+
const uri = mockRequests.createEventSource.mock.calls[0][0];
480+
expect(uri).not.toContain('basis');
481+
482+
base.close();
483+
});
484+
433485
it('close is idempotent', () => {
434486
const mockEventSource = createMockEventSource();
435487
const mockRequests = createMockRequests(mockEventSource);

packages/shared/sdk-client/__tests__/datasource/fdv2/streamingTestHelpers.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,13 +114,15 @@ export function createBase(
114114
pingHandler?: PingHandler;
115115
streamUriPath?: string;
116116
parameters?: { key: string; value: string }[];
117+
selectorGetter?: () => string | undefined;
117118
} = {},
118119
) {
119120
return createStreamingBase({
120121
requests: mockRequests as any,
121122
serviceEndpoints,
122123
streamUriPath: options.streamUriPath ?? '/sdk/stream/eval/test-context',
123124
parameters: options.parameters ?? [],
125+
selectorGetter: options.selectorGetter,
124126
headers: baseHeaders,
125127
initialRetryDelayMillis: 1000,
126128
logger,

packages/shared/sdk-client/src/datasource/fdv2/StreamingFDv2Base.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ export function createStreamingBase(config: {
8282
serviceEndpoints: ServiceEndpoints;
8383
streamUriPath: string;
8484
parameters: { key: string; value: string }[];
85+
selectorGetter?: () => string | undefined;
8586
headers: LDHeaders;
8687
initialRetryDelayMillis: number;
8788
logger?: LDLogger;
@@ -94,13 +95,17 @@ export function createStreamingBase(config: {
9495
config.logger,
9596
);
9697

97-
const streamUri = getStreamingUri(
98-
config.serviceEndpoints,
99-
config.streamUriPath,
100-
config.parameters,
101-
);
10298
const headers: { [key: string]: string | string[] } = { ...config.headers };
10399

100+
function buildStreamUri(): string {
101+
const params = [...config.parameters];
102+
const basis = config.selectorGetter?.();
103+
if (basis) {
104+
params.push({ key: 'basis', value: encodeURIComponent(basis) });
105+
}
106+
return getStreamingUri(config.serviceEndpoints, config.streamUriPath, params);
107+
}
108+
104109
let eventSource: EventSource | undefined;
105110
let connectionAttemptStartTime: number | undefined;
106111
let fdv1Fallback = false;
@@ -256,7 +261,7 @@ export function createStreamingBase(config: {
256261

257262
logConnectionAttempt();
258263

259-
const es = config.requests.createEventSource(streamUri, {
264+
const es = config.requests.createEventSource(buildStreamUri(), {
260265
headers,
261266
errorFilter: (error: HttpErrorResponse) => handleError(error),
262267
initialRetryDelayMillis: config.initialRetryDelayMillis,

0 commit comments

Comments
 (0)