fix: bootstrapped client can do evaluation when not ready #1024
Conversation
This commit will allow variation calls to use bootstrapped values during the client initialization process.
|
@launchdarkly/browser size report |
|
@launchdarkly/js-client-sdk-common size report |
|
@launchdarkly/js-client-sdk size report |
|
@launchdarkly/js-sdk-common size report |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| this._uncheckedContext = pristineContext; | ||
| this._checkedContext = Context.fromLDContext(this._uncheckedContext); | ||
| this._flagManager.setBootstrap(this._checkedContext, newFlags); | ||
| } |
There was a problem hiding this comment.
Bug: Bootstrap fails silently for contexts without valid keys
The setBootstrap method doesn't validate that the context is valid before calling _flagManager.setBootstrap. When Context.fromLDContext creates an invalid context (e.g., for anonymous contexts without keys like { kind: 'user', anonymous: true }), accessing canonicalKey on this invalid context throws a TypeError because _context is undefined. This error is caught in BrowserClient.identifyResult and logged as "failed to bootstrap data" without explaining the root cause. The bootstrap data is silently lost, and users won't understand why their bootstrap configuration isn't working for anonymous contexts.
Additional Locations (1)
| this.logger, | ||
| identifyOptionsWithUpdatedDefaults.bootstrap, | ||
| ); | ||
| this.setBootstrap(context, bootstrapData); |
There was a problem hiding this comment.
Bug: Bootstrap uses unprocessed context causing potential key mismatch
The setBootstrap call receives the raw context parameter before super.identifyResult processes it through ensureKey and addAutoEnv. When AutoEnvAttributes are enabled, addAutoEnv converts single-kind contexts to multi-kind by adding ld_application and ld_device kinds, changing the canonical key. This means the _activeContextKey set during early bootstrap differs from the final processed context's canonical key. While this gets corrected when BrowserDataManager._finishIdentifyFromBootstrap is later called with the processed context, flag evaluations during the brief window between these calls may behave unexpectedly.
| */ | ||
| protected setBootstrap(pristineContext: LDContext, newFlags: { [key: string]: ItemDescriptor }) { | ||
| this._uncheckedContext = pristineContext; | ||
| this._checkedContext = Context.fromLDContext(this._uncheckedContext); |
There was a problem hiding this comment.
I assume this will still get ultimately set by the normal code path, which would then handle any normal context processing? Do we need to be setting context information at all? I think in the previous SDK bootstrap would fail to send events before init complete, so this may be better for some cases.
There was a problem hiding this comment.
I think, if nothing else, we need to make sure that the bootstrap data actually gets set even if the context has anonymous contexts without keys.
There was a problem hiding this comment.
Yea so it looks like the bootstrapping will fail when we use anonymous key... will look into this
| identifyOptionsWithUpdatedDefaults.sheddable = true; | ||
| } | ||
|
|
||
| if (!this._bootstrapAttempted && identifyOptionsWithUpdatedDefaults.bootstrap) { |
There was a problem hiding this comment.
I think we need to gate on only the very first identify if bootstrap is there.
|
replaced by #1036 |
) **Related issues** sdk-1653 sdk-1376 sdk-1663 sdk-1681 **Describe the solution you've provided** - Introduced `ActiveContextTracker` to manage the current active context and its serialized state. - Updated `LDClientImpl` to utilize the new context tracker for identifying and evaluating flags. - Added logic in `BrowserClientImpl` to read flags from bootstrap data during the initial identification process. - Added a new `presetFlags` function in client sdk common that allow flagstore to take in contexless data before initialization - Bootstrapped data will first `preset` the flag store so they can be evaluated before a full context is made - Added logic to suppress event creation if context is not validated (while the client only has preset data) **Additional context** - eventually I would like to consolidate the context tracker with the tracking logic we have for waitForInitialization and have a general state tracker for client sdk common. - I am also open to suggestions on distinguishing between "no context" state and "pre initialized" state. Though, it seems like, at the moment, those 2 states are handled in the same way. Supersedes #1024 <!-- CURSOR_SUMMARY --> --- > [!NOTE] > Enable flag evaluation from bootstrap data before identify completes and refactor context handling via ActiveContextTracker with event suppression when no valid context. > > - **Browser SDK**: > - Read bootstrap data in `BrowserClient.identifyResult` and `preset` flags for immediate evaluation; track first-identify via `_identifyAttempted`. > - Add test verifying flags evaluate from bootstrap before identify completes in `packages/sdk/browser/__tests__/BrowserClient.test.ts`. > - **Core SDK (shared)**: > - Introduce `ActiveContextTracker` (`src/context/createActiveContextTracker.ts`) and update `LDClientImpl` to use it for getting/setting context and coordinating identify promises. > - Allow evaluations without an active context (log warning) and suppress event creation when context is absent; add `presetFlags` to `LDClientImpl`. > - **Flag management**: > - Add `presetFlags` to `FlagManager`/`DefaultFlagManager` to initialize in-memory flags without persistence. > - Update `FlagUpdater` to track active `Context` (not just key) and adjust `init/initCached/upsert` and change callbacks accordingly. > > <sup>Written by [Cursor Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit b7b4bf8. This will update automatically on new commits. Configure [here](https://cursor.com/dashboard?tab=bugbot).</sup> <!-- /CURSOR_SUMMARY -->
This commit will allow variation calls to use bootstrapped values during the client initialization process.
The bootstrap configuration will only be done once per client instance as we expect this fall back will only be active for a brief amount of time.
Note
Enables pre-initialization flag evaluation by applying bootstrap flags once during identify, with a new shared setBootstrap hook.
packages/sdk/browser/src/BrowserClient.ts)identifyResult: read flags viareadFlagsFromBootstrap, callsetBootstrap(context, data), with a once-only guard (_bootstrapAttempted) and error logging.packages/shared/sdk-client/src/LDClientImpl.ts)setBootstrap(pristineContext, newFlags)to set context and initialize flags viaFlagManager.setBootstrap.ItemDescriptorfor bootstrap flag descriptors.Written by Cursor Bugbot for commit 15e59c7. This will update automatically on new commits. Configure here.