From 9c4589b5332e2f8cfd5ee6642ac17b1775f8c012 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 3 Apr 2025 14:38:21 -0700 Subject: [PATCH 1/9] feat: Add hook support. --- package.json | 2 +- src/HookRunner.js | 119 ++++++++++++++++++++++++++++++++++++++++++++++ src/index.js | 23 ++++++++- typings.d.ts | 108 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 249 insertions(+), 3 deletions(-) create mode 100644 src/HookRunner.js diff --git a/package.json b/package.json index d236b19..72493c4 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "launchdarkly-js-sdk-common", - "version": "5.4.0", + "version": "5.5.0-beta.1", "description": "LaunchDarkly SDK for JavaScript - common code", "author": "LaunchDarkly ", "license": "Apache-2.0", diff --git a/src/HookRunner.js b/src/HookRunner.js new file mode 100644 index 0000000..59582f6 --- /dev/null +++ b/src/HookRunner.js @@ -0,0 +1,119 @@ +const UNKNOWN_HOOK_NAME = 'unknown hook'; +const BEFORE_EVALUATION_STAGE_NAME = 'beforeEvaluation'; +const AFTER_EVALUATION_STAGE_NAME = 'afterEvaluation'; + +function tryExecuteStage(logger, method, hookName, stage, def) { + try { + return stage(); + } catch (err) { + logger?.error(`An error was encountered in "${method}" of the "${hookName}" hook: ${err}`); + return def; + } +} + +function getHookName(logger, hook) { + 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, hooks, hookContext) { + return hooks.map(hook => + tryExecuteStage( + logger, + BEFORE_EVALUATION_STAGE_NAME, + getHookName(logger, hook), + () => hook?.beforeEvaluation?.(hookContext, {}) ?? {}, + {} + ) + ); +} + +function executeAfterEvaluation(logger, hooks, hookContext, updatedData, result) { + // 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, hooks, hookContext) { + return hooks.map(hook => + tryExecuteStage( + logger, + BEFORE_EVALUATION_STAGE_NAME, + getHookName(logger, hook), + () => hook?.beforeIdentify?.(hookContext, {}) ?? {}, + {} + ) + ); +} + +function executeAfterIdentify(logger, hooks, hookContext, updatedData, result) { + // 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) ?? {}, + {} + ); + } +} + +class HookRunner { + constructor(logger, initialHooks) { + this._logger = logger; + this._hooks = initialHooks ? [...initialHooks] : []; + } + + withEvaluation(key, context, defaultValue, method) { + if (this._hooks.length === 0) { + return method(); + } + const hooks = [...this._hooks]; + const hookContext = { + 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, timeout) { + const hooks = [...this._hooks]; + const hookContext = { + context, + timeout, + }; + const hookData = executeBeforeIdentify(this._logger, hooks, hookContext); + return result => { + executeAfterIdentify(this._logger, hooks, hookContext, hookData, result); + }; + } + + addHook(hook) { + this._hooks.push(hook); + } +} + +module.exports = HookRunner; diff --git a/src/index.js b/src/index.js index 5b3c503..2fee70c 100644 --- a/src/index.js +++ b/src/index.js @@ -17,6 +17,7 @@ const messages = require('./messages'); const { checkContext, getContextKeys } = require('./context'); const { InspectorTypes, InspectorManager } = require('./InspectorManager'); const timedPromise = require('./timedPromise'); +const HookRunner = require('./HookRunner'); const changeEvent = 'change'; const internalChangeEvent = 'internal-change'; @@ -40,6 +41,7 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) { const sendEvents = options.sendEvents; let environment = env; let hash = options.hash; + const hookRunner = new HookRunner(logger, options.hooks); const persistentStorage = PersistentStorage(platform.localStorage, logger); @@ -256,11 +258,16 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) { logger.warn(messages.identifyDisabled()); return utils.wrapPromiseCallback(Promise.resolve(utils.transformVersionedValuesToValues(flags)), onDone); } + let afterIdentify; const clearFirst = useLocalStorage && persistentFlagStore ? persistentFlagStore.clearFlags() : Promise.resolve(); return utils.wrapPromiseCallback( clearFirst .then(() => anonymousContextProcessor.processContext(context)) .then(verifyContext) + .then(context => { + afterIdentify = hookRunner.identify(context, undefined); + return context; + }) .then(validatedContext => requestor .fetchFlagSettings(validatedContext, newHash) @@ -277,12 +284,14 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) { }) ) .then(flagValueMap => { + afterIdentify?.({ status: 'completed' }); if (streamActive) { connectStream(); } return flagValueMap; }) .catch(err => { + afterIdentify?.({ status: 'error' }); emitter.maybeReportError(err); return Promise.reject(err); }), @@ -299,11 +308,16 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) { } function variation(key, defaultValue) { - return variationDetailInternal(key, defaultValue, true, false, false, true).value; + const { value } = hookRunner.withEvaluation(key, ident.getContext(), defaultValue, () => + variationDetailInternal(key, defaultValue, true, false, false, true) + ); + return value; } function variationDetail(key, defaultValue) { - return variationDetailInternal(key, defaultValue, true, true, false, true); + return hookRunner.withEvaluation(key, ident.getContext(), defaultValue, () => + variationDetailInternal(key, defaultValue, true, true, false, true) + ); } function variationDetailInternal(key, defaultValue, sendEvent, includeReasonInEvent, isAllFlags, notifyInspection) { @@ -826,6 +840,10 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) { return initializationStateTracker.getInitializationPromise(); } + function addHook(hook) { + hookRunner.addHook(hook); + } + const client = { waitForInitialization, waitUntilReady: () => initializationStateTracker.getReadyPromise(), @@ -840,6 +858,7 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) { flush: flush, allFlags: allFlags, close: close, + addHook: addHook, }; return { diff --git a/typings.d.ts b/typings.d.ts index 615c8a4..a514a65 100644 --- a/typings.d.ts +++ b/typings.d.ts @@ -40,6 +40,85 @@ declare module 'launchdarkly-js-sdk-common' { error: (message: string) => void; } + /** + * Contextual information provided to evaluation stages. + */ +export interface EvaluationSeriesContext { + readonly flagKey: string; + readonly context: LDContext; + readonly defaultValue: unknown; + readonly method: string; +} + + /** + * 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; + } + + /** + * Interface for extending SDK functionality via hooks. + */ + export interface Hook { + /** + * Get metadata about the hook implementation. + */ + getMetadata(): HookMetadata; + + /** + * The before 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; + + /** + * The after 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; + } + /** * LaunchDarkly initialization options that are supported by all variants of the JS client. * The browser SDK and Electron SDK may support additional options. @@ -277,6 +356,25 @@ declare module 'launchdarkly-js-sdk-common' { * Inspectors can be used for collecting information for monitoring, analytics, and debugging. */ inspectors?: LDInspection[]; + + /** + * 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 { TheHook } from '@launchdarkly/some-hook'; + * + * const client = init('my-sdk-key', { hooks: [new TheHook()] }); + * ``` + */ + hooks?: Hook[]; } /** @@ -909,6 +1007,16 @@ declare module 'launchdarkly-js-sdk-common' { * closing is finished. It will never be rejected. */ close(onDone?: () => void): Promise; + + /** + * 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; } /** From be99f3cf218f400f2a5ec602c745379322c69921 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 3 Apr 2025 15:09:57 -0700 Subject: [PATCH 2/9] Add correct client-side typing. --- package.json | 2 +- typings.d.ts | 119 ++++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 110 insertions(+), 11 deletions(-) diff --git a/package.json b/package.json index 72493c4..c7015cd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "launchdarkly-js-sdk-common", - "version": "5.5.0-beta.1", + "version": "5.5.0-beta.2", "description": "LaunchDarkly SDK for JavaScript - common code", "author": "LaunchDarkly ", "license": "Apache-2.0", diff --git a/typings.d.ts b/typings.d.ts index a514a65..e43b106 100644 --- a/typings.d.ts +++ b/typings.d.ts @@ -41,14 +41,28 @@ declare module 'launchdarkly-js-sdk-common' { } /** - * Contextual information provided to evaluation stages. - */ -export interface EvaluationSeriesContext { - readonly flagKey: string; - readonly context: LDContext; - readonly defaultValue: unknown; - readonly method: string; -} + * Contextual information provided to evaluation stages. + */ + export interface EvaluationSeriesContext { + /** + * The flag key the evaluation is for. + */ + readonly flagKey: string; + /** + * Optional in case evaluations are performed before a context is set. + */ + readonly context?: LDContext; + /** + * The default value that was provided. + */ + 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. @@ -63,9 +77,56 @@ export interface EvaluationSeriesContext { * Meta-data about a hook implementation. */ export interface HookMetadata { + /** + * Name of the hook. + */ readonly name: string; } + /** + * Contextual information provided to identify stages. + */ + export interface IdentifySeriesContext { + /** + * The context associated with the identify operation. + */ + readonly context: LDContext; + /** + * The timeout, in seconds, associated with the identify operation. + */ + readonly timeout?: number; + } + + /** + * Implementation specific hook data for identify stages. + * + * Hook implementations can use this to store data needed between stages. + */ + export interface IdentifySeriesData { + readonly [index: string]: unknown; + } + + /** + * The status an identify operation completed with. + * + * An example in which an error may occur is lack of network connectivity + * preventing the SDK from functioning. + */ + 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 identify series. + * + * For example, a network issue may cause an identify to error since the SDK + * can't refresh its cached data from the cloud at that moment, but then later + * the when the network issue is resolved, the SDK will refresh cached data. + */ + export interface IdentifySeriesResult { + status: IdentifySeriesStatus; + } + /** * Interface for extending SDK functionality via hooks. */ @@ -76,7 +137,7 @@ export interface EvaluationSeriesContext { getMetadata(): HookMetadata; /** - * The before method is called during the execution of a variation method + * 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 @@ -96,7 +157,7 @@ export interface EvaluationSeriesContext { ): EvaluationSeriesData; /** - * The after method is called during the execution of the variation method + * 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 @@ -117,6 +178,44 @@ export interface 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: IdentifySeriesResult, + ): IdentifySeriesData; } /** From 2dc1b2db899136776c9aa7ac1f76a5fcc2223912 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Thu, 3 Apr 2025 15:23:53 -0700 Subject: [PATCH 3/9] Execute identify hook stages during initialization. --- package.json | 2 +- src/index.js | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index c7015cd..6281381 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "launchdarkly-js-sdk-common", - "version": "5.5.0-beta.2", + "version": "5.5.0-beta.3", "description": "LaunchDarkly SDK for JavaScript - common code", "author": "LaunchDarkly ", "license": "Apache-2.0", diff --git a/src/index.js b/src/index.js index 2fee70c..477cc4d 100644 --- a/src/index.js +++ b/src/index.js @@ -682,10 +682,16 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) { if (!env) { return Promise.reject(new errors.LDInvalidEnvironmentIdError(messages.environmentNotSpecified())); } + let afterIdentify; return anonymousContextProcessor .processContext(context) .then(verifyContext) + .then(context => { + afterIdentify = hookRunner.identify(context, undefined); + return context; + }) .then(validatedContext => { + afterIdentify?.({ status: 'completed' }); ident.setContext(validatedContext); if (typeof options.bootstrap === 'object') { // flags have already been set earlier @@ -695,6 +701,10 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) { } else { return finishInitWithPolling(); } + }) + .catch(err => { + afterIdentify?.({ status: 'error' }); + throw err; }); } From bbf62d4c71016ccf4b1eeda89144dbdf7bec474c Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 8 Apr 2025 16:55:08 -0700 Subject: [PATCH 4/9] Use a factory and add tests. --- src/HookRunner.js | 133 ++++++++++-- src/__tests__/HookRunner-test.js | 335 +++++++++++++++++++++++++++++++ src/index.js | 4 +- 3 files changed, 451 insertions(+), 21 deletions(-) create mode 100644 src/__tests__/HookRunner-test.js diff --git a/src/HookRunner.js b/src/HookRunner.js index 59582f6..c64dd9e 100644 --- a/src/HookRunner.js +++ b/src/HookRunner.js @@ -1,7 +1,18 @@ const UNKNOWN_HOOK_NAME = 'unknown hook'; const BEFORE_EVALUATION_STAGE_NAME = 'beforeEvaluation'; const AFTER_EVALUATION_STAGE_NAME = 'afterEvaluation'; +const BEFORE_IDENTIFY_STAGE_NAME = 'beforeIdentify'; +const AFTER_IDENTIFY_STAGE_NAME = 'afterIdentify'; +/** + * Safely executes a hook stage function, logging any errors. + * @param {{ error: (message: string) => void } | undefined} logger The logger instance. + * @param {string} method The name of the hook stage being executed (e.g., 'beforeEvaluation'). + * @param {string} hookName The name of the hook. + * @param {() => any} stage The function representing the hook stage to execute. + * @param {any} def The default value to return if the stage function throws an error. + * @returns {any} The result of the stage function, or the default value if an error occurred. + */ function tryExecuteStage(logger, method, hookName, stage, def) { try { return stage(); @@ -11,6 +22,12 @@ function tryExecuteStage(logger, method, hookName, stage, def) { } } +/** + * Safely gets the name of a hook from its metadata. + * @param {{ error: (message: string) => void }} logger The logger instance. + * @param {{ getMetadata: () => { name?: string } }} hook The hook instance. + * @returns {string} The name of the hook, or 'unknown hook' if unable to retrieve it. + */ function getHookName(logger, hook) { try { return hook.getMetadata().name || UNKNOWN_HOOK_NAME; @@ -20,6 +37,13 @@ function getHookName(logger, hook) { } } +/** + * Executes the 'beforeEvaluation' stage for all registered hooks. + * @param {{ error: (message: string) => void }} logger The logger instance. + * @param {Array<{ beforeEvaluation?: (hookContext: object, data: object) => object }>} hooks The array of hook instances. + * @param {{ flagKey: string, context: object, defaultValue: any }} hookContext The context for the evaluation series. + * @returns {Array} An array containing the data returned by each hook's 'beforeEvaluation' stage. + */ function executeBeforeEvaluation(logger, hooks, hookContext) { return hooks.map(hook => tryExecuteStage( @@ -32,6 +56,15 @@ function executeBeforeEvaluation(logger, hooks, hookContext) { ); } +/** + * Executes the 'afterEvaluation' stage for all registered hooks in reverse order. + * @param {{ error: (message: string) => void }} logger The logger instance. + * @param {Array<{ afterEvaluation?: (hookContext: object, data: object, result: object) => object }>} hooks The array of hook instances. + * @param {{ flagKey: string, context: object, defaultValue: any }} hookContext The context for the evaluation series. + * @param {Array} updatedData The data collected from the 'beforeEvaluation' stages. + * @param {{ value: any, variationIndex?: number, reason?: object }} result The result of the flag evaluation. + * @returns {void} + */ function executeAfterEvaluation(logger, hooks, hookContext, updatedData, result) { // This iterates in reverse, versus reversing a shallow copy of the hooks, // for efficiency. @@ -48,11 +81,18 @@ function executeAfterEvaluation(logger, hooks, hookContext, updatedData, result) } } +/** + * Executes the 'beforeIdentify' stage for all registered hooks. + * @param {{ error: (message: string) => void }} logger The logger instance. + * @param {Array<{ beforeIdentify?: (hookContext: object, data: object) => object }>} hooks The array of hook instances. + * @param {{ context: object, timeout?: number }} hookContext The context for the identify series. + * @returns {Array} An array containing the data returned by each hook's 'beforeIdentify' stage. + */ function executeBeforeIdentify(logger, hooks, hookContext) { return hooks.map(hook => tryExecuteStage( logger, - BEFORE_EVALUATION_STAGE_NAME, + BEFORE_IDENTIFY_STAGE_NAME, getHookName(logger, hook), () => hook?.beforeIdentify?.(hookContext, {}) ?? {}, {} @@ -60,6 +100,15 @@ function executeBeforeIdentify(logger, hooks, hookContext) { ); } +/** + * Executes the 'afterIdentify' stage for all registered hooks in reverse order. + * @param {{ error: (message: string) => void }} logger The logger instance. + * @param {Array<{ afterIdentify?: (hookContext: object, data: object, result: object) => object }>} hooks The array of hook instances. + * @param {{ context: object, timeout?: number }} hookContext The context for the identify series. + * @param {Array} updatedData The data collected from the 'beforeIdentify' stages. + * @param {{ status: string }} result The result of the identify operation. + * @returns {void} + */ function executeAfterIdentify(logger, hooks, hookContext, updatedData, result) { // This iterates in reverse, versus reversing a shallow copy of the hooks, // for efficiency. @@ -68,7 +117,7 @@ function executeAfterIdentify(logger, hooks, hookContext, updatedData, result) { const data = updatedData[hookIndex]; tryExecuteStage( logger, - AFTER_EVALUATION_STAGE_NAME, + AFTER_IDENTIFY_STAGE_NAME, getHookName(logger, hook), () => hook?.afterIdentify?.(hookContext, data, result) ?? {}, {} @@ -76,44 +125,90 @@ function executeAfterIdentify(logger, hooks, hookContext, updatedData, result) { } } -class HookRunner { - constructor(logger, initialHooks) { - this._logger = logger; - this._hooks = initialHooks ? [...initialHooks] : []; - } +/** + * Factory function to create a HookRunner instance. + * Manages the execution of hooks for flag evaluations and identify operations. + * @param {{ error: (message: string) => void }} logger The logger instance. + * @param {Array | undefined} initialHooks An optional array of hooks to initialize with. + * @returns {{ + * withEvaluation: (key: string, context: object, defaultValue: any, method: () => { value: any, variationIndex?: number, reason?: object }) => { value: any, variationIndex?: number, reason?: object }, + * identify: (context: object, timeout?: number) => (result: { status: string }) => void, + * addHook: (hook: object) => void + * }} The hook runner object with methods to manage and execute hooks. + */ +function createHookRunner(logger, initialHooks) { + // Use local variable instead of instance property + const hooksInternal = initialHooks ? [...initialHooks] : []; - withEvaluation(key, context, defaultValue, method) { - if (this._hooks.length === 0) { + /** + * Wraps a flag evaluation method with before/after hook stages. + * @param {string} key The flag key. + * @param {object} context The evaluation context. + * @param {any} defaultValue The default value for the flag. + * @param {() => { value: any, variationIndex?: number, reason?: object }} method The function that performs the actual flag evaluation. + * @returns {{ value: any, variationIndex?: number, reason?: object }} The result of the flag evaluation. + */ + function withEvaluation(key, context, defaultValue, method) { + if (hooksInternal.length === 0) { return method(); } - const hooks = [...this._hooks]; + const hooks = [...hooksInternal]; + /** @type {{ flagKey: string, context: object, defaultValue: any }} */ const hookContext = { flagKey: key, context, defaultValue, }; - const hookData = executeBeforeEvaluation(this._logger, hooks, hookContext); + // Use the logger passed into the factory + const hookData = executeBeforeEvaluation(logger, hooks, hookContext); const result = method(); - executeAfterEvaluation(this._logger, hooks, hookContext, hookData, result); + executeAfterEvaluation(logger, hooks, hookContext, hookData, result); return result; } - identify(context, timeout) { - const hooks = [...this._hooks]; + /** + * Wraps the identify operation with before/after hook stages. + * Executes the 'beforeIdentify' stage immediately and returns a function + * to execute the 'afterIdentify' stage later. + * @param {object} context The context being identified. + * @param {number | undefined} timeout Optional timeout for the identify operation. + * @returns {(result: { status: string }) => void} A function to call after the identify operation completes. + */ + function identify(context, timeout) { + const hooks = [...hooksInternal]; + /** @type {{ context: object, timeout?: number }} */ const hookContext = { context, timeout, }; - const hookData = executeBeforeIdentify(this._logger, hooks, hookContext); + // Use the logger passed into the factory + const hookData = executeBeforeIdentify(logger, hooks, hookContext); + /** + * Executes the 'afterIdentify' hook stage. + * @param {{ status: string }} result The result of the identify operation. + */ return result => { - executeAfterIdentify(this._logger, hooks, hookContext, hookData, result); + executeAfterIdentify(logger, hooks, hookContext, hookData, result); }; } - addHook(hook) { - this._hooks.push(hook); + /** + * Adds a new hook to the runner. + * @param {object} hook The hook instance to add. + * @returns {void} + */ + function addHook(hook) { + // Mutate the internal hooks array + hooksInternal.push(hook); } + + // Return the public API + return { + withEvaluation, + identify, + addHook, + }; } -module.exports = HookRunner; +module.exports = createHookRunner; diff --git a/src/__tests__/HookRunner-test.js b/src/__tests__/HookRunner-test.js new file mode 100644 index 0000000..e9dd68d --- /dev/null +++ b/src/__tests__/HookRunner-test.js @@ -0,0 +1,335 @@ +// The HookRunner factory function under test +const createHookRunner = require('../HookRunner'); + +// Mock the logger functions we expect to be called +const mockLogger = () => ({ + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), +}); + +// Define a basic Hook structure for tests +const createTestHook = (name = 'Test Hook') => ({ + getMetadata: jest.fn().mockReturnValue({ name }), + beforeEvaluation: jest.fn(), + afterEvaluation: jest.fn(), + beforeIdentify: jest.fn(), + afterIdentify: jest.fn(), +}); + +describe('Given a logger, runner, and hook', () => { + let logger; + let testHook; + let hookRunner; + + beforeEach(() => { + // Reset mocks and create fresh instances for each test + logger = mockLogger(); + testHook = createTestHook(); + // Initialize the runner with the test hook + hookRunner = createHookRunner(logger, [testHook]); + }); + + it('evaluation: should execute hooks and return the evaluation result', () => { + const key = 'test-flag'; + const context = { kind: 'user', key: 'user-123' }; + const defaultValue = false; + const evaluationResult = { + value: true, + variationIndex: 1, + reason: { kind: 'OFF' }, + }; + // Mock the core evaluation method + const method = jest.fn().mockReturnValue(evaluationResult); + + // The data expected to be passed between stages initially is empty + const initialData = {}; + + const result = hookRunner.withEvaluation(key, context, defaultValue, method); + + // Check if beforeEvaluation was called correctly + expect(testHook.beforeEvaluation).toHaveBeenCalledWith( + expect.objectContaining({ + flagKey: key, + context, + defaultValue, + }), + initialData // Initial data passed to beforeEvaluation + ); + + // Check if the original evaluation method was called + expect(method).toHaveBeenCalled(); + + // Check if afterEvaluation was called correctly + expect(testHook.afterEvaluation).toHaveBeenCalledWith( + expect.objectContaining({ + flagKey: key, + context, + defaultValue, + }), + initialData, // Data returned from (mocked) beforeEvaluation + evaluationResult + ); + + // Verify the final result matches the evaluation result + expect(result).toEqual(evaluationResult); + }); + + it('evaluation: should handle errors in beforeEvaluation hook', () => { + const errorHook = createTestHook('Error Hook'); + const testError = new Error('Hook error in before'); + errorHook.beforeEvaluation.mockImplementation(() => { + throw testError; + }); + + const errorHookRunner = createHookRunner(logger, [errorHook]); + const method = jest.fn().mockReturnValue({ value: 'default', reason: { kind: 'ERROR' } }); + const initialData = {}; // Data returned by the failing hook (default) + + errorHookRunner.withEvaluation('test-flag', { kind: 'user', key: 'user-123' }, false, method); + + // Error should be logged + expect(logger.error).toHaveBeenCalledWith( + 'An error was encountered in "beforeEvaluation" of the "Error Hook" hook: Error: Hook error in before' + ); + // Method should still be called + expect(method).toHaveBeenCalled(); + // After evaluation should still be called, passing the default data ({}) because before failed + expect(errorHook.afterEvaluation).toHaveBeenCalledWith(expect.anything(), initialData, expect.anything()); + }); + + it('evaluation: should handle errors in afterEvaluation hook', () => { + const errorHook = createTestHook('Error Hook'); + const testError = new Error('Hook error in after'); + errorHook.afterEvaluation.mockImplementation(() => { + throw testError; + }); + + const errorHookRunner = createHookRunner(logger, [errorHook]); + const method = jest.fn().mockReturnValue({ value: 'default', reason: { kind: 'FALLTHROUGH' } }); + const initialData = {}; + + errorHookRunner.withEvaluation('test-flag', { kind: 'user', key: 'user-123' }, false, method); + + // Before evaluation should be called normally + expect(errorHook.beforeEvaluation).toHaveBeenCalled(); + // Method should be called normally + expect(method).toHaveBeenCalled(); + // Error should be logged for afterEvaluation + expect(logger.error).toHaveBeenCalledWith( + 'An error was encountered in "afterEvaluation" of the "Error Hook" hook: Error: Hook error in after' + ); + }); + + it('evaluation: should skip hook execution if no hooks are provided', () => { + const emptyHookRunner = createHookRunner(logger, []); // No initial hooks + const method = jest.fn().mockReturnValue({ value: true }); + + emptyHookRunner.withEvaluation('test-flag', { kind: 'user', key: 'user-123' }, false, method); + + // Only the method should be called + expect(method).toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); + expect(logger.warn).not.toHaveBeenCalled(); + }); + + it('evaluation: should pass data from beforeEvaluation to afterEvaluation', () => { + const key = 'test-flag'; + const context = { kind: 'user', key: 'user-123' }; + const defaultValue = false; + const evaluationResult = { value: true }; + const seriesData = { testData: 'before data' }; + + // Mock beforeEvaluation to return specific data + testHook.beforeEvaluation.mockReturnValue(seriesData); + const method = jest.fn().mockReturnValue(evaluationResult); + + hookRunner.withEvaluation(key, context, defaultValue, method); + + expect(testHook.beforeEvaluation).toHaveBeenCalled(); + // afterEvaluation should receive the data returned by beforeEvaluation + expect(testHook.afterEvaluation).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining(seriesData), // Check if the passed data includes seriesData + evaluationResult + ); + }); + + it('identify: should execute identify hooks', () => { + const context = { kind: 'user', key: 'user-123' }; + const timeout = 10; + const identifyResult = { status: 'completed' }; // Example result structure + const initialData = {}; + + // Call identify to get the callback + const identifyCallback = hookRunner.identify(context, timeout); + + // Check if beforeIdentify was called immediately + expect(testHook.beforeIdentify).toHaveBeenCalledWith( + expect.objectContaining({ + context, + timeout, + }), + initialData // Initial data passed to beforeIdentify + ); + + // Now invoke the callback returned by identify + identifyCallback(identifyResult); + + // Check if afterIdentify was called with the correct arguments + expect(testHook.afterIdentify).toHaveBeenCalledWith( + expect.objectContaining({ + context, + timeout, + }), + initialData, // Data returned from (mocked) beforeIdentify + identifyResult + ); + }); + + it('identify: should handle errors in beforeIdentify hook', () => { + const errorHook = createTestHook('Error Hook'); + const testError = new Error('Hook error in before identify'); + errorHook.beforeIdentify.mockImplementation(() => { + throw testError; + }); + + const errorHookRunner = createHookRunner(logger, [errorHook]); + const identifyCallback = errorHookRunner.identify({ kind: 'user', key: 'user-456' }, 1000); + + // Error should be logged immediately from beforeIdentify + expect(logger.error).toHaveBeenCalledWith( + 'An error was encountered in "beforeIdentify" of the "Error Hook" hook: Error: Hook error in before identify' + ); + + // Execute the callback - afterIdentify should still be called + identifyCallback({ status: 'error' }); // Example result + + // Check afterIdentify was called, receiving default data {} + expect(errorHook.afterIdentify).toHaveBeenCalledWith(expect.anything(), {}, expect.anything()); + }); + + it('identify: should handle errors in afterIdentify hook', () => { + const errorHook = createTestHook('Error Hook'); + const testError = new Error('Hook error in after identify'); + errorHook.afterIdentify.mockImplementation(() => { + throw testError; + }); + + const errorHookRunner = createHookRunner(logger, [errorHook]); + const identifyCallback = errorHookRunner.identify({ kind: 'user', key: 'user-456' }, 1000); + + // Before should run fine + expect(errorHook.beforeIdentify).toHaveBeenCalled(); + expect(logger.error).not.toHaveBeenCalled(); + + // Execute the callback - this should trigger the error in afterIdentify + identifyCallback({ status: 'completed' }); // Example result + + // Error should be logged from afterIdentify + expect(logger.error).toHaveBeenCalledWith( + 'An error was encountered in "afterIdentify" of the "Error Hook" hook: Error: Hook error in after identify' + ); + }); + + + it('identify: should pass data from beforeIdentify to afterIdentify', () => { + const context = { kind: 'user', key: 'user-789' }; + const timeout = 50; + const identifyResult = { status: 'completed' }; + const seriesData = { testData: 'before identify data' }; + + // Mock beforeIdentify to return specific data + testHook.beforeIdentify.mockReturnValue(seriesData); + + const identifyCallback = hookRunner.identify(context, timeout); + identifyCallback(identifyResult); + + expect(testHook.beforeIdentify).toHaveBeenCalled(); + // afterIdentify should receive the data returned by beforeIdentify + expect(testHook.afterIdentify).toHaveBeenCalledWith( + expect.anything(), + expect.objectContaining(seriesData), // Check if the passed data includes seriesData + identifyResult + ); + }); + + it('addHook: should use the added hook in future invocations', () => { + const newHook = createTestHook('New Hook'); + hookRunner.addHook(newHook); + + const method = jest.fn().mockReturnValue({ value: true }); + hookRunner.withEvaluation('test-flag', { kind: 'user', key: 'user-123' }, false, method); + + // Both the original and the new hook should have been called + expect(testHook.beforeEvaluation).toHaveBeenCalled(); + expect(testHook.afterEvaluation).toHaveBeenCalled(); + expect(newHook.beforeEvaluation).toHaveBeenCalled(); + expect(newHook.afterEvaluation).toHaveBeenCalled(); + }); + + it('error handling: should log "unknown hook" when getMetadata throws', () => { + const errorMetadataHook = { + getMetadata: jest.fn().mockImplementation(() => { + throw new Error('Metadata error'); + }), + beforeEvaluation: jest.fn().mockImplementation(() => { + throw new Error('Eval error'); // Add an error here to trigger logging + }), + afterEvaluation: jest.fn(), + // Add other methods if needed for completeness, mocked simply + beforeIdentify: jest.fn(), + afterIdentify: jest.fn(), + }; + + const errorRunner = createHookRunner(logger, [errorMetadataHook]); + errorRunner.withEvaluation('flag', {}, false, () => ({ value: null })); + + // First error: getting metadata + expect(logger.error).toHaveBeenCalledWith( + 'Exception thrown getting metadata for hook. Unable to get hook name.' + ); + // Second error: executing the stage with 'unknown hook' name + expect(logger.error).toHaveBeenCalledWith( + 'An error was encountered in "beforeEvaluation" of the "unknown hook" hook: Error: Eval error' + ); + }); + + it('error handling: should log "unknown hook" when getMetadata returns empty name', () => { + const emptyNameHook = createTestHook(''); // Create hook with empty name + emptyNameHook.beforeEvaluation.mockImplementation(() => { + throw new Error('Eval error'); // Add an error here to trigger logging + }); + + const errorRunner = createHookRunner(logger, [emptyNameHook]); + errorRunner.withEvaluation('flag', {}, false, () => ({ value: null })); + + // Verify getMetadata was called (even though name is empty) + expect(emptyNameHook.getMetadata).toHaveBeenCalled(); + + // Verify the error uses 'unknown hook' + expect(logger.error).toHaveBeenCalledWith( + 'An error was encountered in "beforeEvaluation" of the "unknown hook" hook: Error: Eval error' + ); + }); + + it('error handling: should log the correct hook name when an error occurs', () => { + const hookName = 'Specific Error Hook'; + const errorHook = createTestHook(hookName); + const testError = new Error('Specific test error'); + errorHook.beforeEvaluation.mockImplementation(() => { + throw testError; + }); + + const specificRunner = createHookRunner(logger, [errorHook]); + specificRunner.withEvaluation('flag', {}, false, () => ({ value: null })); + + // Verify getMetadata was called + expect(errorHook.getMetadata).toHaveBeenCalled(); + // Verify the error message includes the correct hook name + expect(logger.error).toHaveBeenCalledWith( + `An error was encountered in "beforeEvaluation" of the "${hookName}" hook: Error: Specific test error` + ); + }); +}); \ No newline at end of file diff --git a/src/index.js b/src/index.js index 477cc4d..619ab3f 100644 --- a/src/index.js +++ b/src/index.js @@ -17,7 +17,7 @@ const messages = require('./messages'); const { checkContext, getContextKeys } = require('./context'); const { InspectorTypes, InspectorManager } = require('./InspectorManager'); const timedPromise = require('./timedPromise'); -const HookRunner = require('./HookRunner'); +const createHookRunner = require('./HookRunner'); const changeEvent = 'change'; const internalChangeEvent = 'internal-change'; @@ -41,7 +41,7 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) { const sendEvents = options.sendEvents; let environment = env; let hash = options.hash; - const hookRunner = new HookRunner(logger, options.hooks); + const hookRunner = createHookRunner(logger, options.hooks); const persistentStorage = PersistentStorage(platform.localStorage, logger); From c959d192f6a78cf6dc58216b53f8da8da74eb38a Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 8 Apr 2025 17:13:16 -0700 Subject: [PATCH 5/9] Add client level tests. --- src/__tests__/LDClient-hooks-test.js | 230 +++++++++++++++++++++++++++ 1 file changed, 230 insertions(+) create mode 100644 src/__tests__/LDClient-hooks-test.js diff --git a/src/__tests__/LDClient-hooks-test.js b/src/__tests__/LDClient-hooks-test.js new file mode 100644 index 0000000..db78509 --- /dev/null +++ b/src/__tests__/LDClient-hooks-test.js @@ -0,0 +1,230 @@ +const { initialize } = require('../index'); +const stubPlatform = require('./stubPlatform'); +const { respondJson } = require('./mockHttp'); + +// Mock the logger functions +const mockLogger = () => ({ + error: jest.fn(), + warn: jest.fn(), + info: jest.fn(), + debug: jest.fn(), +}); + +// Define a basic Hook structure for tests +const createTestHook = (name = 'Test Hook') => ({ + getMetadata: jest.fn().mockReturnValue({ name }), + beforeEvaluation: jest.fn(), + afterEvaluation: jest.fn(), + beforeIdentify: jest.fn().mockImplementation((_ctx, data) => data), // Pass data through + afterIdentify: jest.fn(), +}); + +// Helper to initialize the client for tests +// Disables network requests and event sending by default +async function withClient(initialContext, configOverrides = {}, hooks = [], testFn) { + const platform = stubPlatform.defaults(); + const server = platform.testing.http.newServer(); + + const logger = mockLogger(); + + // Disable streaming and event sending unless overridden + // Configure client to use the mock server's URL + const defaults = { + baseUrl: server.url, // Use mock server URL + streaming: false, + sendEvents: false, + useLdd: false, + logger: logger, + hooks: hooks, // Pass initial hooks here + }; + const config = { ...defaults, ...configOverrides }; + const { client, start } = initialize('env', initialContext, config, platform); + + // Set the mock server to return an empty flag set by default + server.byDefault(respondJson({})); // Correct way to provide initial flags + + start(); // Start the client components + + try { + // Wait briefly for initialization (client will hit the mock server) + await client.waitForInitialization(10); // Use a short timeout + await testFn(client, logger, platform); // Pass client, logger, platform to the test + } finally { + await client.close(); + server.close(); // Close the mock server + } +} + +describe('LDClient Hooks Integration', () => { + const initialContext = { kind: 'user', key: 'user-key-initial' }; + const flagKey = 'test-flag'; + const flagDefaultValue = false; + // Expected result when flag is not found (as it will be with empty flags) + const flagNotFoundDetail = { + value: flagDefaultValue, + variationIndex: null, + reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' }, + }; + + it('should use hooks registered during configuration', async () => { + const testHook = createTestHook('Initial Hook'); + const initialData = {}; // Hooks start with empty data + + await withClient(initialContext, {}, [testHook], async (client) => { + + // Call variation + await client.variation(flagKey, flagDefaultValue); + + // Check identify hooks + expect(testHook.beforeIdentify).toHaveBeenCalledTimes(1); + expect(testHook.beforeIdentify).toHaveBeenCalledWith( + expect.objectContaining({ + context: initialContext, + // timeout will be undefined unless explicitly passed to identify + }), + initialData + ); + expect(testHook.afterIdentify).toHaveBeenCalledTimes(1); + expect(testHook.afterIdentify).toHaveBeenCalledWith( + expect.objectContaining({ + context: initialContext, + }), + initialData, // Assumes beforeIdentify just returned the initial data + { status: 'completed' } + ); + + // Check evaluation hooks (context from identify is now current) + expect(testHook.beforeEvaluation).toHaveBeenCalledTimes(1); + expect(testHook.beforeEvaluation).toHaveBeenCalledWith( + expect.objectContaining({ + flagKey: flagKey, + context: initialContext, + defaultValue: flagDefaultValue, + }), + initialData + ); + expect(testHook.afterEvaluation).toHaveBeenCalledTimes(1); + expect(testHook.afterEvaluation).toHaveBeenCalledWith( + expect.objectContaining({ + flagKey: flagKey, + context: initialContext, + defaultValue: flagDefaultValue, + }), + initialData, // Assumes beforeEvaluation just returned the initial data + flagNotFoundDetail // Using the default flag not found result + ); + }); + }); + + it('should execute hooks that are added using addHook', async () => { + const addedHook = createTestHook('Added Hook'); + const identifyContext = { kind: 'user', key: 'user-key-added' }; + const initialData = {}; + + // Initialize client *without* the hook initially + await withClient(initialContext, {}, [], async (client) => { + // Add the hook dynamically + client.addHook(addedHook); + + // Call identify and variation + await client.identify(identifyContext); + await client.variation(flagKey, flagDefaultValue); + + // Check identify hooks + expect(addedHook.beforeIdentify).toHaveBeenCalledTimes(1); + expect(addedHook.beforeIdentify).toHaveBeenCalledWith( + expect.objectContaining({ context: identifyContext }), + initialData + ); + expect(addedHook.afterIdentify).toHaveBeenCalledTimes(1); + expect(addedHook.afterIdentify).toHaveBeenCalledWith( + expect.objectContaining({ context: identifyContext }), + initialData, + { status: 'completed' } + ); + + // Check evaluation hooks + expect(addedHook.beforeEvaluation).toHaveBeenCalledTimes(1); + expect(addedHook.beforeEvaluation).toHaveBeenCalledWith( + expect.objectContaining({ flagKey, context: identifyContext, defaultValue: flagDefaultValue }), + initialData + ); + expect(addedHook.afterEvaluation).toHaveBeenCalledTimes(1); + expect(addedHook.afterEvaluation).toHaveBeenCalledWith( + expect.objectContaining({ flagKey, context: identifyContext, defaultValue: flagDefaultValue }), + initialData, + flagNotFoundDetail + ); + }); + }); + + it('should execute both initial hooks and hooks added using addHook', async () => { + const initialHook = createTestHook('Initial Hook For Both'); + const addedHook = createTestHook('Added Hook For Both'); + const identifyContext = { kind: 'user', key: 'user-key-both' }; + const initialData = {}; + + // Initialize client *with* the initial hook + await withClient(initialContext, {}, [initialHook], async (client) => { + // Add the second hook dynamically + client.addHook(addedHook); + + await client.identify(identifyContext); + await client.variation(flagKey, flagDefaultValue); + + expect(initialHook.beforeIdentify).toHaveBeenCalledTimes(2); + expect(initialHook.beforeIdentify).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ context: initialContext }), + initialData + ); + expect(initialHook.beforeIdentify).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ context: identifyContext }), + initialData + ); + + expect(initialHook.afterIdentify).toHaveBeenCalledTimes(2); + expect(initialHook.afterIdentify).toHaveBeenNthCalledWith( + 1, + expect.objectContaining({ context: initialContext }), + initialData, // Assuming pass-through + { status: 'completed' } + ); + expect(initialHook.afterIdentify).toHaveBeenNthCalledWith( + 2, + expect.objectContaining({ context: identifyContext }), + initialData, // Assuming pass-through + { status: 'completed' } + ); + + expect(addedHook.beforeIdentify).toHaveBeenCalledTimes(1); + expect(addedHook.beforeIdentify).toHaveBeenCalledWith( + expect.objectContaining({ context: identifyContext }), + initialData + ); + expect(addedHook.afterIdentify).toHaveBeenCalledTimes(1); + expect(addedHook.afterIdentify).toHaveBeenCalledWith( + expect.objectContaining({ context: identifyContext }), + initialData, // Assuming pass-through + { status: 'completed' } + ); + + + // Check evaluation hooks for BOTH hooks + [initialHook, addedHook].forEach(hook => { + expect(hook.beforeEvaluation).toHaveBeenCalledTimes(1); + expect(hook.beforeEvaluation).toHaveBeenCalledWith( + expect.objectContaining({ flagKey, context: identifyContext, defaultValue: flagDefaultValue }), + initialData + ); + expect(hook.afterEvaluation).toHaveBeenCalledTimes(1); + expect(hook.afterEvaluation).toHaveBeenCalledWith( + expect.objectContaining({ flagKey, context: identifyContext, defaultValue: flagDefaultValue }), + initialData, // Assuming pass-through + flagNotFoundDetail + ); + }); + }); + }); +}); \ No newline at end of file From 2aeb5d47ee942cbf3d81747e147ad1667974a470 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 8 Apr 2025 17:14:07 -0700 Subject: [PATCH 6/9] Remove extra comment. --- src/HookRunner.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/HookRunner.js b/src/HookRunner.js index c64dd9e..3e314ad 100644 --- a/src/HookRunner.js +++ b/src/HookRunner.js @@ -203,7 +203,6 @@ function createHookRunner(logger, initialHooks) { hooksInternal.push(hook); } - // Return the public API return { withEvaluation, identify, From 7e901c09009881378072a1c350637c559d418d3a Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 8 Apr 2025 17:15:00 -0700 Subject: [PATCH 7/9] Remove package.json change. --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6281381..d236b19 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "launchdarkly-js-sdk-common", - "version": "5.5.0-beta.3", + "version": "5.4.0", "description": "LaunchDarkly SDK for JavaScript - common code", "author": "LaunchDarkly ", "license": "Apache-2.0", From feb012dfd3dd6c804ea45bf1dd7af277882d2cba Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 8 Apr 2025 17:16:54 -0700 Subject: [PATCH 8/9] Linting --- src/__tests__/HookRunner-test.js | 18 ++++++--------- src/__tests__/LDClient-hooks-test.js | 34 +++++++++++++--------------- 2 files changed, 23 insertions(+), 29 deletions(-) diff --git a/src/__tests__/HookRunner-test.js b/src/__tests__/HookRunner-test.js index e9dd68d..fd1a099 100644 --- a/src/__tests__/HookRunner-test.js +++ b/src/__tests__/HookRunner-test.js @@ -108,7 +108,6 @@ describe('Given a logger, runner, and hook', () => { const errorHookRunner = createHookRunner(logger, [errorHook]); const method = jest.fn().mockReturnValue({ value: 'default', reason: { kind: 'FALLTHROUGH' } }); - const initialData = {}; errorHookRunner.withEvaluation('test-flag', { kind: 'user', key: 'user-123' }, false, method); @@ -210,7 +209,7 @@ describe('Given a logger, runner, and hook', () => { expect(errorHook.afterIdentify).toHaveBeenCalledWith(expect.anything(), {}, expect.anything()); }); - it('identify: should handle errors in afterIdentify hook', () => { + it('identify: should handle errors in afterIdentify hook', () => { const errorHook = createTestHook('Error Hook'); const testError = new Error('Hook error in after identify'); errorHook.afterIdentify.mockImplementation(() => { @@ -233,7 +232,6 @@ describe('Given a logger, runner, and hook', () => { ); }); - it('identify: should pass data from beforeIdentify to afterIdentify', () => { const context = { kind: 'user', key: 'user-789' }; const timeout = 50; @@ -287,9 +285,7 @@ describe('Given a logger, runner, and hook', () => { errorRunner.withEvaluation('flag', {}, false, () => ({ value: null })); // First error: getting metadata - expect(logger.error).toHaveBeenCalledWith( - 'Exception thrown getting metadata for hook. Unable to get hook name.' - ); + expect(logger.error).toHaveBeenCalledWith('Exception thrown getting metadata for hook. Unable to get hook name.'); // Second error: executing the stage with 'unknown hook' name expect(logger.error).toHaveBeenCalledWith( 'An error was encountered in "beforeEvaluation" of the "unknown hook" hook: Error: Eval error' @@ -298,15 +294,15 @@ describe('Given a logger, runner, and hook', () => { it('error handling: should log "unknown hook" when getMetadata returns empty name', () => { const emptyNameHook = createTestHook(''); // Create hook with empty name - emptyNameHook.beforeEvaluation.mockImplementation(() => { - throw new Error('Eval error'); // Add an error here to trigger logging - }); + emptyNameHook.beforeEvaluation.mockImplementation(() => { + throw new Error('Eval error'); // Add an error here to trigger logging + }); const errorRunner = createHookRunner(logger, [emptyNameHook]); errorRunner.withEvaluation('flag', {}, false, () => ({ value: null })); // Verify getMetadata was called (even though name is empty) - expect(emptyNameHook.getMetadata).toHaveBeenCalled(); + expect(emptyNameHook.getMetadata).toHaveBeenCalled(); // Verify the error uses 'unknown hook' expect(logger.error).toHaveBeenCalledWith( @@ -332,4 +328,4 @@ describe('Given a logger, runner, and hook', () => { `An error was encountered in "beforeEvaluation" of the "${hookName}" hook: Error: Specific test error` ); }); -}); \ No newline at end of file +}); diff --git a/src/__tests__/LDClient-hooks-test.js b/src/__tests__/LDClient-hooks-test.js index db78509..43c3713 100644 --- a/src/__tests__/LDClient-hooks-test.js +++ b/src/__tests__/LDClient-hooks-test.js @@ -24,7 +24,7 @@ const createTestHook = (name = 'Test Hook') => ({ async function withClient(initialContext, configOverrides = {}, hooks = [], testFn) { const platform = stubPlatform.defaults(); const server = platform.testing.http.newServer(); - + const logger = mockLogger(); // Disable streaming and event sending unless overridden @@ -70,8 +70,7 @@ describe('LDClient Hooks Integration', () => { const testHook = createTestHook('Initial Hook'); const initialData = {}; // Hooks start with empty data - await withClient(initialContext, {}, [testHook], async (client) => { - + await withClient(initialContext, {}, [testHook], async client => { // Call variation await client.variation(flagKey, flagDefaultValue); @@ -122,7 +121,7 @@ describe('LDClient Hooks Integration', () => { const initialData = {}; // Initialize client *without* the hook initially - await withClient(initialContext, {}, [], async (client) => { + await withClient(initialContext, {}, [], async client => { // Add the hook dynamically client.addHook(addedHook); @@ -165,7 +164,7 @@ describe('LDClient Hooks Integration', () => { const initialData = {}; // Initialize client *with* the initial hook - await withClient(initialContext, {}, [initialHook], async (client) => { + await withClient(initialContext, {}, [initialHook], async client => { // Add the second hook dynamically client.addHook(addedHook); @@ -198,18 +197,17 @@ describe('LDClient Hooks Integration', () => { { status: 'completed' } ); - expect(addedHook.beforeIdentify).toHaveBeenCalledTimes(1); - expect(addedHook.beforeIdentify).toHaveBeenCalledWith( - expect.objectContaining({ context: identifyContext }), - initialData - ); - expect(addedHook.afterIdentify).toHaveBeenCalledTimes(1); - expect(addedHook.afterIdentify).toHaveBeenCalledWith( - expect.objectContaining({ context: identifyContext }), - initialData, // Assuming pass-through - { status: 'completed' } - ); - + expect(addedHook.beforeIdentify).toHaveBeenCalledTimes(1); + expect(addedHook.beforeIdentify).toHaveBeenCalledWith( + expect.objectContaining({ context: identifyContext }), + initialData + ); + expect(addedHook.afterIdentify).toHaveBeenCalledTimes(1); + expect(addedHook.afterIdentify).toHaveBeenCalledWith( + expect.objectContaining({ context: identifyContext }), + initialData, // Assuming pass-through + { status: 'completed' } + ); // Check evaluation hooks for BOTH hooks [initialHook, addedHook].forEach(hook => { @@ -227,4 +225,4 @@ describe('LDClient Hooks Integration', () => { }); }); }); -}); \ No newline at end of file +}); From cf9b19ee94bf2439baec8986625ed0bf9afe0757 Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Wed, 9 Apr 2025 14:36:07 -0700 Subject: [PATCH 9/9] PR feedback. --- src/__tests__/utils-test.js | 32 +++++++++++++++++++++++++++++++- src/index.js | 4 ++-- src/utils.js | 21 +++++++++++++++++++++ typings.d.ts | 6 +++--- 4 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/__tests__/utils-test.js b/src/__tests__/utils-test.js index a83404f..c427abb 100644 --- a/src/__tests__/utils-test.js +++ b/src/__tests__/utils-test.js @@ -1,4 +1,4 @@ -import { appendUrlPath, getLDUserAgentString, wrapPromiseCallback } from '../utils'; +import { appendUrlPath, getLDUserAgentString, wrapPromiseCallback, once } from '../utils'; import * as stubPlatform from './stubPlatform'; @@ -63,4 +63,34 @@ describe('utils', () => { expect(ua).toEqual('stubClient/7.8.9'); }); }); + + it('when using once the original function is only called once', () => { + let count = 0; + const fn = once(() => { + count++; + return count; + }); + + expect(fn()).toBe(1); + expect(fn()).toBe(1); + expect(fn()).toBe(1); + expect(count).toBe(1); + }); + + it('once works with async functions', async () => { + let count = 0; + const fn = once(async () => { + count++; + return count; + }); + + const result1 = await fn(); + const result2 = await fn(); + const result3 = await fn(); + + expect(result1).toBe(1); + expect(result2).toBe(1); + expect(result3).toBe(1); + expect(count).toBe(1); + }); }); diff --git a/src/index.js b/src/index.js index 619ab3f..26327f9 100644 --- a/src/index.js +++ b/src/index.js @@ -265,7 +265,7 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) { .then(() => anonymousContextProcessor.processContext(context)) .then(verifyContext) .then(context => { - afterIdentify = hookRunner.identify(context, undefined); + afterIdentify = utils.once(hookRunner.identify(context, undefined)); return context; }) .then(validatedContext => @@ -687,7 +687,7 @@ function initialize(env, context, specifiedOptions, platform, extraOptionDefs) { .processContext(context) .then(verifyContext) .then(context => { - afterIdentify = hookRunner.identify(context, undefined); + afterIdentify = utils.once(hookRunner.identify(context, undefined)); return context; }) .then(validatedContext => { diff --git a/src/utils.js b/src/utils.js index ee1a424..a26d7e2 100644 --- a/src/utils.js +++ b/src/utils.js @@ -144,6 +144,26 @@ function sanitizeContext(context) { return newContext || context; } +/** + * Creates a function that will invoke the provided function only once. + * + * If the function returns a value, then that returned value will be re-used for subsequent invocations. + * + * @param {Function} func The function to restrict. + * @returns {Function} Returns the new restricted function. + */ +function once(func) { + let called = false; + let result; + return function(...args) { + if (!called) { + called = true; + result = func.apply(this, args); + } + return result; + }; +} + module.exports = { appendUrlPath, base64URLEncode, @@ -158,4 +178,5 @@ module.exports = { transformValuesToVersionedValues, transformVersionedValuesToValues, wrapPromiseCallback, + once, }; diff --git a/typings.d.ts b/typings.d.ts index e43b106..35a17d0 100644 --- a/typings.d.ts +++ b/typings.d.ts @@ -183,7 +183,7 @@ declare module 'launchdarkly-js-sdk-common' { * 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 + * @param hookContext Contains information about the identify operation 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. @@ -200,7 +200,7 @@ declare module 'launchdarkly-js-sdk-common' { * 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 + * @param hookContext Contains information about the identify operation 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. @@ -467,7 +467,7 @@ declare module 'launchdarkly-js-sdk-common' { * * Example: * ```typescript - * import { init } from '@launchdarkly/node-server-sdk'; + * import { initialize } from 'launchdarkly-js-client-sdk'; * import { TheHook } from '@launchdarkly/some-hook'; * * const client = init('my-sdk-key', { hooks: [new TheHook()] });