Skip to content

fix: honor FDv1 fallback directive during initializer phase#158

Open
keelerm84 wants to merge 7 commits intomainfrom
mk/sdk-2288/fdv1-fallback
Open

fix: honor FDv1 fallback directive during initializer phase#158
keelerm84 wants to merge 7 commits intomainfrom
mk/sdk-2288/fdv1-fallback

Conversation

@keelerm84
Copy link
Copy Markdown
Member

@keelerm84 keelerm84 commented Apr 30, 2026

Summary

  • FDv2DataSource now honors X-LD-FD-Fallback: true during the initializer phase: any accompanying payload is applied first, then the loop bails out and either swaps to the configured FDv1 fallback synchronizer or transitions the data source to OFF when no fallback is configured. Synchronizer-phase behavior is unchanged.
  • Contract test service declares the new fdv1-fallback capability and accepts a top-level dataSystem.fdv1Fallback config object, wiring it directly to dataSystemBuilder.fDv1FallbackSynchronizer(...) instead of inferring the fallback from the last synchronizer entry.
  • Bumps the contract-tests pin from v3.0.0-alpha.3 to v3.0.0-alpha.6 to pick up the new directive test suite.

Mirrors the Go reference change in go-server-sdk#365 and contract-test wiring in go-server-sdk#368. Spec rationale lives in sdk-specs#155; the new harness suite is in sdk-test-harness#336.

Test plan

  • Existing synchronizer-phase fallback tests stay green.
  • New initializer-phase tests cover: success-with-payload, error, no-FDv1-configured, success-no-selector. All deterministic — no real wall clocks or sleeps.
  • Full :lib:sdk:server:test suite green.
  • Contract-test workflow against v3.0.0-alpha.6 runs the new "FDv1 Fallback Directive" suite cleanly in CI.

Note

Medium Risk
Changes core data-source orchestration to short-circuit from FDv2 initializers into an FDv1 polling fallback (or OFF) based on a server directive, which could affect initialization/availability flows if mis-handled.

Overview
Honors the server-directed X-LD-FD-Fallback directive during the FDv2 initializer phase. FDv2DataSource now applies any initializer payload, then immediately switches to the configured FDv1 fallback synchronizer; if no fallback is configured it transitions the data source to OFF (preserving any initializer error info) and skips running FDv2 synchronizers.

Updates contract-test wiring for explicit FDv1 fallback config. The contract test service advertises a new fdv1-fallback capability, accepts dataSystem.fdv1Fallback as a dedicated config object (instead of inferring from the synchronizer list), and bumps the v3 harness pin to v3.0.0-alpha.6 to run the new directive suite. Adds/adjusts unit tests to cover initializer-phase fallback scenarios and to assert synchronizer-phase directive halts the data system when no fallback is configured.

Reviewed by Cursor Bugbot for commit 8cdf09d. Bugbot is set up for automated code reviews on this repo. Configure here.

When an FDv2 initializer returns a result with the FDv1 fallback signal,
apply any accompanying payload first, then halt the FDv2 chain and switch
terminally to the FDv1 fallback synchronizer. If no FDv1 fallback is
configured, transition the data source status to OFF with the underlying
error preserved instead of staying stuck at INITIALIZING.

The synchronizer-phase handling already honored the directive; this brings
the initializer phase in line with the spec so the directive is honored
throughout the data system lifecycle.
Declare the fdv1-fallback capability and accept the new top-level
dataSystem.fdv1Fallback config object (baseUri, pollIntervalMs) sent by
sdk-test-harness. Wire it directly to the SDK's FDv1 fallback synchronizer
instead of inferring it from the last entry of the FDv2 synchronizer list,
which misrepresented the SDK's architecture: the FDv1 Fallback Synchronizer
is distinct from the FDv2 Primary/Fallback chain.

Bumps the test harness pin to v3.0.0-alpha.6 to pull in the new directive
test suite that exercises this configuration.
… one configured

The synchronizer-phase fallback handler previously required a configured
FDv1 fallback synchronizer to honor the X-LD-FD-Fallback directive: when
no FDv1 fallback was configured, the directive was silently ignored and
the SDK kept reconnecting to the FDv2 synchronizer. Per Data System spec
1.6.3(4), the directive must terminally halt the data system in this case.

