Skip to content

fix(node-server-sdk): No FDv1 fallback when using custom datasystem #1088

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
joker23 merged 4 commits into from
Feb 3, 2026
Merged

fix(node-server-sdk): No FDv1 fallback when using custom datasystem #1088

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e2a9d38
fix(node-server-sdk): No FDv1 fallback when using custom datasystem
joker23 Feb 2, 2026
35e43ca
Merge branch 'main' into skz/node-server-contract-test
joker23 Feb 2, 2026
6864953
chore: remove scoffolding for contract test
joker23 Feb 3, 2026
e7a2241
Merge branch 'main' into skz/node-server-contract-test
joker23 Feb 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions .github/workflows/server-node.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,16 +38,12 @@ jobs:
- name: Launch the test service in the background
run: yarn contract-test-service 2>&1 &
- name: Clone and run contract tests from feat/fdv2 branch
# NOTE: This is a temporary workaround to run the contract tests from the feat/fdv2 branch.
# The commit hash is a few commits before the HEAD of the feat/fdv2 branch. We do this because
# the HEAD of the feat/fdv2 branch requires the synchronizers be passed as a list which is not
# implemented yet. (SDK-1798)
run: |
mkdir -p /tmp/sdk-test-harness
git clone https://github.com/launchdarkly/sdk-test-harness.git /tmp/sdk-test-harness
cp ./contract-tests/testharness-suppressions-fdv2.txt /tmp/sdk-test-harness/testharness-suppressions-fdv2.txt
cd /tmp/sdk-test-harness
git checkout de833af990da23a89b66c5366809b5be8c27e3f8
git checkout feat/fdv2
Copy link
Member

@kinyoklion kinyoklion Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we could use the downloader script to download the v3 version instead. But I am fine with this as well.

go build -o test-harness .
./test-harness -url http://localhost:8000 -debug --skip-from=testharness-suppressions-fdv2.txt
env:
Expand Down
104 changes: 51 additions & 53 deletions contract-tests/src/sdkClientEntity.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ import ld, {
LDOptions,
LDSerialExecution,
LDUser,
PollingDataSourceConfiguration,
StreamingDataSourceConfiguration,
} from '@launchdarkly/node-server-sdk';

