Skip to content

fix: Keep data from a store that is not initialized out of override evaluations and AllFlagsState, and report a store failure - #491

Merged
kinyoklion merged 5 commits into
v7from
rlamb/overrides-go-allflags-overrides-only
Nov 4, 2026
Merged

kinyoklion merged 5 commits into
v7from
rlamb/overrides-go-allflags-overrides-only

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Oct 28, 2026 •

Copy link
Copy Markdown
Member

Summary

Overrides only add to what the SDK reports: a flag that the override layer does not hold must evaluate, and appear in AllFlagsState, as it does without an override source. Three paths broke this. The first two were found while reviewing the same code in the .NET port (dotnet-core #364), the third by the contract tests in launchdarkly/sdk-test-harness#539.

  1. AllFlagsState with no LaunchDarkly data and an override source configured. The state was built from the merged read through the overlay, so the flags of a store that is not initialized (a persistent store that holds flags but no init marker) entered it. With an empty layer, that returned a valid state of LaunchDarkly data that the SDK returns as invalid without an override source; with overrides present, the state held those flags alongside the overrides. Without an override source the SDK reports no flags in this state.
  2. AllFlagsState when the store read fails. Overlay.GetAll returned the layer's entries alone, with no error, when the base store failed and the layer held entries. An initialized client (daemon mode, for example) whose store fails GetAll then returned a state marked valid with only the override flags and logged nothing. Without an override source, the same failure logs it and returns an invalid, empty state.
  3. Evaluation with no LaunchDarkly data and an override source configured. Every key was read through the overlay, so a flag that the layer does not hold fell through to the store, and a store that holds flags but no init marker served them. Without an override source the same evaluation returns the not-ready default.

The fixes:

  • The overrides-only state is built from the layer's flags directly, read through a new FDv2.OverrideItems, and only when no LaunchDarkly data is available; a layer without flags still gives an invalid state. Without an override source the SDK reports no flags in this state, so the override flags only add to it.
  • Overlay.GetAll returns the base error whatever the layer holds. AllFlagsState is its only caller; per-key Get still serves an override entry without the base. When the store's read fails, AllFlagsState logs the failure as before and returns an invalid state that holds the override layer's flags, and with none it is empty as without a source. flagstate.AllFlagsBuilder gains Valid(bool) so that a state can be invalid and still hold flags. A client-side SDK bootstrapped from the state uses its flags whatever $valid says (js-client-sdk 3.x and the 4.x browser SDK only log a warning), so the overridden values still reach it.
  • With no LaunchDarkly data, evaluation reads the flag from the layer itself through a new FDv2.OverrideItem, and a miss returns the not-ready default. Prerequisites and segments are still read through the overlay.

These follow OVERRIDE Requirements 1.2.2 and 1.5.3, and Requirements 1.4.1.2 and 1.4.1.3 proposed in launchdarkly/sdk-specs#280, which replace the sentence in Requirement 1.4.1 that the uninitialized state contains only the overridden flags.

Tests

  • ldclient_overrides_test.go, with a persistent store that holds a flag but no init marker and a client that never initializes:
    • an empty override layer gives an invalid, empty state, and a layer with one flag gives a valid state with only that flag;
    • an empty override layer gives the not-ready default for the store's flag, and a layer that holds another flag gives the not-ready default for the store's flag while serving the overridden one.
  • ldclient_overrides_test.go, in daemon mode: with an override present, a store read failure gives an invalid state that holds the overridden flag with its marked reason and tracking off, and logs the failure; with no override flags the same failure gives an invalid, empty state.
  • interfaces/flagstate/flags_state_test.go: a builder marked invalid keeps its flags and serializes "$valid": false with them.
  • internal/overrides/overrides_test.go: the overlay test that pinned the old behavior now asserts that GetAll returns the base error with entries in the layer, while Get still serves the override entry.

Each fix fails its tests when reverted. go test -race ./... passes and make lint reports no issues. The contract tests in launchdarkly/sdk-test-harness#539 fail on v7 and pass on this branch, together with the full Redis persistent data store and flag overrides suites (131 tests).

SDK-3419

Opened from termag

@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

1 similar comment
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment •

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Stale Bugbot comment from a previous run.

… store fails

When the store cannot list its flags, AllFlagsState still logs the
failure and returns an invalid state, as it does without an override
source. Overrides only add to that state, so it now holds the override
layer's flags. A client-side SDK bootstrapped from the state uses its
flags whatever $valid says, so the overridden values still reach it.
With no override flags the state is invalid and empty, as before.

flagstate.AllFlagsBuilder gains Valid(bool), so a state can be invalid
and still hold flags.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

When the store cannot list its flags and the override layer holds
flags, the warning says that the state is invalid and holds only the
override flags, instead of saying that it is empty.
@kinyoklion

Copy link
Copy Markdown
Member Author

bugbot review

@cursor cursor Bot left a comment

Copy link
Copy Markdown

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 using default effort 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 e1a9dbc. Configure here.

Comment thread ldclient.go Resolved
@kinyoklion
kinyoklion marked this pull request as ready for review October 30, 2026 23:40
@kinyoklion
kinyoklion requested a review from a team as a code owner October 30, 2026 23:40
…e no LaunchDarkly data is available

With an override source configured and no LaunchDarkly data, evaluation read every key through the overlay. A key that the override layer does not hold fell through to the store, and a persistent store that holds flags but no init marker served them. Without an override source the same evaluation returns the not-ready default. The flag is now read from the override layer itself, and a miss returns the not-ready default.
@kinyoklion kinyoklion changed the title fix: Build the not-initialized all-flags state from overrides alone and report a store failure fix: Keep data from a store that is not initialized out of override evaluations and AllFlagsState, and report a store failure Nov 2, 2026
kinyoklion added a commit to launchdarkly/sdk-test-harness that referenced this pull request Nov 3, 2026
…thout the init key (#539)

<!-- TERMAG_PR_BODY_BEGIN -->
## Summary

Overrides only add to what the SDK reports: a flag that the override
layer does not hold evaluates as it does without an override source, and
the all-flags state holds every flag it holds without one, plus the
overridden flags. A client that has not initialized is where SDKs broke
this, and only when the store holds flags without being initialized, for
example a persistent store that holds data but no init key. The flag
overrides suite uses the SDK's in-memory store, which is empty until a
data source provides data, so it could not tell a read of the override
layer apart from a read of everything the store returns.

A new `flag overrides` group in the persistent data store suite
(`server_side_persistence_overrides.go`) runs when the test service has
the `flag-overrides` capability. It writes flags to the store without
the init key and gives the client a data source that never provides
data. Each test compares a client with an override source to a client
without one against the same store, so it holds whatever an SDK reports
for an uninitialized store:

- **a flag without an override evaluates as it does without an override
source**: an override file that holds a different flag leaves the
evaluation of a store flag unchanged (OVERRIDE Requirements 1.5.1 and
1.5.3).
- **an empty override source leaves the all-flags state unchanged**
(Requirement 1.2.2).
- **overrides add to the all-flags state and make it valid**: the state
holds exactly the flags of the client without a source, with override
values for the overridden keys, plus the flags that exist only in the
override layer, and it is valid (launchdarkly/sdk-specs#280,
Requirements 1.4.1.2 and 1.4.1.3).

The flag overrides suite gains **evaluate all flags returns only the
override flags as a valid state** in its uninitialized client group,
where the store is empty and every SDK reports only the override flags.

## Results

Run with `-enable-persistence-tests` against Redis.

| Test service | flag without an override | empty override source |
overrides add |
|---|---|---|---|
| Go `v7` | fails: served from the store | fails: valid state of the
store's flags | fails: the store's flags join the overrides |
| Go launchdarkly/go-server-sdk#491 | passes | passes | passes |
| Node `main` | passes | fails: valid state of the store's flags |
passes |
| Node launchdarkly/js-core#2132 first commit (`57498a9e`) | passes |
passes | fails: drops a store flag the state holds without overrides |
| Node launchdarkly/js-core#2132 | passes | passes | passes |
| Python (top of the launchdarkly/python-server-sdk#530 stack, sync and
async) | passes | passes | passes |
| Ruby (top of the launchdarkly/ruby-server-sdk#454 stack) | passes |
passes | passes |

The Java (launchdarkly/java-core#225) and .NET (`dotnet-server-sdk-v9`)
test services do not support persistent stores, so the group skips
there; the new uninitialized client test passes on both. The full Redis
persistent data store suite and the flag overrides suite pass against Go
#491 at `30d4f216` (131 tests run). `golangci-lint` reports no issues.

Go and Node fail these tests until launchdarkly/go-server-sdk#491 and
launchdarkly/js-core#2132 are merged, for test services whose CI runs
the persistent data store suite with this harness version.

[SDK-3419](https://launchdarkly.atlassian.net/browse/SDK-3419)
<!-- TERMAG_PR_BODY_END -->

<!-- termag: box=i-0575d1c54853058d7 project=sdk-test-harness -->
<sub>Opened from termag</sub>


[SDK-3419]:
https://launchdarkly.atlassian.net/browse/SDK-3419?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Overview**
> Adds harness coverage for **file flag overrides** when the SDK never
initializes but a **persistent store already holds feature data without
the `$inited` key**—a case the existing in-memory override suite could
not distinguish from “read everything from the store.”
> 
> A new **`flag overrides`** group under the server-side persistent data
store suite (`server_side_persistence_overrides.go`, gated on
`flag-overrides`) seeds Redis/Consul/Dynamo with flags, uses a blocking
unavailable data source, and compares clients with and without override
files via `uninitializedClient`. Scenarios assert: store flags not in
the override file behave like no override source; an empty override file
leaves **evaluate all flags** unchanged; overrides merge into all-flags
state, mark it **valid**, and apply override values while retaining
other store flags.
> 
> The standalone flag-overrides **`uninitialized client`** group gains
**evaluate all flags returns only the override flags as a valid state**
(empty store, overrides-only valid state). The persistent suite
registers the new group from `server_side_persistence_base.go`.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
09a8e02. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
@kinyoklion
kinyoklion merged commit 1005a1a into v7 Nov 4, 2026
36 of 37 checks passed
@kinyoklion
kinyoklion deleted the rlamb/overrides-go-allflags-overrides-only branch November 4, 2026 18:37
beekld pushed a commit that referenced this pull request Nov 6, 2026
🤖 I have created a release *beep* *boop*
---


##
[7.22.0](v7.21.2...v7.22.0)
(2026-11-06)


### Features

* Broadcast Off to FDv2 status listeners when the data system stops
([#505](#505))
([5c5b8ce](5c5b8ce))
* End an FDv1 fallback after a default TTL of one hour
([#499](#499))
([1f03a5f](1f03a5f))
* Park FDv2 synchronizers instead of retiring them on unexpected
failures
([#489](#489))
([644253a](644253a))


### Bug Fixes

* Keep data from a store that is not initialized out of override
evaluations and AllFlagsState, and report a store failure
([#491](#491))
([1005a1a](1005a1a))
* Keep the override marking when a migration flag serves an unrecognized
stage ([#492](#492))
([9f9911f](9f9911f))
* Leave StateSince alone when a synchronizer starts
([#502](#502))
([823b9aa](823b9aa))
* Make TestData ValueForAll return its value when targeting is off
([#507](#507))
([7300bc1](7300bc1))
* Park a synchronizer that reports Off or closes its results
([#500](#500))
([f6e9910](f6e9910))
* Poll the override files when they cannot be watched
([#495](#495))
([170ad3b](170ad3b))
* Prevent a full status listener from hanging Close
([#498](#498))
([9efd570](9efd570))
* Say that the data system retries after an unrecoverable HTTP error
([#501](#501))
([d4066cc](d4066cc))
* Treat a repeated override file path in a different case as a repeat on
Windows
([#494](#494))
([eeb68d3](eeb68d3))
* Warn when offline mode skips the override source
([#493](#493))
([69bbe61](69bbe61))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Overview**
> **Cuts the Go server SDK release `7.22.0` and records its notes.**
> 
> The root **`.release-please-manifest.json`** and
**`internal/version.go`** now declare **`7.22.0`** (replacing `7.21.2`).
**`CHANGELOG.md`** gains a new **`[7.22.0]`** section dated
**2026-11-06** documenting shipped work: FDv2 data-system behavior
(status **Off** broadcasts, synchronizer **parking** instead of retire,
**FDv1 fallback TTL**, **StateSince** handling, parking on
**Off**/closed results), plus multiple **bug fixes** around
uninitialized store data in overrides/**AllFlagsState**, migration
override marking, **TestData** when targeting is off, **override file**
polling/deduping on Windows, offline-mode warnings, status listener
**Close** hangs, and messaging for unrecoverable HTTP retries.
> 
> No application code changes appear in this PR; it’s version
bookkeeping and release notes only.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit
c763cc0. Bugbot is set up for automated
code reviews on this repo. Configure
[here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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