Repository navigation
fix: Base the validity of the uninitialized all flags state on the override layer's flags - #2132
kinyoklion wants to merge 2 commits into
Conversation
Before the client is initialized and while the store is not initialized, allFlagsState read the merged view and reported a valid state whenever it returned any flag and a source was configured. A store that holds LaunchDarkly flags without being initialized therefore produced a valid state full of LaunchDarkly data when the layer was empty, and LaunchDarkly flags next to the overridden ones when it was not. The overrides-only state is now built from the layer's flags, and a layer without flags leaves the state as it is without an override source.
|
bugbot review |
|
@launchdarkly/js-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/js-client-sdk-common size report |
|
bugbot review |
… overrides are present Without a source, an uninitialized store's flags are evaluated into an invalid state. Building the state from the override layer alone dropped them whenever the layer held flags, so configuring overrides removed flags from the state. The read goes through the overlay again, so the state holds the store's flags with the override entries taking precedence; whether the layer holds flags still decides the validity, so an empty layer leaves the state exactly as without a source.
kinyoklion
changed the title
|
bugbot review |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ Bugbot reviewed your changes and found no new issues!
Comment @cursor review or bugbot run to trigger another review on this PR
Reviewed by Cursor Bugbot for commit 4d26701. Configure here.
Summary
Before the client is initialized, and while the data store is not initialized either,
allFlagsStateevaluates whatever flags the store holds into an invalid state. A store can hold LaunchDarkly flags without being initialized, for example a persistent store with data but without the marker that a complete data set was written. With an override source configured, the state was marked valid whenever the merged read through the overlay returned any flag, so a source whose layer is empty turned that invalid state into a valid one. Configuring a source that supplies nothing must change nothing (OVERRIDE Requirement 1.2.2).Whether the override layer holds flags now decides the validity, instead of whether the merged read returned any. The read itself still goes through the overlay, so the state holds the store's flags with the override layer's entries taking precedence. Overrides only add to the flags the state would report without them, and never remove one: a layer with flags gives a valid state of the store's flags plus the overrides, and a layer without flags gives exactly the state that a client without an override source gives. The once-per-client warning now says that the state holds flags from the override layer and the data store.
This was found while reviewing the same code in the .NET port (launchdarkly/dotnet-core#364). In the other server-side SDKs this branch returned an empty state before overrides, so a state of the override flags alone adds to it there; in this SDK the store's flags were already in the state, so they stay.
Tests
Two tests in
LDClientImpl.overrides.test.tsuse a persistent store that holds LaunchDarkly flags but reports that it is not initialized. One checks that a client with an empty override source reports the same invalid state as a client without a source. The other checks that a client with overrides reports the store's flags with the overrides added and taking precedence for a shared key. Reading the layer alone fails the second; deciding validity from the merged read fails the first. The sdk-server suite passes (1561 passed, 7 skipped), and lint and formatting are clean.Opened from termag
Note
Overview
When the client and feature store are both uninitialized,
allFlagsStateno longer treats the merged overlay read as the source of truth forvalid. Validity now depends on whether the override layer actually holds feature flags—aligning with OVERRIDE requirement 1.2.2 so an empty override source cannot flip an otherwise invalid state to valid.Evaluation still reads through the overlay: store flags remain in the snapshot, overrides add and win on key collision, and an empty override layer yields the same invalid outcome as having no override source. The one-time warning now states that results come from both the override layer and the data store, not overrides alone.
Tests add a persistent store that has flag data but reports
initialized: false, plus updated expectations for the warning message.Reviewed by Cursor Bugbot for commit 4d26701. Bugbot is set up for automated code reviews on this repo. Configure here.