Repository navigation
fix: Base the validity of the uninitialized all flags state on the override layer's flags - #2132
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.
The general comment covers the uninitialized store, and the override caveat sits at the override check.
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
SDK-3419
Note
Overview
When
allFlagsStateruns before the client and persistent store are initialized, the SDK still reads flags through the store overlay, butLDFlagsState.validno longer follows “any merged flag came back.” Validity now depends only on whether the override layer has feature flags, so an empty override source cannot turn an otherwise invalid partial-store snapshot into a valid one (OVERRIDE 1.2.2).If the override layer has flags, the state is valid and includes store flags plus overrides (overrides win on key clashes); the once-per-client warning text now says flags come from both the override layer and the data store. If the layer is empty, behavior matches a client with no override source: invalid state with whatever the store returned, plus the existing “data store not available” path when appropriate.
Tests add an uninitialized store that still holds LD flags and assert the empty-source vs merged-override cases; existing override tests were updated for the new log message.
Reviewed by Cursor Bugbot for commit 6c26c5c. Bugbot is set up for automated code reviews on this repo. Configure here.