Now, when a synchronizer surfaces a result with FallbackToFDv1=true and no
FDv1 fallback is configured, the current synchronizer is blocked, the
status transitions to OFF with the underlying error info preserved, and
runSynchronizers() returns terminally so no further FDv2 synchronizers are
attempted. The caller in run() observes the deliberate halt and skips the
"unexpected exhaustion" log so the OFF status is not clobbered.
@keelerm84 keelerm84 marked this pull request as ready for review May 1, 2026 11:31
@keelerm84 keelerm84 requested a review from a team as a code owner May 1, 2026 11:31
Previously, Java marked the data source VALID and completed startFuture
on `anyDataReceived` -- any CHANGE_SET applied during the initializer
phase, regardless of whether the basis carried a defined selector. The
other FDv2-supporting SDKs (Go, Python, Ruby) only consider initialization
complete when a selectorful basis is applied; the synchronizer phase (or
FDv1 fallback) is responsible for the eventual VALID transition when the
initializer phase only produced selectorless data.

Drop the "treat data without a selector as enough" path in
runInitializers so Java matches the cross-SDK contract. Selectorless
bases are still applied to the store so evaluations can serve them
during the gap between init and the first selectorful basis from a
synchronizer.

Update two tests that asserted the old behavior:
- statusTransitionsToValidAfterInitialization now uses a selectorful
  basis (matching its name).
- initializerChangeSetWithoutSelectorCompletesIfLastInitializer is
  renamed and inverted to assert the new contract: a selectorless
  initializer with no synchronizer transitions to OFF, not VALID.
"Synchronizer '{}' requested FDv1 fallback, but no FDv1 fallback synchronizer is configured; halting the data system.",
synchronizer.name()
);
sourceManager.blockCurrentSynchronizer();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why does this only block the current synchronizer and not all non-fallback synchronizers? Singling out a specific synchronizer seems unexpected since fallback has us blocking all fdv2 synchronizers via sourceManager.fdv1Fallback();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think you can call sourceManager.fdv1Fallback(); unconditionally and that will block all non-fallback synchronizers. Then the if (sourceManager.hasFDv1Fallback()) check is just about logging and deciding to keep running or terminate with return.

// consider ourselves initialized.
if (anyDataReceived) {
dataSourceUpdates.updateStatus(DataSourceStatusProvider.State.VALID, null);
startFuture.complete(true);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this results in a change in behavior if there are no synchronizers to follow the initializers.

…applied data

Restore the previous behavior where a selectorless basis applied during
the initializer phase marks the data source VALID once the entire
initializer chain is exhausted. Without this, an SDK configured with
only selectorless initializers (and no synchronizer, or a synchronizer
that hasn't yet produced a selectorful payload) would never transition
out of INITIALIZING.

The selectorful early-return path is unchanged: a basis with a defined
selector continues to mark VALID immediately, before any further
initializers run. The directive-on-selectorless-basis path is also
unchanged: the FDv1 fallback continues to be triggered without a
premature VALID transition there.

Drops a unit test that asserted the cross-SDK "no VALID without
selector" gate that this commit reverses.
Per PR #158 review feedback, sourceManager.fdv1Fallback() blocks every
FDv2 synchronizer regardless of whether an FDv1 fallback is configured
(the unblock-FDv1 loop is a no-op when none exists). Calling it
unconditionally lets the subsequent hasFDv1Fallback() check focus on
logging and control flow only -- hand off to FDv1 vs. halt with OFF.

The halt branch's explicit blockCurrentSynchronizer() is now redundant
(fdv1Fallback() already blocked the current sync along with the rest of
the FDv2 chain) and was dropped. blockCurrentSynchronizer remains in
use by the TERMINAL_ERROR branch.

No behavior change.
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ 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 8cdf09d. Configure here.


// If we had synchronizers, and we ran out of them, and we aren't shutting down, then that was unexpected,
// and we will report it.
maybeReportUnexpectedExhaustion("All data source acquisition methods have been exhausted.");
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Initializer phase doesn't block FDv2 synchronizers unconditionally on fallback

Medium Severity

In the initializer-phase fallback handling, sourceManager.fdv1Fallback() is only called when hasFDv1Fallback() is true. In the synchronizer phase (line 460), fdv1Fallback() is called unconditionally before checking hasFDv1Fallback(), which blocks all FDv2 synchronizers regardless. The initializer phase omits blocking FDv2 synchronizers when no FDv1 fallback is configured, creating an inconsistency with the synchronizer phase pattern. The reviewer explicitly flagged this — fdv1Fallback() should be called unconditionally so all non-fallback synchronizers are blocked, with the hasFDv1Fallback() check only governing logging and the continue-vs-halt decision.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 8cdf09d. Configure here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants