diff --git a/packages/sdk/browser/__tests__/BrowserClient.test.ts b/packages/sdk/browser/__tests__/BrowserClient.test.ts index bd6a7e3cae..089d19286e 100644 --- a/packages/sdk/browser/__tests__/BrowserClient.test.ts +++ b/packages/sdk/browser/__tests__/BrowserClient.test.ts @@ -189,6 +189,43 @@ describe('given a mock platform for a BrowserClient', () => { }); }); + it('can evaluate flags with bootstrap data before identify completes', async () => { + const client = makeClient( + 'client-side-id', + AutoEnvAttributes.Disabled, + { + streaming: false, + logger, + diagnosticOptOut: true, + }, + platform, + ); + + const identifyPromise = client.identify( + { kind: 'user', key: 'bob' }, + { + bootstrap: goodBootstrapDataWithReasons, + }, + ); + + const flagValue = client.jsonVariationDetail('json', undefined); + expect(flagValue).toEqual({ + reason: { + kind: 'OFF', + }, + value: ['a', 'b', 'c', 'd'], + variationIndex: 1, + }); + + expect(client.getContext()).toBeUndefined(); + + // Wait for identify to complete + await identifyPromise; + + // Verify that active context is now set + expect(client.getContext()).toEqual({ kind: 'user', key: 'bob' }); + }); + it('can shed intermediate identify calls', async () => { const client = makeClient( 'client-side-id', diff --git a/packages/sdk/browser/src/BrowserClient.ts b/packages/sdk/browser/src/BrowserClient.ts index 454206aad0..0cffec51d4 100644 --- a/packages/sdk/browser/src/BrowserClient.ts +++ b/packages/sdk/browser/src/BrowserClient.ts @@ -19,6 +19,7 @@ import { Platform, } from '@launchdarkly/js-client-sdk-common'; +import { readFlagsFromBootstrap } from './bootstrap'; import { getHref } from './BrowserApi'; import BrowserDataManager from './BrowserDataManager'; import { BrowserIdentifyOptions as LDIdentifyOptions } from './BrowserIdentifyOptions'; @@ -44,6 +45,10 @@ class BrowserClientImpl extends LDClientImpl { private _initResolve?: (result: LDWaitForInitializationResult) => void; private _initializeResult?: LDWaitForInitializationResult; + // NOTE: keeps track of when we tried an initial identification. We should consolidate this + // with the waitForInitialization logic in the future. + private _identifyAttempted: boolean = false; + constructor( clientSideId: string, autoEnvAttributes: AutoEnvAttributes, @@ -222,6 +227,20 @@ class BrowserClientImpl extends LDClientImpl { if (identifyOptions?.sheddable === undefined) { identifyOptionsWithUpdatedDefaults.sheddable = true; } + + if (!this._identifyAttempted && identifyOptionsWithUpdatedDefaults.bootstrap) { + this._identifyAttempted = true; + const bootstrapData = readFlagsFromBootstrap( + this.logger, + identifyOptionsWithUpdatedDefaults.bootstrap, + ); + try { + this.presetFlags(bootstrapData); + } catch { + this.logger.error('Failed to bootstrap data'); + } + } + const res = await super.identifyResult(context, identifyOptionsWithUpdatedDefaults); if (res.status === 'completed') { this._initializeResult = { status: 'complete' }; diff --git a/packages/shared/sdk-client/package.json b/packages/shared/sdk-client/package.json index d01ea875b4..489772a11c 100644 --- a/packages/shared/sdk-client/package.json +++ b/packages/shared/sdk-client/package.json @@ -37,7 +37,7 @@ "build": "npx tsc --noEmit && rollup -c rollup.config.js && npm run make-package-jsons", "clean": "rimraf dist", "lint": "npx eslint . --ext .ts", - "lint:fix": "yarn run lint -- --fix", + "lint:fix": "npx eslint . --ext .ts --fix", "prettier": "prettier --write 'src/*.@(js|ts|tsx|json)'", "check": "yarn && yarn prettier && yarn lint && tsc && yarn test" }, diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index 0e45544483..86336c25a9 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -32,6 +32,10 @@ import { LDIdentifyOptions } from './api/LDIdentifyOptions'; import { createAsyncTaskQueue } from './async/AsyncTaskQueue'; import { Configuration, ConfigurationImpl, LDClientInternalOptions } from './configuration'; import { addAutoEnv } from './context/addAutoEnv'; +import { + ActiveContextTracker, + createActiveContextTracker, +} from './context/createActiveContextTracker'; import { ensureKey } from './context/ensureKey'; import { DataManager, DataManagerFactory } from './DataManager'; import createDiagnosticsManager from './diagnostics/createDiagnosticsManager'; @@ -43,6 +47,7 @@ import createEventProcessor from './events/createEventProcessor'; import EventFactory from './events/EventFactory'; import DefaultFlagManager, { FlagManager } from './flag-manager/FlagManager'; import { FlagChangeType } from './flag-manager/FlagUpdater'; +import { ItemDescriptor } from './flag-manager/ItemDescriptor'; import HookRunner from './HookRunner'; import { getInspectorHook } from './inspection/getInspectorHook'; import InspectorManager from './inspection/InspectorManager'; @@ -55,12 +60,12 @@ const DEFAULT_IDENTIFY_TIMEOUT_SECONDS = 5; export default class LDClientImpl implements LDClient, LDClientIdentifyResult { private readonly _config: Configuration; - private _uncheckedContext?: LDContext; - private _checkedContext?: Context; private readonly _diagnosticsManager?: internal.DiagnosticsManager; private _eventProcessor?: internal.EventProcessor; readonly logger: LDLogger; + private _activeContextTracker: ActiveContextTracker = createActiveContextTracker(); + private readonly _highTimeoutThreshold: number = 15; private _eventFactoryDefault = new EventFactory(false); @@ -200,27 +205,22 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { // code. We are returned the unchecked context so that if a consumer identifies with an invalid context // and then calls getContext, they get back the same context they provided, without any assertion about // validity. - return this._uncheckedContext ? clone(this._uncheckedContext) : undefined; + return this._activeContextTracker.hasContext() + ? clone(this._activeContextTracker.getPristineContext()) + : undefined; } protected getInternalContext(): Context | undefined { - return this._checkedContext; + return this._activeContextTracker.getContext(); } - private _createIdentifyPromise(): { - identifyPromise: Promise; - identifyResolve: () => void; - identifyReject: (err: Error) => void; - } { - let res: any; - let rej: any; - - const basePromise = new Promise((resolve, reject) => { - res = resolve; - rej = reject; - }); - - return { identifyPromise: basePromise, identifyResolve: res, identifyReject: rej }; + /** + * Preset flags are used to set the flags before the client is initialized. This is useful for + * when client has precached flags that are ready to evaluate without full initialization. + * @param newFlags - The flags to preset. + */ + protected presetFlags(newFlags: { [key: string]: ItemDescriptor }) { + this._flagManager.presetFlags(newFlags); } /** @@ -307,15 +307,14 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { this.emitter.emit('error', context, error); return Promise.reject(error); } - this._uncheckedContext = context; - this._checkedContext = checkedContext; + this._activeContextTracker.set(context, checkedContext); this._eventProcessor?.sendEvent( - this._eventFactoryDefault.identifyEvent(this._checkedContext), + this._eventFactoryDefault.identifyEvent(checkedContext), ); const { identifyPromise, identifyResolve, identifyReject } = - this._createIdentifyPromise(); - this.logger.debug(`Identifying ${JSON.stringify(this._checkedContext)}`); + this._activeContextTracker.newIdentificationPromise(); + this.logger.debug(`Identifying ${JSON.stringify(checkedContext)}`); await this.dataManager.identify( identifyResolve, @@ -370,7 +369,7 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { } track(key: string, data?: any, metricValue?: number): void { - if (!this._checkedContext || !this._checkedContext.valid) { + if (!this._activeContextTracker.hasValidContext()) { this.logger.warn(ClientMessages.MissingContextKeyNoEvent); return; } @@ -382,14 +381,19 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { this._eventProcessor?.sendEvent( this._config.trackEventModifier( - this._eventFactoryDefault.customEvent(key, this._checkedContext!, data, metricValue), + this._eventFactoryDefault.customEvent( + key, + this._activeContextTracker.getContext()!, + data, + metricValue, + ), ), ); this._hookRunner.afterTrack({ key, // The context is pre-checked above, so we know it can be unwrapped. - context: this._uncheckedContext!, + context: this._activeContextTracker.getPristineContext()!, data, metricValue, }); @@ -401,12 +405,20 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { eventFactory: EventFactory, typeChecker?: (value: any) => [boolean, string], ): LDEvaluationDetail { - if (!this._uncheckedContext) { - this.logger.debug(ClientMessages.MissingContextKeyNoEvent); - return createErrorEvaluationDetail(ErrorKinds.UserNotSpecified, defaultValue); + // We are letting evaulations happen without a context. The main case for this + // is when cached data is loaded, but the client is not fully initialized. In this + // case, we will write out a warning for each evaluation attempt. + + // NOTE: we will be changing this behavior soon once we have a tracker on the + // client initialization state. + const hasContext = this._activeContextTracker.hasContext(); + if (!hasContext) { + this.logger?.warn( + 'Flag evaluation called before client is fully initialized, data from this evaulation could be stale.', + ); } - const evalContext = Context.fromLDContext(this._uncheckedContext); + const evalContext = this._activeContextTracker.getContext()!; const foundItem = this._flagManager.get(flagKey); if (foundItem === undefined || foundItem.flag.deleted) { @@ -414,10 +426,13 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { const error = new LDClientError( `Unknown feature flag "${flagKey}"; returning default value ${defVal}.`, ); - this.emitter.emit('error', this._uncheckedContext, error); - this._eventProcessor?.sendEvent( - this._eventFactoryDefault.unknownFlagEvent(flagKey, defVal, evalContext), - ); + + this.emitter.emit('error', this._activeContextTracker.getPristineContext(), error); + if (hasContext) { + this._eventProcessor?.sendEvent( + this._eventFactoryDefault.unknownFlagEvent(flagKey, defVal, evalContext), + ); + } return createErrorEvaluationDetail(ErrorKinds.FlagNotFound, defaultValue); } @@ -426,20 +441,22 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { if (typeChecker) { const [matched, type] = typeChecker(value); if (!matched) { - this._eventProcessor?.sendEvent( - eventFactory.evalEventClient( - flagKey, - defaultValue, // track default value on type errors - defaultValue, - foundItem.flag, - evalContext, - reason, - ), - ); + if (hasContext) { + this._eventProcessor?.sendEvent( + eventFactory.evalEventClient( + flagKey, + defaultValue, // track default value on type errors + defaultValue, + foundItem.flag, + evalContext, + reason, + ), + ); + } const error = new LDClientError( `Wrong type "${type}" for feature flag "${flagKey}"; returning default value`, ); - this.emitter.emit('error', this._uncheckedContext, error); + this.emitter.emit('error', this._activeContextTracker.getPristineContext(), error); return createErrorEvaluationDetail(ErrorKinds.WrongType, defaultValue); } } @@ -453,31 +470,36 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { prerequisites?.forEach((prereqKey) => { this._variationInternal(prereqKey, undefined, this._eventFactoryDefault); }); - this._eventProcessor?.sendEvent( - eventFactory.evalEventClient( - flagKey, - value, - defaultValue, - foundItem.flag, - evalContext, - reason, - ), - ); + if (hasContext) { + this._eventProcessor?.sendEvent( + eventFactory.evalEventClient( + flagKey, + value, + defaultValue, + foundItem.flag, + evalContext, + reason, + ), + ); + } return successDetail; } variation(flagKey: string, defaultValue?: LDFlagValue): LDFlagValue { const { value } = this._hookRunner.withEvaluation( flagKey, - this._uncheckedContext, + this._activeContextTracker.getPristineContext(), defaultValue, () => this._variationInternal(flagKey, defaultValue, this._eventFactoryDefault), ); return value; } variationDetail(flagKey: string, defaultValue?: LDFlagValue): LDEvaluationDetail { - return this._hookRunner.withEvaluation(flagKey, this._uncheckedContext, defaultValue, () => - this._variationInternal(flagKey, defaultValue, this._eventFactoryWithReasons), + return this._hookRunner.withEvaluation( + flagKey, + this._activeContextTracker.getPristineContext(), + defaultValue, + () => this._variationInternal(flagKey, defaultValue, this._eventFactoryWithReasons), ); } @@ -487,8 +509,11 @@ export default class LDClientImpl implements LDClient, LDClientIdentifyResult { eventFactory: EventFactory, typeChecker: (value: unknown) => [boolean, string], ): LDEvaluationDetailTyped { - return this._hookRunner.withEvaluation(key, this._uncheckedContext, defaultValue, () => - this._variationInternal(key, defaultValue, eventFactory, typeChecker), + return this._hookRunner.withEvaluation( + key, + this._activeContextTracker.getPristineContext(), + defaultValue, + () => this._variationInternal(key, defaultValue, eventFactory, typeChecker), ); } diff --git a/packages/shared/sdk-client/src/context/createActiveContextTracker.ts b/packages/shared/sdk-client/src/context/createActiveContextTracker.ts new file mode 100644 index 0000000000..b9dde1359e --- /dev/null +++ b/packages/shared/sdk-client/src/context/createActiveContextTracker.ts @@ -0,0 +1,92 @@ +import { Context, LDContext } from '@launchdarkly/js-sdk-common'; + +/** + * ActiveContextTracker is an internal class that helps tracks the current active context + * used by the client. + */ +export interface ActiveContextTracker { + _pristineContext?: LDContext; + _context?: Context; + + /** + * Set the active context and pristine context. This will only be called when the passed in context + * is checked and valid. + * + * @param pristineContext - The pristine context, which is the context as it was passed in to the SDK. + * @param context - The active context, which is the context as it was checked and validated. + */ + set(pristineContext: LDContext, context: Context): void; + + /** + * Get the active context. + * + * @returns The active context or undefined if it has not been set. + */ + getContext(): Context | undefined; + + /** + * Get the pristine context. + * + * @returns The pristine context or undefined if it has not been set. + */ + getPristineContext(): LDContext | undefined; + + /** + * Create a new identification promise. To allow other parts of the SDK to track the identification process. + * + * TODO(self): this is a very generic method so maybe it doesn't belong here? + */ + newIdentificationPromise(): { + identifyPromise: Promise; + identifyResolve: () => void; + identifyReject: (err: Error) => void; + }; + + /** + * Check if the active context is set. Regardless of whether it is valid or not. + * + * @returns True if the active context is set, false otherwise. + */ + hasContext(): boolean; + + /** + * Check if the active context is valid. + * + * @returns True if the active context is valid, false otherwise. + */ + hasValidContext(): boolean; +} + +export function createActiveContextTracker(): ActiveContextTracker { + return { + _pristineContext: undefined, + _context: undefined, + set(pristineContext: LDContext, context: Context) { + this._pristineContext = pristineContext; + this._context = context; + }, + getContext() { + return this._context; + }, + getPristineContext() { + return this._pristineContext; + }, + newIdentificationPromise() { + let res: () => void; + let rej: (err: Error) => void; + + const basePromise = new Promise((resolve, reject) => { + res = resolve; + rej = reject; + }); + + return { identifyPromise: basePromise, identifyResolve: res!, identifyReject: rej! }; + }, + hasContext() { + return this._context !== undefined; + }, + hasValidContext() { + return this.hasContext() && this._context!.valid; + }, + }; +} diff --git a/packages/shared/sdk-client/src/context/ensureKey.ts b/packages/shared/sdk-client/src/context/ensureKey.ts index 877ef7dd83..cc2887dec7 100644 --- a/packages/shared/sdk-client/src/context/ensureKey.ts +++ b/packages/shared/sdk-client/src/context/ensureKey.ts @@ -10,7 +10,7 @@ import { } from '@launchdarkly/js-sdk-common'; import { getOrGenerateKey } from '../storage/getOrGenerateKey'; -import { namespaceForAnonymousGeneratedContextKey } from '../storage/namespaceUtils'; +import { namespaceForGeneratedContextKey } from '../storage/namespaceUtils'; const { isLegacyUser, isMultiKind, isSingleKind } = internal; @@ -31,7 +31,7 @@ const ensureKeyCommon = async (kind: string, c: LDContextCommon, platform: Platf const { anonymous, key } = c; if (anonymous && !key) { - const storageKey = await namespaceForAnonymousGeneratedContextKey(kind); + const storageKey = await namespaceForGeneratedContextKey(kind); // This mutates a cloned copy of the original context from ensureyKey so this is safe. // eslint-disable-next-line no-param-reassign c.key = await getOrGenerateKey(storageKey, platform); diff --git a/packages/shared/sdk-client/src/flag-manager/FlagManager.ts b/packages/shared/sdk-client/src/flag-manager/FlagManager.ts index 61338c4f74..abf6621315 100644 --- a/packages/shared/sdk-client/src/flag-manager/FlagManager.ts +++ b/packages/shared/sdk-client/src/flag-manager/FlagManager.ts @@ -40,6 +40,15 @@ export interface FlagManager { */ loadCached(context: Context): Promise; + /** + * Updates in-memory storage with the specified flags without a context + * or persistent storage. Flags set in this way are considered emphemeral and + * should be replaced as soon as initialization is done. + * + * @param newFlags - cached flags + */ + presetFlags(newFlags: { [key: string]: ItemDescriptor }): void; + /** * Update in-memory storage with the specified flags, but do not persistent them to cache * storage. @@ -114,6 +123,10 @@ export default class DefaultFlagManager implements FlagManager { return this._flagStore.getAll(); } + presetFlags(newFlags: { [key: string]: ItemDescriptor }): void { + this._flagStore.init(newFlags); + } + setBootstrap(context: Context, newFlags: { [key: string]: ItemDescriptor }): void { // Bypasses the persistence as we do not want to put these flags into any cache. // Generally speaking persistence likely *SHOULD* be disabled when using bootstrap. diff --git a/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts b/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts index c7b1a130f0..03366dacf8 100644 --- a/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts +++ b/packages/shared/sdk-client/src/flag-manager/FlagUpdater.ts @@ -33,7 +33,7 @@ export type FlagsChangeCallback = ( export default class FlagUpdater { private _flagStore: FlagStore; private _logger: LDLogger; - private _activeContextKey: string | undefined; + private _activeContext: Context | undefined; private _changeCallbacks = new Array(); constructor(flagStore: FlagStore, logger: LDLogger) { @@ -42,7 +42,7 @@ export default class FlagUpdater { } init(context: Context, newFlags: { [key: string]: ItemDescriptor }) { - this._activeContextKey = context.canonicalKey; + this._activeContext = context; const oldFlags = this._flagStore.getAll(); this._flagStore.init(newFlags); const changed = calculateChangedKeys(oldFlags, newFlags); @@ -58,7 +58,7 @@ export default class FlagUpdater { } initCached(context: Context, newFlags: { [key: string]: ItemDescriptor }) { - if (this._activeContextKey === context.canonicalKey) { + if (this._activeContext?.canonicalKey === context.canonicalKey) { return; } @@ -66,7 +66,7 @@ export default class FlagUpdater { } upsert(context: Context, key: string, item: ItemDescriptor): boolean { - if (this._activeContextKey !== context.canonicalKey) { + if (this._activeContext?.canonicalKey !== context.canonicalKey) { this._logger.warn('Received an update for an inactive context.'); return false; } @@ -80,7 +80,7 @@ export default class FlagUpdater { this._flagStore.insertOrUpdate(key, item); this._changeCallbacks.forEach((callback) => { try { - callback(context, [key], 'patch'); + callback(this._activeContext!, [key], 'patch'); } catch (err) { /* intentionally empty */ }