feat: wire fdv1-fallback capability into node-client, browser, and react-native contract-test entities - #1858
Conversation
…act-native contract-test entities
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk size report |
| # RN's forked EventSource (react-native-sse) fires `open` too late (from | ||
| # onreadystatechange at readyState DONE, not from onprogress at LOADING) and never | ||
| # populates headers on error events, so FDv1 fallback directives carried on stream | ||
| # response headers don't reach the SDK in time (or at all, on the error path). | ||
| streaming/fdv2/FDv1 fallback directive/directive on streaming error engages FDv1 fallback | ||
| streaming/fdv2/FDv1 fallback directive/directive on streaming success applies payload then engages FDv1 | ||
| streaming/fdv2/FDv1 fallback directive/directive without FDv1 fallback configured halts the data system | ||
| tags/FDv1 fallback directive requests |
There was a problem hiding this comment.
This is tracked in a separate ticket
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| function translateModeDefinition( | ||
| modeDef: SDKConfigModeDefinition, | ||
| fdv1Fallback?: SDKConfigPollingParams | null, | ||
| ): ModeDefinition { |
There was a problem hiding this comment.
🟡 Fallback polling settings are ignored when the test harness uses custom connection modes
The fallback polling settings sent by the test harness are only forwarded for the simple configuration path (translateModeDefinition(modeDef, options.dataSystem.fdv1Fallback) at packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts:167) and are silently dropped when the harness also asks for custom connection modes, so those test scenarios run against the wrong endpoint.
Impact: Contract-test scenarios that combine custom connection modes with a fallback endpoint will not exercise the requested fallback and can fail or pass for the wrong reason.
Only one of the two configuration branches forwards options.dataSystem.fdv1Fallback
SDKConfigDataSystem.fdv1Fallback (packages/tooling/contract-test-utils/src/types/ConfigParams.ts:30) is a sibling of connectionModeConfig, so the harness can send both. In all three entities the connectionModeConfig branch still calls translateModeDefinition(modeDef) with no second argument (packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts:150, packages/sdk/node-client/contract-tests/src/sdkClientEntity.ts:156, packages/sdk/react-native/contract-tests/entity/src/ClientEntity.ts:149), while only the top-level initializers/synchronizers branch passes options.dataSystem.fdv1Fallback. In that case the SDK falls back to the built-in default fallback endpoint/interval instead of the one the harness specified.
Prompt for agents
In the three client contract-test entities (packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts, packages/sdk/node-client/contract-tests/src/sdkClientEntity.ts, packages/sdk/react-native/contract-tests/entity/src/ClientEntity.ts) the new fdv1Fallback parameter of translateModeDefinition is only supplied from the branch that wraps top-level initializers/synchronizers into a single 'streaming' mode. The branch that handles options.dataSystem.connectionModeConfig.customConnectionModes still calls translateModeDefinition(modeDef) without the fallback, so a harness configuration that sets both connectionModeConfig and dataSystem.fdv1Fallback loses the requested fallback endpoint/poll interval. Consider forwarding options.dataSystem.fdv1Fallback in the custom connection modes loop as well (for every mode, or at least the modes that have synchronizers).
Was this helpful? React with 👍 or 👎 to provide feedback.
| streaming/requests/method and headers/REPORT/http | ||
| streaming/requests/URL path is computed correctly/no environment filter/base URI has no trailing slash/REPORT | ||
| streaming/requests/URL path is computed correctly/no environment filter/base URI has a trailing slash/REPORT |
There was a problem hiding this comment.
🟡 Browser contract tests will fail because fallback-directive scenarios are advertised but not skipped
The browser test entity now tells the test harness it supports fallback directives while only one of the four directive-based scenarios is skipped (tags/FDv1 fallback directive requests at packages/sdk/browser/contract-tests/suppressions_datamode_changes.txt:7), even though the browser can never observe those directives, so the remaining scenarios run and fail.
Impact: The browser contract-test job in CI is expected to fail on the fallback-directive scenarios.
Browser EventSource reports headers: false, and the FDv2 harness run uses this suppressions file
packages/sdk/browser/contract-tests/entity/src/TestHarnessWebSocket.ts:48 adds the fdv1-fallback capability, so the v3 harness (run with --skip-from=.../suppressions_datamode_changes.txt, see .github/workflows/browser.yml:113-119) will execute the streaming/fdv2/FDv1 fallback directive/... scenarios. The directive is carried on streaming response headers (packages/shared/sdk-client/src/datasource/fdv2/fallbackDirective.ts), but the browser platform uses the native EventSource and reports headers: false (packages/sdk/browser/src/platform/BrowserRequests.ts:22-28, packages/sdk/browser/src/platform/DefaultBrowserEventSource.ts:58). The react-native entity, which has the same limitation on the error path, suppresses those three streaming scenarios (packages/sdk/react-native/contract-tests/suppressions-fdv2.txt:9-11); the browser suppressions omit them.
(Refers to lines 5-8)
Prompt for agents
The browser contract-test entity now advertises the fdv1-fallback capability, but the browser uses the native EventSource which cannot read streaming response headers (BrowserRequests.getEventSourceCapabilities returns headers: false), so the harness scenarios that deliver the FDv1 fallback directive over the streaming response cannot pass. The react-native suppressions file lists the equivalent scenarios (streaming/fdv2/FDv1 fallback directive/...). Either add those streaming scenarios to packages/sdk/browser/contract-tests/suppressions_datamode_changes.txt or do not advertise the fdv1-fallback capability from the browser entity.
Was this helpful? React with 👍 or 👎 to provide feedback.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit f296bbf. Configure here.
| initialConnectionMode: 'streaming', | ||
| }; | ||
| dataSystem.connectionModes = { | ||
| streaming: translateModeDefinition(modeDef), |
There was a problem hiding this comment.
FDv1 fallback skipped for custom modes
Medium Severity
translateModeDefinition now accepts dataSystem.fdv1Fallback, but the connectionModeConfig.customConnectionModes path still calls it without that argument. Harness configs that combine custom modes with fdv1Fallback keep losing the dedicated FDv1 polling URI to applyEndpointOverrides, the same failure this PR fixes on the top-level initializers/synchronizers path.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit f296bbf. Configure here.
1 participant
Summary
Wires the harness's
dataSystem.fdv1Fallback.baseUriinto the node-client, browser, and react-native contract-test entities so the FDv1 fallback synchronizer connects to the harness's dedicated FDv1 polling endpoint instead of the polling initializer's endpoint (whichapplyEndpointOverridesotherwise overwrites it with).Completes contract-test coverage for the FDv1 fallback/recovery behavior built across this stack.
Note
Overview
Contract-test harness entities for browser, node-client, and react-native now map harness
dataSystem.fdv1Fallback(polling base URI and optional interval) into each SDK’s streamingconnectionModesvia an extendedtranslateModeDefinition, so the FDv1 fallback synchronizer uses the harness’s dedicated FDv1 polling endpoint instead of endpoints overwritten by initializer/synchronizerapplyEndpointOverrides.contract-test-utils adds
fdv1FallbackonSDKConfigDataSystem. Harness services advertise thefdv1-fallbackcapability (node-client enables it formally; browser/RN websocket handlers list it). React Native also advertisesclient-event-source-http-errors.Suppressions document platform limits: browser skips FDv1 directive request tag tests because native
EventSourcecannot read stream response headers; React Native skips several FDv1 directive streaming scenarios becausereact-native-ssedelivers headers/opentoo late for those tests.Reviewed by Cursor Bugbot for commit f296bbf. Bugbot is set up for automated code reviews on this repo. Configure here.