From 160e7c7a8faf6e2752cd1f6aaddbf2bd482a939e Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 2 Oct 2024 14:03:08 -0700 Subject: [PATCH 1/8] feat: Add support for hooks. --- .../sdk-client/__tests__/HookRunner.test.ts | 227 ++++++++++++++++++ packages/shared/sdk-client/src/HookRunner.ts | 164 +++++++++++++ .../shared/sdk-client/src/LDClientImpl.ts | 38 ++- .../shared/sdk-client/src/api/LDClient.ts | 11 + .../shared/sdk-client/src/api/LDOptions.ts | 20 ++ packages/shared/sdk-client/src/api/index.ts | 1 + .../sdk-client/src/api/integrations/Hooks.ts | 155 ++++++++++++ .../sdk-client/src/api/integrations/index.ts | 1 + .../src/configuration/Configuration.ts | 5 +- .../src/configuration/validators.ts | 1 + 10 files changed, 617 insertions(+), 6 deletions(-) create mode 100644 packages/shared/sdk-client/__tests__/HookRunner.test.ts create mode 100644 packages/shared/sdk-client/src/HookRunner.ts create mode 100644 packages/shared/sdk-client/src/api/integrations/Hooks.ts create mode 100644 packages/shared/sdk-client/src/api/integrations/index.ts diff --git a/packages/shared/sdk-client/__tests__/HookRunner.test.ts b/packages/shared/sdk-client/__tests__/HookRunner.test.ts new file mode 100644 index 0000000000..60ce7b7a12 --- /dev/null +++ b/packages/shared/sdk-client/__tests__/HookRunner.test.ts @@ -0,0 +1,227 @@ +import { LDContext, LDEvaluationDetail, LDLogger } from '@launchdarkly/js-sdk-common'; + +import { Hook, IdentifyResult } from '../src/api/integrations/Hooks'; +import HookRunner from '../src/HookRunner'; + +describe('given a hook runner and test hook', () => { + let logger: LDLogger; + let testHook: Hook; + let hookRunner: HookRunner; + + beforeEach(() => { + logger = { + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), + }; + + testHook = { + getMetadata: jest.fn().mockReturnValue({ name: 'Test Hook' }), + beforeEvaluation: jest.fn(), + afterEvaluation: jest.fn(), + beforeIdentify: jest.fn(), + afterIdentify: jest.fn(), + }; + + hookRunner = new HookRunner(logger, [testHook]); + }); + + describe('when evaluating flags', () => { + it('should execute hooks and return the evaluation result', () => { + const key = 'test-flag'; + const context: LDContext = { kind: 'user', key: 'user-123' }; + const defaultValue = false; + const evaluationResult: LDEvaluationDetail = { + value: true, + variationIndex: 1, + reason: { kind: 'OFF' }, + }; + + const method = jest.fn().mockReturnValue(evaluationResult); + + const result = hookRunner.withEvaluation(key, context, defaultValue, method); + + expect(testHook.beforeEvaluation).toHaveBeenCalledWith( + expect.objectContaining({ + flagKey: key, + context, + defaultValue, + }), + {}, + ); + + expect(method).toHaveBeenCalled(); + + expect(testHook.afterEvaluation).toHaveBeenCalledWith( + expect.objectContaining({ + flagKey: key, + context, + defaultValue, + }), + {}, + evaluationResult, + ); + + expect(result).toEqual(evaluationResult); + }); + + it('should handle errors in hooks', () => { + const errorHook: Hook = { + getMetadata: jest.fn().mockReturnValue({ name: 'Error Hook' }), + beforeEvaluation: jest.fn().mockImplementation(() => { + throw new Error('Hook error'); + }), + afterEvaluation: jest.fn(), + }; + + const errorHookRunner = new HookRunner(logger, [errorHook]); + + const method = jest + .fn() + .mockReturnValue({ value: true, variationIndex: 1, reason: { kind: 'OFF' } }); + + errorHookRunner.withEvaluation('test-flag', { kind: 'user', key: 'user-123' }, false, method); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining( + 'An error was encountered in "beforeEvaluation" of the "Error Hook" hook: Error: Hook error', + ), + ); + }); + + it('should skip hook execution if there are no hooks', () => { + const emptyHookRunner = new HookRunner(logger, []); + const method = jest + .fn() + .mockReturnValue({ value: true, variationIndex: 1, reason: { kind: 'OFF' } }); + + emptyHookRunner.withEvaluation('test-flag', { kind: 'user', key: 'user-123' }, false, method); + + expect(method).toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); + }); + }); + + describe('when handling an identifcation', () => { + it('should execute identify hooks', () => { + const context: LDContext = { kind: 'user', key: 'user-123' }; + const timeout = 10; + const identifyResult: IdentifyResult = 'completed'; + + const identifyCallback = hookRunner.identify(context, timeout); + identifyCallback(identifyResult); + + expect(testHook.beforeIdentify).toHaveBeenCalledWith( + expect.objectContaining({ + context, + timeout, + }), + {}, + ); + + expect(testHook.afterIdentify).toHaveBeenCalledWith( + expect.objectContaining({ + context, + timeout, + }), + {}, + identifyResult, + ); + }); + + it('should handle errors in identify hooks', () => { + const errorHook: Hook = { + getMetadata: jest.fn().mockReturnValue({ name: 'Error Hook' }), + beforeIdentify: jest.fn().mockImplementation(() => { + throw new Error('Hook error'); + }), + afterIdentify: jest.fn(), + }; + + const errorHookRunner = new HookRunner(logger, [errorHook]); + + const identifyCallback = errorHookRunner.identify({ kind: 'user', key: 'user-123' }, 1000); + identifyCallback('error'); + + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining( + 'An error was encountered in "beforeEvaluation" of the "Error Hook" hook: Error: Hook error', + ), + ); + }); + }); + + it('should use the added hook in future invocations', () => { + const newHook: Hook = { + getMetadata: jest.fn().mockReturnValue({ name: 'New Hook' }), + beforeEvaluation: jest.fn(), + afterEvaluation: jest.fn(), + }; + + hookRunner.addHook(newHook); + + const method = jest + .fn() + .mockReturnValue({ value: true, variationIndex: 1, reason: { kind: 'OFF' } }); + + hookRunner.withEvaluation('test-flag', { kind: 'user', key: 'user-123' }, false, method); + + expect(newHook.beforeEvaluation).toHaveBeenCalled(); + expect(newHook.afterEvaluation).toHaveBeenCalled(); + }); + + it('should log "unknown hook" when getMetadata throws an error', () => { + const errorHook: Hook = { + getMetadata: jest.fn().mockImplementation(() => { + throw new Error('Metadata error'); + }), + beforeEvaluation: jest.fn().mockImplementation(() => { + throw new Error('Test error in beforeEvaluation'); + }), + afterEvaluation: jest.fn(), + }; + + const errorHookRunner = new HookRunner(logger, [errorHook]); + + errorHookRunner.withEvaluation('test-flag', { kind: 'user', key: 'user-123' }, false, () => ({ + value: true, + variationIndex: 1, + reason: { kind: 'OFF' }, + })); + + expect(logger.error).toHaveBeenCalledWith( + 'Exception thrown getting metadata for hook. Unable to get hook name.', + ); + + // Verify that the error was logged with the correct hook name + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining( + 'An error was encountered in "beforeEvaluation" of the "unknown hook" hook: Error: Test error in beforeEvaluation', + ), + ); + }); + + it('should log the correct hook name when an error occurs', () => { + // Modify the testHook to throw an error in beforeEvaluation + testHook.beforeEvaluation = jest.fn().mockImplementation(() => { + throw new Error('Test error in beforeEvaluation'); + }); + + hookRunner.withEvaluation('test-flag', { kind: 'user', key: 'user-123' }, false, () => ({ + value: true, + variationIndex: 1, + reason: { kind: 'OFF' }, + })); + + // Verify that getMetadata was called to get the hook name + expect(testHook.getMetadata).toHaveBeenCalled(); + + // Verify that the error was logged with the correct hook name + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining( + 'An error was encountered in "beforeEvaluation" of the "Test Hook" hook: Error: Test error in beforeEvaluation', + ), + ); + }); +}); diff --git a/packages/shared/sdk-client/src/HookRunner.ts b/packages/shared/sdk-client/src/HookRunner.ts new file mode 100644 index 0000000000..d73277cc95 --- /dev/null +++ b/packages/shared/sdk-client/src/HookRunner.ts @@ -0,0 +1,164 @@ +import { LDContext, LDLogger } from '@launchdarkly/js-sdk-common'; + +import { + EvaluationSeriesContext, + EvaluationSeriesData, + Hook, + IdentifyResult, + IdentifySeriesContext, + IdentifySeriesData, +} from './api/integrations/Hooks'; +import { LDEvaluationDetail } from './api/LDEvaluationDetail'; + +const UNKNOWN_HOOK_NAME = 'unknown hook'; +const BEFORE_EVALUATION_STAGE_NAME = 'beforeEvaluation'; +const AFTER_EVALUATION_STAGE_NAME = 'afterEvaluation'; + +function tryExecuteStage( + logger: LDLogger, + method: string, + hookName: string, + stage: () => TData, + def: TData, +): TData { + try { + return stage(); + } catch (err) { + logger?.error(`An error was encountered in "${method}" of the "${hookName}" hook: ${err}`); + return def; + } +} + +function getHookName(logger: LDLogger, hook?: Hook): string { + try { + return hook?.getMetadata().name ?? UNKNOWN_HOOK_NAME; + } catch { + logger.error(`Exception thrown getting metadata for hook. Unable to get hook name.`); + return UNKNOWN_HOOK_NAME; + } +} + +function executeBeforeEvaluation( + logger: LDLogger, + hooks: Hook[], + hookContext: EvaluationSeriesContext, +): EvaluationSeriesData[] { + return hooks.map((hook) => + tryExecuteStage( + logger, + BEFORE_EVALUATION_STAGE_NAME, + getHookName(logger, hook), + () => hook?.beforeEvaluation?.(hookContext, {}) ?? {}, + {}, + ), + ); +} + +function executeAfterEvaluation( + logger: LDLogger, + hooks: Hook[], + hookContext: EvaluationSeriesContext, + updatedData: (EvaluationSeriesData | undefined)[], + result: LDEvaluationDetail, +) { + // This iterates in reverse, versus reversing a shallow copy of the hooks, + // for efficiency. + for (let hookIndex = hooks.length - 1; hookIndex >= 0; hookIndex -= 1) { + const hook = hooks[hookIndex]; + const data = updatedData[hookIndex] ?? {}; + tryExecuteStage( + logger, + AFTER_EVALUATION_STAGE_NAME, + getHookName(logger, hook), + () => hook?.afterEvaluation?.(hookContext, data, result) ?? {}, + {}, + ); + } +} + +function executeBeforeIdentify( + logger: LDLogger, + hooks: Hook[], + hookContext: IdentifySeriesContext, +): IdentifySeriesData[] { + return hooks.map((hook) => + tryExecuteStage( + logger, + BEFORE_EVALUATION_STAGE_NAME, + getHookName(logger, hook), + () => hook?.beforeIdentify?.(hookContext, {}) ?? {}, + {}, + ), + ); +} + +function executeAfterIdentify( + logger: LDLogger, + hooks: Hook[], + hookContext: IdentifySeriesContext, + updatedData: (IdentifySeriesData | undefined)[], + result: IdentifyResult, +) { + // This iterates in reverse, versus reversing a shallow copy of the hooks, + // for efficiency. + for (let hookIndex = hooks.length - 1; hookIndex >= 0; hookIndex -= 1) { + const hook = hooks[hookIndex]; + const data = updatedData[hookIndex] ?? {}; + tryExecuteStage( + logger, + AFTER_EVALUATION_STAGE_NAME, + getHookName(logger, hook), + () => hook?.afterIdentify?.(hookContext, data, result) ?? {}, + {}, + ); + } +} + +export default class HookRunner { + private readonly hooks: Hook[] = []; + + constructor( + private readonly logger: LDLogger, + initialHooks: Hook[], + ) { + this.hooks.push(...initialHooks); + } + + withEvaluation( + key: string, + context: LDContext | undefined, + defaultValue: unknown, + method: () => LDEvaluationDetail, + ): LDEvaluationDetail { + if (this.hooks.length === 0) { + return method(); + } + const hooks: Hook[] = [...this.hooks]; + const hookContext: EvaluationSeriesContext = { + flagKey: key, + context, + defaultValue, + }; + + const hookData = executeBeforeEvaluation(this.logger, hooks, hookContext); + const result = method(); + executeAfterEvaluation(this.logger, hooks, hookContext, hookData, result); + return result; + } + + identify(context: LDContext, timeout: number | undefined): (result: IdentifyResult) => void { + const hooks: Hook[] = [...this.hooks]; + const hookContext: IdentifySeriesContext = { + context, + timeout, + }; + const hookData = executeBeforeIdentify(this.logger, hooks, hookContext); + return (result) => { + executeAfterIdentify(this.logger, hooks, hookContext, hookData, result); + }; + } + + addHook(hook: Hook): void { + this.hooks.push(hook); + } +} diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index 7802bbbcf6..24054732ad 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -16,7 +16,7 @@ import { TypeValidators, } from '@launchdarkly/js-sdk-common'; -import { LDClient, type LDOptions } from './api'; +import { Hook, LDClient, type LDOptions } from './api'; import { LDEvaluationDetail, LDEvaluationDetailTyped } from './api/LDEvaluationDetail'; import { LDIdentifyOptions } from './api/LDIdentifyOptions'; import { Configuration, ConfigurationImpl, LDClientInternalOptions } from './configuration'; @@ -31,6 +31,7 @@ import { import createEventProcessor from './events/createEventProcessor'; import EventFactory from './events/EventFactory'; import DefaultFlagManager, { FlagManager } from './flag-manager/FlagManager'; +import HookRunner from './HookRunner'; import LDEmitter, { EventName } from './LDEmitter'; const { ClientMessages, ErrorKinds } = internal; @@ -55,6 +56,7 @@ export default class LDClientImpl implements LDClient { private eventSendingEnabled: boolean = false; private baseHeaders: LDHeaders; protected dataManager: DataManager; + private hookRunner: HookRunner; /** * Creates the client object synchronously. No async, no network calls. @@ -118,6 +120,8 @@ export default class LDClientImpl implements LDClient { this.emitter, this.diagnosticsManager, ); + + this.hookRunner = new HookRunner(this.logger, this.config.hooks); } allFlags(): LDFlagSet { @@ -239,6 +243,8 @@ export default class LDClientImpl implements LDClient { ); this.logger.debug(`Identifying ${JSON.stringify(this.checkedContext)}`); + const afterIdentify = this.hookRunner.identify(context, identifyOptions?.timeout); + await this.dataManager.identify( identifyResolve, identifyReject, @@ -246,7 +252,16 @@ export default class LDClientImpl implements LDClient { identifyOptions, ); - return identifyPromise; + return identifyPromise.then( + (res) => { + afterIdentify('completed'); + return res; + }, + (e) => { + afterIdentify('error'); + throw e; + }, + ); } off(eventName: EventName, listener: Function): void { @@ -343,11 +358,18 @@ export default class LDClientImpl implements LDClient { } variation(flagKey: string, defaultValue?: LDFlagValue): LDFlagValue { - const { value } = this.variationInternal(flagKey, defaultValue, this.eventFactoryDefault); + const { value } = this.hookRunner.withEvaluation( + flagKey, + this.uncheckedContext, + defaultValue, + () => this.variationInternal(flagKey, defaultValue, this.eventFactoryDefault), + ); return value; } variationDetail(flagKey: string, defaultValue?: LDFlagValue): LDEvaluationDetail { - return this.variationInternal(flagKey, defaultValue, this.eventFactoryWithReasons); + return this.hookRunner.withEvaluation(flagKey, this.uncheckedContext, defaultValue, () => + this.variationInternal(flagKey, defaultValue, this.eventFactoryWithReasons), + ); } private typedEval( @@ -356,7 +378,9 @@ export default class LDClientImpl implements LDClient { eventFactory: EventFactory, typeChecker: (value: unknown) => [boolean, string], ): LDEvaluationDetailTyped { - return this.variationInternal(key, defaultValue, eventFactory, typeChecker); + return this.hookRunner.withEvaluation(key, this.uncheckedContext, defaultValue, () => + this.variationInternal(key, defaultValue, eventFactory, typeChecker), + ); } boolVariation(key: string, defaultValue: boolean): boolean { @@ -409,6 +433,10 @@ export default class LDClientImpl implements LDClient { return this.variationDetail(key, defaultValue); } + addHook(hook: Hook): void { + this.hookRunner.addHook(hook); + } + /** * Enable/Disable event sending. * @param enabled True to enable event processing, false to disable. diff --git a/packages/shared/sdk-client/src/api/LDClient.ts b/packages/shared/sdk-client/src/api/LDClient.ts index a89253e087..cec06b37f3 100644 --- a/packages/shared/sdk-client/src/api/LDClient.ts +++ b/packages/shared/sdk-client/src/api/LDClient.ts @@ -1,5 +1,6 @@ import { LDContext, LDFlagSet, LDFlagValue, LDLogger } from '@launchdarkly/js-sdk-common'; +import { Hook } from './integrations/Hooks'; import { LDEvaluationDetail, LDEvaluationDetailTyped } from './LDEvaluationDetail'; import { LDIdentifyOptions } from './LDIdentifyOptions'; @@ -318,4 +319,14 @@ export interface LDClient { * An {@link LDEvaluationDetail} object containing the value and explanation. */ variationDetail(key: string, defaultValue?: LDFlagValue): LDEvaluationDetail; + + /** + * Add a hook to the client. In order to register a hook before the client + * starts, please use the `hooks` property of {@link LDOptions}. + * + * Hooks provide entrypoints which allow for observation of SDK functions. + * + * @param Hook The hook to add. + */ + addHook?(hook: Hook): void; } diff --git a/packages/shared/sdk-client/src/api/LDOptions.ts b/packages/shared/sdk-client/src/api/LDOptions.ts index 5e2c115135..123bcd2182 100644 --- a/packages/shared/sdk-client/src/api/LDOptions.ts +++ b/packages/shared/sdk-client/src/api/LDOptions.ts @@ -1,4 +1,5 @@ import type { LDLogger } from '@launchdarkly/js-sdk-common'; +import { Hook } from './integrations/Hooks'; export interface LDOptions { /** @@ -236,4 +237,23 @@ export interface LDOptions { * config property. */ payloadFilterKey?: string; + + /** + * Initial set of hooks for the client. + * + * Hooks provide entrypoints which allow for observation of SDK functions. + * + * LaunchDarkly provides integration packages, and most applications will not + * need to implement their own hooks. Refer to the `@launchdarkly/node-server-sdk-otel` + * for instrumentation for the `@launchdarkly/node-server-sdk`. + * + * Example: + * ```typescript + * import { init } from '@launchdarkly/node-server-sdk'; + * import { TracingHook } from '@launchdarkly/node-server-sdk-otel'; + * + * const client = init('my-sdk-key', { hooks: [new TracingHook()] }); + * ``` + */ + hooks?: Hook[]; } diff --git a/packages/shared/sdk-client/src/api/index.ts b/packages/shared/sdk-client/src/api/index.ts index 5440396ba1..bae5cebdf7 100644 --- a/packages/shared/sdk-client/src/api/index.ts +++ b/packages/shared/sdk-client/src/api/index.ts @@ -3,6 +3,7 @@ import ConnectionMode from './ConnectionMode'; export * from './LDOptions'; export * from './LDClient'; export * from './LDEvaluationDetail'; +export * from './integrations'; export { ConnectionMode }; export * from './LDIdentifyOptions'; diff --git a/packages/shared/sdk-client/src/api/integrations/Hooks.ts b/packages/shared/sdk-client/src/api/integrations/Hooks.ts new file mode 100644 index 0000000000..e8326581dd --- /dev/null +++ b/packages/shared/sdk-client/src/api/integrations/Hooks.ts @@ -0,0 +1,155 @@ +import { LDContext } from '@launchdarkly/js-sdk-common'; + +import { LDEvaluationDetail } from '../LDEvaluationDetail'; + +/** + * Contextual information provided to evaluation stages. + */ +export interface EvaluationSeriesContext { + readonly flagKey: string; + /** + * Optional in case evaluations are performed before a context is set. + */ + readonly context?: LDContext; + readonly defaultValue: unknown; + + /** + * Implementation note: Omitting method name because of the associated size. + * If we need this functionality, then we may want to consider adding it and + * taking the associated size hit. + */ +} + +/** + * Implementation specific hook data for evaluation stages. + * + * Hook implementations can use this to store data needed between stages. + */ +export interface EvaluationSeriesData { + readonly [index: string]: unknown; +} + +/** + * Meta-data about a hook implementation. + */ +export interface HookMetadata { + readonly name: string; +} + +/** + * Contextual information provided to evaluation stages. + */ +export interface IdentifySeriesContext { + readonly context: LDContext; + /** + * The timeout, in seconds, associated with the identify operation. + */ + readonly timeout?: number; +} + +/** + * Implementation specific hook data for evaluation stages. + * + * Hook implementations can use this to store data needed between stages. + */ +export interface IdentifySeriesData { + readonly [index: string]: unknown; +} + +/** + * The result applies to a single identify operation. An operation may complete + * with an error and then later complete successfully. Only the first completion + * will be executed in the evaluation series. + */ +export type IdentifyResult = 'completed' | 'error'; + +/** + * Interface for extending SDK functionality via hooks. + */ +export interface Hook { + /** + * Get metadata about the hook implementation. + */ + getMetadata(): HookMetadata; + + /** + * This method is called during the execution of a variation method + * before the flag value has been determined. The method is executed synchronously. + * + * @param hookContext Contains information about the evaluation being performed. This is not + * mutable. + * @param data A record associated with each stage of hook invocations. Each stage is called with + * the data of the previous stage for a series. The input record should not be modified. + * @returns Data to use when executing the next state of the hook in the evaluation series. It is + * recommended to expand the previous input into the return. This helps ensure your stage remains + * compatible moving forward as more stages are added. + * ```js + * return {...data, "my-new-field": /*my data/*} + * ``` + */ + beforeEvaluation?( + hookContext: EvaluationSeriesContext, + data: EvaluationSeriesData, + ): EvaluationSeriesData; + + /** + * This method is called during the execution of the variation method + * after the flag value has been determined. The method is executed synchronously. + * + * @param hookContext Contains read-only information about the evaluation + * being performed. + * @param data A record associated with each stage of hook invocations. Each + * stage is called with the data of the previous stage for a series. + * @param detail The result of the evaluation. This value should not be + * modified. + * @returns Data to use when executing the next state of the hook in the evaluation series. It is + * recommended to expand the previous input into the return. This helps ensure your stage remains + * compatible moving forward as more stages are added. + * ```js + * return {...data, "my-new-field": /*my data/*} + * ``` + */ + afterEvaluation?( + hookContext: EvaluationSeriesContext, + data: EvaluationSeriesData, + detail: LDEvaluationDetail, + ): EvaluationSeriesData; + + /** + * This method is called during the execution of the identify process before the operation + * completes, but after any context modifications are performed. + * + * @param hookContext Contains information about the evaluation being performed. This is not + * mutable. + * @param data A record associated with each stage of hook invocations. Each stage is called with + * the data of the previous stage for a series. The input record should not be modified. + * @returns Data to use when executing the next state of the hook in the evaluation series. It is + * recommended to expand the previous input into the return. This helps ensure your stage remains + * compatible moving forward as more stages are added. + * ```js + * return {...data, "my-new-field": /*my data/*} + * ``` + */ + beforeIdentify?(hookContext: IdentifySeriesContext, data: IdentifySeriesData): IdentifySeriesData; + + /** + * This method is called during the execution of the identify process before the operation + * completes, but after any context modifications are performed. + * + * @param hookContext Contains information about the evaluation being performed. This is not + * mutable. + * @param data A record associated with each stage of hook invocations. Each stage is called with + * the data of the previous stage for a series. The input record should not be modified. + * @returns Data to use when executing the next state of the hook in the evaluation series. It is + * recommended to expand the previous input into the return. This helps ensure your stage remains + * compatible moving forward as more stages are added. + * ```js + * return {...data, "my-new-field": /*my data/*} + * ``` + */ + afterIdentify?( + hookContext: IdentifySeriesContext, + data: IdentifySeriesData, + result: IdentifyResult, + ): IdentifySeriesData; +} diff --git a/packages/shared/sdk-client/src/api/integrations/index.ts b/packages/shared/sdk-client/src/api/integrations/index.ts new file mode 100644 index 0000000000..c2df1c4b06 --- /dev/null +++ b/packages/shared/sdk-client/src/api/integrations/index.ts @@ -0,0 +1 @@ +export * from './Hooks'; diff --git a/packages/shared/sdk-client/src/configuration/Configuration.ts b/packages/shared/sdk-client/src/configuration/Configuration.ts index 50ce86f3ff..47f6e15394 100644 --- a/packages/shared/sdk-client/src/configuration/Configuration.ts +++ b/packages/shared/sdk-client/src/configuration/Configuration.ts @@ -11,7 +11,7 @@ import { TypeValidators, } from '@launchdarkly/js-sdk-common'; -import { type LDOptions } from '../api'; +import { Hook, type LDOptions } from '../api'; import validators from './validators'; const DEFAULT_POLLING_INTERVAL: number = 60 * 5; @@ -52,6 +52,7 @@ export interface Configuration { readonly pollInterval: number; readonly userAgentHeaderName: 'user-agent' | 'x-launchdarkly-user-agent'; readonly trackEventModifier: (event: internal.InputCustomEvent) => internal.InputCustomEvent; + readonly hooks: Hook[]; } const DEFAULT_POLLING: string = 'https://clientsdk.launchdarkly.com'; @@ -114,6 +115,8 @@ export default class ConfigurationImpl implements Configuration { public readonly userAgentHeaderName: 'user-agent' | 'x-launchdarkly-user-agent'; + public readonly hooks: Hook[] = []; + public readonly trackEventModifier: ( event: internal.InputCustomEvent, ) => internal.InputCustomEvent; diff --git a/packages/shared/sdk-client/src/configuration/validators.ts b/packages/shared/sdk-client/src/configuration/validators.ts index 4ec0a449a3..be637d90dc 100644 --- a/packages/shared/sdk-client/src/configuration/validators.ts +++ b/packages/shared/sdk-client/src/configuration/validators.ts @@ -32,6 +32,7 @@ const validators: Record = { wrapperName: TypeValidators.String, wrapperVersion: TypeValidators.String, payloadFilterKey: TypeValidators.stringMatchingRegex(/^[a-zA-Z0-9](\w|\.|-)*$/), + hooks: TypeValidators.createTypeArray('Hook[]', {}), }; export default validators; From 10603d05a95897f0195bf80d31e4320703975af3 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 2 Oct 2024 15:29:07 -0700 Subject: [PATCH 2/8] Extend test coverage --- .../sdk-client/__tests__/HookRunner.test.ts | 27 +++++++++++++++++++ packages/shared/sdk-client/src/HookRunner.ts | 12 ++++----- .../shared/sdk-client/src/api/LDOptions.ts | 1 + 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/packages/shared/sdk-client/__tests__/HookRunner.test.ts b/packages/shared/sdk-client/__tests__/HookRunner.test.ts index 60ce7b7a12..bb8019bce4 100644 --- a/packages/shared/sdk-client/__tests__/HookRunner.test.ts +++ b/packages/shared/sdk-client/__tests__/HookRunner.test.ts @@ -202,6 +202,33 @@ describe('given a hook runner and test hook', () => { ); }); + it('should log "unknown hook" when getMetadata returns an empty name', () => { + const errorHook: Hook = { + getMetadata: jest.fn().mockImplementation(() => ({ + name: '', + })), + beforeEvaluation: jest.fn().mockImplementation(() => { + throw new Error('Test error in beforeEvaluation'); + }), + afterEvaluation: jest.fn(), + }; + + const errorHookRunner = new HookRunner(logger, [errorHook]); + + errorHookRunner.withEvaluation('test-flag', { kind: 'user', key: 'user-123' }, false, () => ({ + value: true, + variationIndex: 1, + reason: { kind: 'OFF' }, + })); + + // Verify that the error was logged with the correct hook name + expect(logger.error).toHaveBeenCalledWith( + expect.stringContaining( + 'An error was encountered in "beforeEvaluation" of the "unknown hook" hook: Error: Test error in beforeEvaluation', + ), + ); + }); + it('should log the correct hook name when an error occurs', () => { // Modify the testHook to throw an error in beforeEvaluation testHook.beforeEvaluation = jest.fn().mockImplementation(() => { diff --git a/packages/shared/sdk-client/src/HookRunner.ts b/packages/shared/sdk-client/src/HookRunner.ts index d73277cc95..f7ea768915 100644 --- a/packages/shared/sdk-client/src/HookRunner.ts +++ b/packages/shared/sdk-client/src/HookRunner.ts @@ -29,9 +29,9 @@ function tryExecuteStage( } } -function getHookName(logger: LDLogger, hook?: Hook): string { +function getHookName(logger: LDLogger, hook: Hook): string { try { - return hook?.getMetadata().name ?? UNKNOWN_HOOK_NAME; + return hook.getMetadata().name || UNKNOWN_HOOK_NAME; } catch { logger.error(`Exception thrown getting metadata for hook. Unable to get hook name.`); return UNKNOWN_HOOK_NAME; @@ -58,14 +58,14 @@ function executeAfterEvaluation( logger: LDLogger, hooks: Hook[], hookContext: EvaluationSeriesContext, - updatedData: (EvaluationSeriesData | undefined)[], + updatedData: EvaluationSeriesData[], result: LDEvaluationDetail, ) { // This iterates in reverse, versus reversing a shallow copy of the hooks, // for efficiency. for (let hookIndex = hooks.length - 1; hookIndex >= 0; hookIndex -= 1) { const hook = hooks[hookIndex]; - const data = updatedData[hookIndex] ?? {}; + const data = updatedData[hookIndex]; tryExecuteStage( logger, AFTER_EVALUATION_STAGE_NAME, @@ -96,14 +96,14 @@ function executeAfterIdentify( logger: LDLogger, hooks: Hook[], hookContext: IdentifySeriesContext, - updatedData: (IdentifySeriesData | undefined)[], + updatedData: IdentifySeriesData[], result: IdentifyResult, ) { // This iterates in reverse, versus reversing a shallow copy of the hooks, // for efficiency. for (let hookIndex = hooks.length - 1; hookIndex >= 0; hookIndex -= 1) { const hook = hooks[hookIndex]; - const data = updatedData[hookIndex] ?? {}; + const data = updatedData[hookIndex]; tryExecuteStage( logger, AFTER_EVALUATION_STAGE_NAME, diff --git a/packages/shared/sdk-client/src/api/LDOptions.ts b/packages/shared/sdk-client/src/api/LDOptions.ts index 123bcd2182..c88c608ebc 100644 --- a/packages/shared/sdk-client/src/api/LDOptions.ts +++ b/packages/shared/sdk-client/src/api/LDOptions.ts @@ -1,4 +1,5 @@ import type { LDLogger } from '@launchdarkly/js-sdk-common'; + import { Hook } from './integrations/Hooks'; export interface LDOptions { From cdba45131975c4704eb5a07280a4bb337b291602 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 2 Oct 2024 15:31:37 -0700 Subject: [PATCH 3/8] Fix browser tests. --- packages/sdk/browser/__tests__/BrowserDataManager.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/sdk/browser/__tests__/BrowserDataManager.test.ts b/packages/sdk/browser/__tests__/BrowserDataManager.test.ts index c863de92d1..5c9e072c0c 100644 --- a/packages/sdk/browser/__tests__/BrowserDataManager.test.ts +++ b/packages/sdk/browser/__tests__/BrowserDataManager.test.ts @@ -92,6 +92,7 @@ describe('given a BrowserDataManager with mocked dependencies', () => { pollInterval: 1000, userAgentHeaderName: 'user-agent', trackEventModifier: (event) => event, + hooks: [], }; const mockedFetch = mockFetch('{"flagA": true}', 200); platform = { From 93b1ba63a397e9c6d078ece9917bed55599fe08c Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 2 Oct 2024 15:32:54 -0700 Subject: [PATCH 4/8] Fix react native tests. --- packages/sdk/react-native/__tests__/MobileDataManager.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/sdk/react-native/__tests__/MobileDataManager.test.ts b/packages/sdk/react-native/__tests__/MobileDataManager.test.ts index a7e0447093..5c651cb3c4 100644 --- a/packages/sdk/react-native/__tests__/MobileDataManager.test.ts +++ b/packages/sdk/react-native/__tests__/MobileDataManager.test.ts @@ -81,6 +81,7 @@ describe('given a MobileDataManager with mocked dependencies', () => { pollInterval: 1000, userAgentHeaderName: 'user-agent', trackEventModifier: (event) => event, + hooks: [], }; const mockedFetch = mockFetch('{"flagA": true}', 200); platform = { From 19585cb75319f144ba8c4b34ed432eb9163b4f17 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 2 Oct 2024 16:09:56 -0700 Subject: [PATCH 5/8] Additional tests. --- .../sdk-client/__tests__/HookRunner.test.ts | 52 +++- .../__tests__/LDClientImpl.hooks.test.ts | 223 ++++++++++++++++++ .../sdk-client/__tests__/TestDataManager.ts | 38 ++- 3 files changed, 311 insertions(+), 2 deletions(-) create mode 100644 packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts diff --git a/packages/shared/sdk-client/__tests__/HookRunner.test.ts b/packages/shared/sdk-client/__tests__/HookRunner.test.ts index bb8019bce4..d6c136b965 100644 --- a/packages/shared/sdk-client/__tests__/HookRunner.test.ts +++ b/packages/shared/sdk-client/__tests__/HookRunner.test.ts @@ -101,9 +101,37 @@ describe('given a hook runner and test hook', () => { expect(method).toHaveBeenCalled(); expect(logger.error).not.toHaveBeenCalled(); }); + + it('should pass evaluation series data from before to after hooks', () => { + const key = 'test-flag'; + const context: LDContext = { kind: 'user', key: 'user-123' }; + const defaultValue = false; + const evaluationResult: LDEvaluationDetail = { + value: true, + variationIndex: 1, + reason: { kind: 'OFF' }, + }; + + testHook.beforeEvaluation = jest + .fn() + .mockImplementation((_, series) => ({ ...series, testData: 'before data' })); + + testHook.afterEvaluation = jest.fn(); + + const method = jest.fn().mockReturnValue(evaluationResult); + + hookRunner.withEvaluation(key, context, defaultValue, method); + + expect(testHook.beforeEvaluation).toHaveBeenCalled(); + expect(testHook.afterEvaluation).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ testData: 'before data' }), + evaluationResult, + ); + }); }); - describe('when handling an identifcation', () => { + describe('when handling an identification', () => { it('should execute identify hooks', () => { const context: LDContext = { kind: 'user', key: 'user-123' }; const timeout = 10; @@ -150,6 +178,28 @@ describe('given a hook runner and test hook', () => { ), ); }); + + it('should pass identify series data from before to after hooks', () => { + const context: LDContext = { kind: 'user', key: 'user-123' }; + const timeout = 10; + const identifyResult: IdentifyResult = 'completed'; + + testHook.beforeIdentify = jest + .fn() + .mockImplementation((_, series) => ({ ...series, testData: 'before identify data' })); + + testHook.afterIdentify = jest.fn(); + + const identifyCallback = hookRunner.identify(context, timeout); + identifyCallback(identifyResult); + + expect(testHook.beforeIdentify).toHaveBeenCalled(); + expect(testHook.afterIdentify).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining({ testData: 'before identify data' }), + identifyResult, + ); + }); }); it('should use the added hook in future invocations', () => { diff --git a/packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts b/packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts new file mode 100644 index 0000000000..bc1df61f9e --- /dev/null +++ b/packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts @@ -0,0 +1,223 @@ +import { AutoEnvAttributes } from '@launchdarkly/js-sdk-common'; + +import { Hook, HookMetadata } from '../src/api'; +import LDClientImpl from '../src/LDClientImpl'; +import { createBasicPlatform } from './createBasicPlatform'; +import { makeTestDataManagerFactory } from './TestDataManager'; + +it('should use hooks registered during configuration', async () => { + const testHook: Hook = { + beforeEvaluation: jest.fn(), + afterEvaluation: jest.fn(), + beforeIdentify: jest.fn(), + afterIdentify: jest.fn(), + getMetadata(): HookMetadata { + return { + name: 'test hook', + }; + }, + }; + + const platform = createBasicPlatform(); + const factory = makeTestDataManagerFactory('sdk-key', platform, { + disableNetwork: true, + }); + const client = new LDClientImpl( + 'sdk-key', + AutoEnvAttributes.Disabled, + platform, + { + sendEvents: false, + hooks: [testHook], + }, + factory, + ); + + await client.identify({ key: 'user-key' }); + await client.variation('flag-key', false); + + expect(testHook.beforeIdentify).toHaveBeenCalledWith( + { context: { key: 'user-key' }, timeout: undefined }, + {}, + ); + expect(testHook.afterIdentify).toHaveBeenCalledWith( + { context: { key: 'user-key' }, timeout: undefined }, + {}, + 'completed', + ); + expect(testHook.beforeEvaluation).toHaveBeenCalledWith( + { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, + {}, + ); + expect(testHook.afterEvaluation).toHaveBeenCalledWith( + { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, + {}, + { + reason: { + errorKind: 'FLAG_NOT_FOUND', + kind: 'ERROR', + }, + value: false, + variationIndex: null, + }, + ); +}); + +it('should execute hooks that are added using addHook', async () => { + const platform = createBasicPlatform(); + const factory = makeTestDataManagerFactory('sdk-key', platform, { + disableNetwork: true, + }); + const client = new LDClientImpl( + 'sdk-key', + AutoEnvAttributes.Disabled, + platform, + { + sendEvents: false, + }, + factory, + ); + + const addedHook: Hook = { + beforeEvaluation: jest.fn(), + afterEvaluation: jest.fn(), + beforeIdentify: jest.fn(), + afterIdentify: jest.fn(), + getMetadata(): HookMetadata { + return { + name: 'added hook', + }; + }, + }; + + client.addHook(addedHook); + + await client.identify({ key: 'user-key' }); + await client.variation('flag-key', false); + + expect(addedHook.beforeIdentify).toHaveBeenCalledWith( + { context: { key: 'user-key' }, timeout: undefined }, + {}, + ); + expect(addedHook.afterIdentify).toHaveBeenCalledWith( + { context: { key: 'user-key' }, timeout: undefined }, + {}, + 'completed', + ); + expect(addedHook.beforeEvaluation).toHaveBeenCalledWith( + { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, + {}, + ); + expect(addedHook.afterEvaluation).toHaveBeenCalledWith( + { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, + {}, + { + reason: { + errorKind: 'FLAG_NOT_FOUND', + kind: 'ERROR', + }, + value: false, + variationIndex: null, + }, + ); +}); + +it('should execute both initial hooks and hooks added using addHook', async () => { + const initialHook: Hook = { + beforeEvaluation: jest.fn(), + afterEvaluation: jest.fn(), + beforeIdentify: jest.fn(), + afterIdentify: jest.fn(), + getMetadata(): HookMetadata { + return { + name: 'initial hook', + }; + }, + }; + + const platform = createBasicPlatform(); + const factory = makeTestDataManagerFactory('sdk-key', platform, { + disableNetwork: true, + }); + const client = new LDClientImpl( + 'sdk-key', + AutoEnvAttributes.Disabled, + platform, + { + sendEvents: false, + hooks: [initialHook], + }, + factory, + ); + + const addedHook: Hook = { + beforeEvaluation: jest.fn(), + afterEvaluation: jest.fn(), + beforeIdentify: jest.fn(), + afterIdentify: jest.fn(), + getMetadata(): HookMetadata { + return { + name: 'added hook', + }; + }, + }; + + client.addHook(addedHook); + + await client.identify({ key: 'user-key' }); + await client.variation('flag-key', false); + + // Check initial hook + expect(initialHook.beforeIdentify).toHaveBeenCalledWith( + { context: { key: 'user-key' }, timeout: undefined }, + {}, + ); + expect(initialHook.afterIdentify).toHaveBeenCalledWith( + { context: { key: 'user-key' }, timeout: undefined }, + {}, + 'completed', + ); + expect(initialHook.beforeEvaluation).toHaveBeenCalledWith( + { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, + {}, + ); + expect(initialHook.afterEvaluation).toHaveBeenCalledWith( + { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, + {}, + { + reason: { + errorKind: 'FLAG_NOT_FOUND', + kind: 'ERROR', + }, + value: false, + variationIndex: null, + }, + ); + + // Check added hook + expect(addedHook.beforeIdentify).toHaveBeenCalledWith( + { context: { key: 'user-key' }, timeout: undefined }, + {}, + ); + expect(addedHook.afterIdentify).toHaveBeenCalledWith( + { context: { key: 'user-key' }, timeout: undefined }, + {}, + 'completed', + ); + expect(addedHook.beforeEvaluation).toHaveBeenCalledWith( + { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, + {}, + ); + expect(addedHook.afterEvaluation).toHaveBeenCalledWith( + { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, + {}, + { + reason: { + errorKind: 'FLAG_NOT_FOUND', + kind: 'ERROR', + }, + value: false, + variationIndex: null, + }, + ); +}); diff --git a/packages/shared/sdk-client/__tests__/TestDataManager.ts b/packages/shared/sdk-client/__tests__/TestDataManager.ts index 7976a1329a..c7e35096df 100644 --- a/packages/shared/sdk-client/__tests__/TestDataManager.ts +++ b/packages/shared/sdk-client/__tests__/TestDataManager.ts @@ -12,8 +12,33 @@ import { Configuration } from '../src/configuration/Configuration'; import { BaseDataManager, DataManagerFactory } from '../src/DataManager'; import { FlagManager } from '../src/flag-manager/FlagManager'; import LDEmitter from '../src/LDEmitter'; +import { DataSourcePaths } from '../src/streaming/DataSourceConfig'; export default class TestDataManager extends BaseDataManager { + constructor( + platform: Platform, + flagManager: FlagManager, + credential: string, + config: Configuration, + getPollingPaths: () => DataSourcePaths, + getStreamingPaths: () => DataSourcePaths, + baseHeaders: LDHeaders, + emitter: LDEmitter, + private readonly disableNetwork: boolean, + diagnosticsManager?: internal.DiagnosticsManager, + ) { + super( + platform, + flagManager, + credential, + config, + getPollingPaths, + getStreamingPaths, + baseHeaders, + emitter, + diagnosticsManager, + ); + } override async identify( identifyResolve: () => void, identifyReject: (err: Error) => void, @@ -33,6 +58,10 @@ export default class TestDataManager extends BaseDataManager { 'Identify - Flags loaded from cache, but identify was requested with "waitForNetworkResults"', ); } + if (this.disableNetwork) { + identifyResolve(); + return; + } this.setupConnection(context, identifyResolve, identifyReject); } @@ -52,7 +81,13 @@ export default class TestDataManager extends BaseDataManager { } } -export function makeTestDataManagerFactory(sdkKey: string, platform: Platform): DataManagerFactory { +export function makeTestDataManagerFactory( + sdkKey: string, + platform: Platform, + options: { + disableNetwork?: boolean; + }, +): DataManagerFactory { return ( flagManager: FlagManager, configuration: Configuration, @@ -83,6 +118,7 @@ export function makeTestDataManagerFactory(sdkKey: string, platform: Platform): }), baseHeaders, emitter, + !!options.disableNetwork, diagnosticsManager, ); } From 4a15070f7aa0900bf598d04d331548c05d69e964 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 3 Oct 2024 08:42:10 -0700 Subject: [PATCH 6/8] Fix sdk-client tests. --- .../sdk-client/__tests__/LDClientImpl.hooks.test.ts | 12 ++++++++++++ .../shared/sdk-client/__tests__/TestDataManager.ts | 4 ++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts b/packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts index bc1df61f9e..c0abb1477c 100644 --- a/packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts +++ b/packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts @@ -29,6 +29,12 @@ it('should use hooks registered during configuration', async () => { { sendEvents: false, hooks: [testHook], + logger: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, }, factory, ); @@ -74,6 +80,12 @@ it('should execute hooks that are added using addHook', async () => { platform, { sendEvents: false, + logger: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, }, factory, ); diff --git a/packages/shared/sdk-client/__tests__/TestDataManager.ts b/packages/shared/sdk-client/__tests__/TestDataManager.ts index c7e35096df..45b526a179 100644 --- a/packages/shared/sdk-client/__tests__/TestDataManager.ts +++ b/packages/shared/sdk-client/__tests__/TestDataManager.ts @@ -84,7 +84,7 @@ export default class TestDataManager extends BaseDataManager { export function makeTestDataManagerFactory( sdkKey: string, platform: Platform, - options: { + options?: { disableNetwork?: boolean; }, ): DataManagerFactory { @@ -118,7 +118,7 @@ export function makeTestDataManagerFactory( }), baseHeaders, emitter, - !!options.disableNetwork, + !!options?.disableNetwork, diagnosticsManager, ); } From c224cd6b8434ba31a7c080b3e6c82479504b88d6 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 3 Oct 2024 14:57:25 -0700 Subject: [PATCH 7/8] PR Feedback. --- .../shared/sdk-client/__tests__/HookRunner.test.ts | 6 +++--- .../__tests__/LDClientImpl.hooks.test.ts | 14 ++++++++++---- packages/shared/sdk-client/src/LDClientImpl.ts | 4 ++-- .../sdk-client/src/api/integrations/Hooks.ts | 9 ++++++++- 4 files changed, 23 insertions(+), 10 deletions(-) diff --git a/packages/shared/sdk-client/__tests__/HookRunner.test.ts b/packages/shared/sdk-client/__tests__/HookRunner.test.ts index d6c136b965..4f6a1709bc 100644 --- a/packages/shared/sdk-client/__tests__/HookRunner.test.ts +++ b/packages/shared/sdk-client/__tests__/HookRunner.test.ts @@ -135,7 +135,7 @@ describe('given a hook runner and test hook', () => { it('should execute identify hooks', () => { const context: LDContext = { kind: 'user', key: 'user-123' }; const timeout = 10; - const identifyResult: IdentifyResult = 'completed'; + const identifyResult: IdentifyResult = { status: 'completed' }; const identifyCallback = hookRunner.identify(context, timeout); identifyCallback(identifyResult); @@ -170,7 +170,7 @@ describe('given a hook runner and test hook', () => { const errorHookRunner = new HookRunner(logger, [errorHook]); const identifyCallback = errorHookRunner.identify({ kind: 'user', key: 'user-123' }, 1000); - identifyCallback('error'); + identifyCallback({ status: 'error' }); expect(logger.error).toHaveBeenCalledWith( expect.stringContaining( @@ -182,7 +182,7 @@ describe('given a hook runner and test hook', () => { it('should pass identify series data from before to after hooks', () => { const context: LDContext = { kind: 'user', key: 'user-123' }; const timeout = 10; - const identifyResult: IdentifyResult = 'completed'; + const identifyResult: IdentifyResult = { status: 'completed' }; testHook.beforeIdentify = jest .fn() diff --git a/packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts b/packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts index c0abb1477c..9697d4179c 100644 --- a/packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts +++ b/packages/shared/sdk-client/__tests__/LDClientImpl.hooks.test.ts @@ -49,7 +49,7 @@ it('should use hooks registered during configuration', async () => { expect(testHook.afterIdentify).toHaveBeenCalledWith( { context: { key: 'user-key' }, timeout: undefined }, {}, - 'completed', + { status: 'completed' }, ); expect(testHook.beforeEvaluation).toHaveBeenCalledWith( { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, @@ -114,7 +114,7 @@ it('should execute hooks that are added using addHook', async () => { expect(addedHook.afterIdentify).toHaveBeenCalledWith( { context: { key: 'user-key' }, timeout: undefined }, {}, - 'completed', + { status: 'completed' }, ); expect(addedHook.beforeEvaluation).toHaveBeenCalledWith( { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, @@ -158,6 +158,12 @@ it('should execute both initial hooks and hooks added using addHook', async () = { sendEvents: false, hooks: [initialHook], + logger: { + debug: jest.fn(), + info: jest.fn(), + warn: jest.fn(), + error: jest.fn(), + }, }, factory, ); @@ -187,7 +193,7 @@ it('should execute both initial hooks and hooks added using addHook', async () = expect(initialHook.afterIdentify).toHaveBeenCalledWith( { context: { key: 'user-key' }, timeout: undefined }, {}, - 'completed', + { status: 'completed' }, ); expect(initialHook.beforeEvaluation).toHaveBeenCalledWith( { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, @@ -214,7 +220,7 @@ it('should execute both initial hooks and hooks added using addHook', async () = expect(addedHook.afterIdentify).toHaveBeenCalledWith( { context: { key: 'user-key' }, timeout: undefined }, {}, - 'completed', + { status: 'completed' }, ); expect(addedHook.beforeEvaluation).toHaveBeenCalledWith( { context: { key: 'user-key' }, defaultValue: false, flagKey: 'flag-key' }, diff --git a/packages/shared/sdk-client/src/LDClientImpl.ts b/packages/shared/sdk-client/src/LDClientImpl.ts index 24054732ad..b80cef3806 100644 --- a/packages/shared/sdk-client/src/LDClientImpl.ts +++ b/packages/shared/sdk-client/src/LDClientImpl.ts @@ -254,11 +254,11 @@ export default class LDClientImpl implements LDClient { return identifyPromise.then( (res) => { - afterIdentify('completed'); + afterIdentify({ status: 'completed' }); return res; }, (e) => { - afterIdentify('error'); + afterIdentify({ status: 'error' }); throw e; }, ); diff --git a/packages/shared/sdk-client/src/api/integrations/Hooks.ts b/packages/shared/sdk-client/src/api/integrations/Hooks.ts index e8326581dd..1c195c0856 100644 --- a/packages/shared/sdk-client/src/api/integrations/Hooks.ts +++ b/packages/shared/sdk-client/src/api/integrations/Hooks.ts @@ -56,12 +56,19 @@ export interface IdentifySeriesData { readonly [index: string]: unknown; } +/** + * The status an identify operation completed with. + */ +export type IdentifyStatus = 'completed' | 'error'; + /** * The result applies to a single identify operation. An operation may complete * with an error and then later complete successfully. Only the first completion * will be executed in the evaluation series. */ -export type IdentifyResult = 'completed' | 'error'; +export interface IdentifyResult { + status: IdentifyStatus; +} /** * Interface for extending SDK functionality via hooks. From 62b14f8a61384778112d6c4030ec46c3da269e5c Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Fri, 4 Oct 2024 14:00:19 -0700 Subject: [PATCH 8/8] Minor changes from testing. --- packages/sdk/browser/src/index.ts | 18 +++++++++++++++++- .../sdk-client/__tests__/HookRunner.test.ts | 6 +++--- packages/shared/sdk-client/src/HookRunner.ts | 9 ++++++--- .../sdk-client/src/api/integrations/Hooks.ts | 8 ++++---- packages/shared/sdk-client/src/index.ts | 8 ++++++++ 5 files changed, 38 insertions(+), 11 deletions(-) diff --git a/packages/sdk/browser/src/index.ts b/packages/sdk/browser/src/index.ts index d171664d68..b90c3a2eb5 100644 --- a/packages/sdk/browser/src/index.ts +++ b/packages/sdk/browser/src/index.ts @@ -1,5 +1,13 @@ import { AutoEnvAttributes, + EvaluationSeriesContext, + EvaluationSeriesData, + Hook, + HookMetadata, + IdentifySeriesContext, + IdentifySeriesData, + IdentifySeriesResult, + IdentifySeriesStatus, LDContext, LDContextCommon, LDContextMeta, @@ -19,7 +27,7 @@ import { BrowserClient, LDClient } from './BrowserClient'; import { BrowserIdentifyOptions as LDIdentifyOptions } from './BrowserIdentifyOptions'; import { BrowserOptions as LDOptions } from './options'; -export { +export type { LDClient, LDFlagSet, LDContext, @@ -34,6 +42,14 @@ export { LDEvaluationDetailTyped, LDEvaluationReason, LDIdentifyOptions, + Hook, + HookMetadata, + EvaluationSeriesContext, + EvaluationSeriesData, + IdentifySeriesContext, + IdentifySeriesData, + IdentifySeriesResult, + IdentifySeriesStatus, }; export function init(clientSideId: string, options?: LDOptions): LDClient { diff --git a/packages/shared/sdk-client/__tests__/HookRunner.test.ts b/packages/shared/sdk-client/__tests__/HookRunner.test.ts index 4f6a1709bc..85d704df49 100644 --- a/packages/shared/sdk-client/__tests__/HookRunner.test.ts +++ b/packages/shared/sdk-client/__tests__/HookRunner.test.ts @@ -1,6 +1,6 @@ import { LDContext, LDEvaluationDetail, LDLogger } from '@launchdarkly/js-sdk-common'; -import { Hook, IdentifyResult } from '../src/api/integrations/Hooks'; +import { Hook, IdentifySeriesResult } from '../src/api/integrations/Hooks'; import HookRunner from '../src/HookRunner'; describe('given a hook runner and test hook', () => { @@ -135,7 +135,7 @@ describe('given a hook runner and test hook', () => { it('should execute identify hooks', () => { const context: LDContext = { kind: 'user', key: 'user-123' }; const timeout = 10; - const identifyResult: IdentifyResult = { status: 'completed' }; + const identifyResult: IdentifySeriesResult = { status: 'completed' }; const identifyCallback = hookRunner.identify(context, timeout); identifyCallback(identifyResult); @@ -182,7 +182,7 @@ describe('given a hook runner and test hook', () => { it('should pass identify series data from before to after hooks', () => { const context: LDContext = { kind: 'user', key: 'user-123' }; const timeout = 10; - const identifyResult: IdentifyResult = { status: 'completed' }; + const identifyResult: IdentifySeriesResult = { status: 'completed' }; testHook.beforeIdentify = jest .fn() diff --git a/packages/shared/sdk-client/src/HookRunner.ts b/packages/shared/sdk-client/src/HookRunner.ts index f7ea768915..f2f6703426 100644 --- a/packages/shared/sdk-client/src/HookRunner.ts +++ b/packages/shared/sdk-client/src/HookRunner.ts @@ -4,9 +4,9 @@ import { EvaluationSeriesContext, EvaluationSeriesData, Hook, - IdentifyResult, IdentifySeriesContext, IdentifySeriesData, + IdentifySeriesResult, } from './api/integrations/Hooks'; import { LDEvaluationDetail } from './api/LDEvaluationDetail'; @@ -97,7 +97,7 @@ function executeAfterIdentify( hooks: Hook[], hookContext: IdentifySeriesContext, updatedData: IdentifySeriesData[], - result: IdentifyResult, + result: IdentifySeriesResult, ) { // This iterates in reverse, versus reversing a shallow copy of the hooks, // for efficiency. @@ -146,7 +146,10 @@ export default class HookRunner { return result; } - identify(context: LDContext, timeout: number | undefined): (result: IdentifyResult) => void { + identify( + context: LDContext, + timeout: number | undefined, + ): (result: IdentifySeriesResult) => void { const hooks: Hook[] = [...this.hooks]; const hookContext: IdentifySeriesContext = { context, diff --git a/packages/shared/sdk-client/src/api/integrations/Hooks.ts b/packages/shared/sdk-client/src/api/integrations/Hooks.ts index 1c195c0856..deb1552c20 100644 --- a/packages/shared/sdk-client/src/api/integrations/Hooks.ts +++ b/packages/shared/sdk-client/src/api/integrations/Hooks.ts @@ -59,15 +59,15 @@ export interface IdentifySeriesData { /** * The status an identify operation completed with. */ -export type IdentifyStatus = 'completed' | 'error'; +export type IdentifySeriesStatus = 'completed' | 'error'; /** * The result applies to a single identify operation. An operation may complete * with an error and then later complete successfully. Only the first completion * will be executed in the evaluation series. */ -export interface IdentifyResult { - status: IdentifyStatus; +export interface IdentifySeriesResult { + status: IdentifySeriesStatus; } /** @@ -157,6 +157,6 @@ export interface Hook { afterIdentify?( hookContext: IdentifySeriesContext, data: IdentifySeriesData, - result: IdentifyResult, + result: IdentifySeriesResult, ): IdentifySeriesData; } diff --git a/packages/shared/sdk-client/src/index.ts b/packages/shared/sdk-client/src/index.ts index cae0e42068..7073c9675a 100644 --- a/packages/shared/sdk-client/src/index.ts +++ b/packages/shared/sdk-client/src/index.ts @@ -20,6 +20,14 @@ export type { LDOptions, ConnectionMode, LDIdentifyOptions, + Hook, + HookMetadata, + EvaluationSeriesContext, + EvaluationSeriesData, + IdentifySeriesContext, + IdentifySeriesData, + IdentifySeriesResult, + IdentifySeriesStatus, } from './api'; export type { DataManager, DataManagerFactory, ConnectionParams } from './DataManager';