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.
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.