import BigSegmentTestStore from './BigSegmentTestStore.js';
Expand All @@ -36,7 +38,7 @@ interface SdkConfigOptions {
};
dataSystem?: {
initializers?: SDKDataSystemInitializerParams[];
synchronizers?: SDKDataSystemSynchronizerParams;
synchronizers?: SDKDataSystemSynchronizerParams[];
payloadFilter?: string;
};
events?: {
Expand Down Expand Up @@ -74,14 +76,8 @@ interface SdkConfigOptions {
}

export interface SDKDataSystemSynchronizerParams {
primary?: {
streaming?: SDKDataSourceStreamingParams;
polling?: SDKDataSourcePollingParams;
};
secondary?: {
streaming?: SDKDataSourceStreamingParams;
polling?: SDKDataSourcePollingParams;
};
streaming?: SDKDataSourceStreamingParams;
polling?: SDKDataSourcePollingParams;
}

export interface SDKDataSystemInitializerParams {
Expand Down Expand Up @@ -222,52 +218,54 @@ export function makeSdkConfig(options: SdkConfigOptions, tag: string): LDOptions
}

if (options.dataSystem) {
const dataSourceStreamingOptions: SDKDataSourceStreamingParams | undefined =
options.dataSystem.synchronizers?.primary?.streaming ??
options.dataSystem.synchronizers?.secondary?.streaming;
const dataSourcePollingOptions: SDKDataSourcePollingParams | undefined =
options.dataSystem.initializers?.[0]?.polling ??
options.dataSystem.synchronizers?.primary?.polling ??
options.dataSystem.synchronizers?.secondary?.polling;

if (dataSourceStreamingOptions) {
cf.streamUri = dataSourceStreamingOptions.baseUri;
cf.streamInitialReconnectDelay = maybeTime(dataSourceStreamingOptions.initialRetryDelayMs);
}
if (dataSourcePollingOptions) {
cf.stream = false;
cf.baseUri = dataSourcePollingOptions.baseUri;
cf.pollInterval = maybeTime(dataSourcePollingOptions.pollIntervalMs);
const dataSourceOptions: DataSourceOptions = {
Copy link
Contributor Author

@joker23 joker23 Feb 3, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

converting the datasystem to only use custom which allows us to ingest a more free form array of initializers/synchronizers

Copy link
Member

@kinyoklion kinyoklion Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we not have a fallback synchronizer to configure somewhere?

Copy link
Member

@kinyoklion kinyoklion Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, later.

dataSourceOptionsType: 'custom',
initializers: [],
synchronizers: [],
};

if (options.dataSystem.initializers) {
options.dataSystem.initializers.forEach((initializer) => {
if (initializer.polling) {
cf.baseUri = initializer.polling.baseUri;
cf.pollInterval = maybeTime(initializer.polling.pollIntervalMs);

const initializerOptions: PollingDataSourceConfiguration = {
type: 'polling',
pollInterval: maybeTime(initializer.polling.pollIntervalMs),
};

dataSourceOptions.initializers.push(initializerOptions);
}
});
}

let dataSourceOptions: DataSourceOptions | undefined;
if (dataSourceStreamingOptions && dataSourcePollingOptions) {
dataSourceOptions = {
dataSourceOptionsType: 'standard',
...(dataSourceStreamingOptions.initialRetryDelayMs != null && {
streamInitialReconnectDelay: maybeTime(dataSourceStreamingOptions.initialRetryDelayMs),
}),
...(dataSourcePollingOptions.pollIntervalMs != null && {
pollInterval: dataSourcePollingOptions.pollIntervalMs,
}),
};
} else if (dataSourceStreamingOptions) {
dataSourceOptions = {
dataSourceOptionsType: 'streamingOnly',
...(dataSourceStreamingOptions.initialRetryDelayMs != null && {
streamInitialReconnectDelay: maybeTime(dataSourceStreamingOptions.initialRetryDelayMs),
}),
};
} else if (dataSourcePollingOptions) {
dataSourceOptions = {
dataSourceOptionsType: 'pollingOnly',
...(dataSourcePollingOptions.pollIntervalMs != null && {
pollInterval: dataSourcePollingOptions.pollIntervalMs,
}),
};
} else {
// No data source options were specified
dataSourceOptions = undefined;
if (options.dataSystem.synchronizers) {
options.dataSystem.synchronizers.forEach((synchronizer) => {
if (synchronizer.streaming) {
cf.streamUri = synchronizer.streaming.baseUri;

const synchronizerOptions: StreamingDataSourceConfiguration = {
type: 'streaming',
};

synchronizerOptions.streamInitialReconnectDelay = maybeTime(
synchronizer.streaming.initialRetryDelayMs,
);

dataSourceOptions.synchronizers.push(synchronizerOptions);
} else if (synchronizer.polling) {
cf.baseUri = synchronizer.polling.baseUri;
cf.pollInterval = maybeTime(synchronizer.polling.pollIntervalMs);

const synchronizerOptions: PollingDataSourceConfiguration = {
type: 'polling',
pollInterval: maybeTime(synchronizer.polling.pollIntervalMs),
};

dataSourceOptions.synchronizers.push(synchronizerOptions);
}
});
}

if (options.dataSystem.payloadFilter) {
Expand Down
1 change: 0 additions & 1 deletion contract-tests/testharness-suppressions-fdv2.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ streaming/requests/URL path is computed correctly/environment_filter_key="encodi
polling/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has no trailing slash/GET
polling/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has a trailing slash/GET

streaming/fdv2/reconnection state management/initializes from polling initializer
streaming/fdv2/reconnection state management/initializes from 2 polling initializers
Copy link
Member

@kinyoklion kinyoklion Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove any of these other suppressions with the updated config?

streaming/fdv2/reconnection state management/saves previously known state
streaming/fdv2/reconnection state management/replaces previously known state
Expand Down
22 changes: 11 additions & 11 deletions packages/shared/sdk-server/src/LDClientImpl.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -437,19 +437,19 @@ function constructFDv2(
),
);
}

// This is short term handling and will be removed once FDv2 adoption is sufficient.
fdv1FallbackSynchronizers.push(
() =>
new PollingProcessorFDv2(
new Requestor(config, platform.requests, baseHeaders, '/sdk/latest-all', config.logger),
pollingInterval,
config.logger,
true,
),
);
}

// This is short term handling and will be removed once FDv2 adoption is sufficient.
Copy link
Contributor Author

@joker23 joker23 Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is moving it out of the top conditional:

if (isCustomOptions(dataSystem.dataSource)) {...} else {...}

Copy link
Member

@kinyoklion kinyoklion Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, do we not allow you to configure it? I guess we may want to see if we standardize. We allow configuration of the fallback in java/dotnet. In case it needs different endpoints, or need to be streaming or something.

Copy link
Contributor Author

@joker23 joker23 Feb 3, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created a task to track this. In general we don't have a way to customize uris for different synchronizer/initializer/fallback

fdv1FallbackSynchronizers.push(
() =>
new PollingProcessorFDv2(
new Requestor(config, platform.requests, baseHeaders, '/sdk/latest-all', config.logger),
config.pollInterval ?? DEFAULT_POLL_INTERVAL,
config.logger,
true,
),
);

dataSource = new CompositeDataSource(
initializers,
synchronizers,
Expand Down