From e27f298a751e70ba500f50d1aaafcd373fe3fd3b Mon Sep 17 00:00:00 2001 From: Ryan Lamb <4955475+kinyoklion@users.noreply.github.com> Date: Tue, 2 Apr 2024 13:00:34 -0700 Subject: [PATCH] chore: Refactor hook execution. (#417) This code lifts the hook running code out of the client. --- .../__tests__/LDClient.hooks.test.ts | 227 +------------ .../__tests__/hooks/HookRunner.test.ts | 291 +++++++++++++++++ .../sdk-server/__tests__/hooks/TestHook.ts | 67 ++++ .../shared/sdk-server/src/LDClientImpl.ts | 299 ++++++------------ .../shared/sdk-server/src/hooks/HookRunner.ts | 147 +++++++++ 5 files changed, 605 insertions(+), 426 deletions(-) create mode 100644 packages/shared/sdk-server/__tests__/hooks/HookRunner.test.ts create mode 100644 packages/shared/sdk-server/__tests__/hooks/TestHook.ts create mode 100644 packages/shared/sdk-server/src/hooks/HookRunner.ts diff --git a/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts b/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts index b69de7083f..efee536ab8 100644 --- a/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts +++ b/packages/shared/sdk-server/__tests__/LDClient.hooks.test.ts @@ -1,78 +1,14 @@ import { basicPlatform } from '@launchdarkly/private-js-mocks'; -import { integrations, LDClientImpl, LDEvaluationDetail, LDMigrationStage } from '../src'; +import { LDClientImpl, LDMigrationStage } from '../src'; import Reasons from '../src/evaluation/Reasons'; import TestData from '../src/integrations/test_data/TestData'; -import TestLogger, { LogLevel } from './Logger'; +import { TestHook } from './hooks/TestHook'; +import TestLogger from './Logger'; import makeCallbacks from './makeCallbacks'; const defaultUser = { kind: 'user', key: 'user-key' }; -type EvalCapture = { - method: string; - hookContext: integrations.EvaluationSeriesContext; - hookData: integrations.EvaluationSeriesData; - detail?: LDEvaluationDetail; -}; - -class TestHook implements integrations.Hook { - captureBefore: EvalCapture[] = []; - captureAfter: EvalCapture[] = []; - - getMetadataImpl: () => integrations.HookMetadata = () => ({ name: 'LaunchDarkly Test Hook' }); - - getMetadata(): integrations.HookMetadata { - return this.getMetadataImpl(); - } - - verifyBefore( - hookContext: integrations.EvaluationSeriesContext, - data: integrations.EvaluationSeriesData, - ) { - expect(this.captureBefore).toHaveLength(1); - expect(this.captureBefore[0].hookContext).toEqual(hookContext); - expect(this.captureBefore[0].hookData).toEqual(data); - } - - verifyAfter( - hookContext: integrations.EvaluationSeriesContext, - data: integrations.EvaluationSeriesData, - detail: LDEvaluationDetail, - ) { - expect(this.captureAfter).toHaveLength(1); - expect(this.captureAfter[0].hookContext).toEqual(hookContext); - expect(this.captureAfter[0].hookData).toEqual(data); - expect(this.captureAfter[0].detail).toEqual(detail); - } - - beforeEvalImpl: ( - hookContext: integrations.EvaluationSeriesContext, - data: integrations.EvaluationSeriesData, - ) => integrations.EvaluationSeriesData = (_hookContext, data) => data; - - afterEvalImpl: ( - hookContext: integrations.EvaluationSeriesContext, - data: integrations.EvaluationSeriesData, - detail: LDEvaluationDetail, - ) => integrations.EvaluationSeriesData = (_hookContext, data, _detail) => data; - - beforeEvaluation?( - hookContext: integrations.EvaluationSeriesContext, - data: integrations.EvaluationSeriesData, - ): integrations.EvaluationSeriesData { - this.captureBefore.push({ method: 'beforeEvaluation', hookContext, hookData: data }); - return this.beforeEvalImpl(hookContext, data); - } - afterEvaluation?( - hookContext: integrations.EvaluationSeriesContext, - data: integrations.EvaluationSeriesData, - detail: LDEvaluationDetail, - ): integrations.EvaluationSeriesData { - this.captureAfter.push({ method: 'afterEvaluation', hookContext, hookData: data, detail }); - return this.afterEvalImpl(hookContext, data, detail); - } -} - describe('given an LDClient with test data', () => { let client: LDClientImpl; let td: TestData; @@ -409,105 +345,6 @@ describe('given an LDClient with test data', () => { }, ); }); - - it('propagates data between stages', async () => { - testHook.beforeEvalImpl = ( - _hookContext: integrations.EvaluationSeriesContext, - data: integrations.EvaluationSeriesData, - ) => ({ - ...data, - added: 'added data', - }); - await client.variation('flagKey', defaultUser, false); - - testHook.verifyAfter( - { - flagKey: 'flagKey', - context: { ...defaultUser }, - defaultValue: false, - method: 'LDClient.variation', - }, - { added: 'added data' }, - { - value: false, - reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' }, - variationIndex: null, - }, - ); - }); - - it('handles an exception thrown in beforeEvaluation', async () => { - testHook.beforeEvalImpl = ( - _hookContext: integrations.EvaluationSeriesContext, - _data: integrations.EvaluationSeriesData, - ) => { - throw new Error('bad hook'); - }; - await client.variation('flagKey', defaultUser, false); - logger.expectMessages([ - { - level: LogLevel.Error, - matches: - /An error was encountered in "beforeEvaluation" of the "LaunchDarkly Test Hook" hook: Error: bad hook/, - }, - ]); - }); - - it('handles an exception thrown in afterEvaluation', async () => { - testHook.afterEvalImpl = () => { - throw new Error('bad hook'); - }; - await client.variation('flagKey', defaultUser, false); - logger.expectMessages([ - { - level: LogLevel.Error, - matches: - /An error was encountered in "afterEvaluation" of the "LaunchDarkly Test Hook" hook: Error: bad hook/, - }, - ]); - }); - - it('handles exception getting the hook metadata', async () => { - testHook.getMetadataImpl = () => { - throw new Error('bad hook'); - }; - await client.variation('flagKey', defaultUser, false); - - logger.expectMessages([ - { - level: LogLevel.Error, - matches: /Exception thrown getting metadata for hook. Unable to get hook name./, - }, - ]); - }); - - it('uses unknown name when the name cannot be accessed', async () => { - testHook.beforeEvalImpl = ( - _hookContext: integrations.EvaluationSeriesContext, - _data: integrations.EvaluationSeriesData, - ) => { - throw new Error('bad hook'); - }; - testHook.getMetadataImpl = () => { - throw new Error('bad hook'); - }; - testHook.afterEvalImpl = () => { - throw new Error('bad hook'); - }; - await client.variation('flagKey', defaultUser, false); - logger.expectMessages([ - { - level: LogLevel.Error, - matches: - /An error was encountered in "afterEvaluation" of the "unknown hook" hook: Error: bad hook/, - }, - { - level: LogLevel.Error, - matches: - /An error was encountered in "beforeEvaluation" of the "unknown hook" hook: Error: bad hook/, - }, - ]); - }); }); it('can add a hook after initialization', async () => { @@ -555,61 +392,3 @@ it('can add a hook after initialization', async () => { }, ); }); - -it('executes hook stages in the specified order', async () => { - const beforeCalledOrder: string[] = []; - const afterCalledOrder: string[] = []; - - const hookA = new TestHook(); - hookA.beforeEvalImpl = (_context, data) => { - beforeCalledOrder.push('a'); - return data; - }; - - hookA.afterEvalImpl = (_context, data, _detail) => { - afterCalledOrder.push('a'); - return data; - }; - - const hookB = new TestHook(); - hookB.beforeEvalImpl = (_context, data) => { - beforeCalledOrder.push('b'); - return data; - }; - hookB.afterEvalImpl = (_context, data, _detail) => { - afterCalledOrder.push('b'); - return data; - }; - - const hookC = new TestHook(); - hookC.beforeEvalImpl = (_context, data) => { - beforeCalledOrder.push('c'); - return data; - }; - - hookC.afterEvalImpl = (_context, data, _detail) => { - afterCalledOrder.push('c'); - return data; - }; - - const logger = new TestLogger(); - const td = new TestData(); - const client = new LDClientImpl( - 'sdk-key', - basicPlatform, - { - updateProcessor: td.getFactory(), - sendEvents: false, - logger, - hooks: [hookA, hookB], - }, - makeCallbacks(true), - ); - - await client.waitForInitialization(); - client.addHook(hookC); - await client.variation('flagKey', defaultUser, false); - - expect(beforeCalledOrder).toEqual(['a', 'b', 'c']); - expect(afterCalledOrder).toEqual(['c', 'b', 'a']); -}); diff --git a/packages/shared/sdk-server/__tests__/hooks/HookRunner.test.ts b/packages/shared/sdk-server/__tests__/hooks/HookRunner.test.ts new file mode 100644 index 0000000000..b72a184f87 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/hooks/HookRunner.test.ts @@ -0,0 +1,291 @@ +import { integrations } from '../../src'; +import Reasons from '../../src/evaluation/Reasons'; +import HookRunner from '../../src/hooks/HookRunner'; +import TestLogger, { LogLevel } from '../Logger'; +import { TestHook } from './TestHook'; + +const defaultUser = { kind: 'user', key: 'user-key' }; + +describe('given a HookRunner', () => { + let testHook: TestHook; + let logger: TestLogger; + let runner: HookRunner; + + beforeEach(async () => { + logger = new TestLogger(); + testHook = new TestHook(); + runner = new HookRunner(logger, [testHook]); + }); + + it('propagates data between stages', async () => { + testHook.beforeEvalImpl = ( + _hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, + ) => ({ + ...data, + added: 'added data', + }); + + await runner.withEvaluationSeries( + 'flagKey', + defaultUser, + false, + 'LDClient.variation', + async () => ({ + value: false, + reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' }, + variationIndex: null, + }), + ); + + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.variation', + }, + { added: 'added data' }, + { + value: false, + reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' }, + variationIndex: null, + }, + ); + }); + + it('handles an exception thrown in beforeEvaluation', async () => { + testHook.beforeEvalImpl = ( + _hookContext: integrations.EvaluationSeriesContext, + _data: integrations.EvaluationSeriesData, + ) => { + throw new Error('bad hook'); + }; + + await runner.withEvaluationSeries( + 'flagKey', + defaultUser, + false, + 'LDClient.variation', + async () => ({ + value: false, + reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' }, + variationIndex: null, + }), + ); + + logger.expectMessages([ + { + level: LogLevel.Error, + matches: + /An error was encountered in "beforeEvaluation" of the "LaunchDarkly Test Hook" hook: Error: bad hook/, + }, + ]); + }); + + it('handles an exception thrown in afterEvaluation', async () => { + testHook.afterEvalImpl = () => { + throw new Error('bad hook'); + }; + await runner.withEvaluationSeries( + 'flagKey', + defaultUser, + false, + 'LDClient.variation', + async () => ({ + value: false, + reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' }, + variationIndex: null, + }), + ); + logger.expectMessages([ + { + level: LogLevel.Error, + matches: + /An error was encountered in "afterEvaluation" of the "LaunchDarkly Test Hook" hook: Error: bad hook/, + }, + ]); + }); + + it('handles exception getting the hook metadata', async () => { + testHook.getMetadataImpl = () => { + throw new Error('bad hook'); + }; + await runner.withEvaluationSeries( + 'flagKey', + defaultUser, + false, + 'LDClient.variation', + async () => ({ + value: false, + reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' }, + variationIndex: null, + }), + ); + + logger.expectMessages([ + { + level: LogLevel.Error, + matches: /Exception thrown getting metadata for hook. Unable to get hook name./, + }, + ]); + }); + + it('uses unknown name when the name cannot be accessed', async () => { + testHook.beforeEvalImpl = ( + _hookContext: integrations.EvaluationSeriesContext, + _data: integrations.EvaluationSeriesData, + ) => { + throw new Error('bad hook'); + }; + testHook.getMetadataImpl = () => { + throw new Error('bad hook'); + }; + testHook.afterEvalImpl = () => { + throw new Error('bad hook'); + }; + await runner.withEvaluationSeries( + 'flagKey', + defaultUser, + false, + 'LDClient.variation', + async () => ({ + value: false, + reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' }, + variationIndex: null, + }), + ); + logger.expectMessages([ + { + level: LogLevel.Error, + matches: + /An error was encountered in "afterEvaluation" of the "unknown hook" hook: Error: bad hook/, + }, + { + level: LogLevel.Error, + matches: + /An error was encountered in "beforeEvaluation" of the "unknown hook" hook: Error: bad hook/, + }, + ]); + }); +}); + +it('can add a hook after initialization', async () => { + const logger = new TestLogger(); + const runner = new HookRunner(logger, []); + + const testHook = new TestHook(); + runner.addHook(testHook); + + await runner.withEvaluationSeries( + 'flagKey', + defaultUser, + false, + 'LDClient.variation', + async () => ({ + value: true, + reason: { kind: 'FALLTHROUGH' }, + variationIndex: 0, + }), + ); + testHook.verifyBefore( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.variation', + }, + {}, + ); + testHook.verifyAfter( + { + flagKey: 'flagKey', + context: { ...defaultUser }, + defaultValue: false, + method: 'LDClient.variation', + }, + {}, + { + value: true, + reason: Reasons.Fallthrough, + variationIndex: 0, + }, + ); +}); + +it('executes hook stages in the specified order', async () => { + const beforeCalledOrder: string[] = []; + const afterCalledOrder: string[] = []; + + const hookA = new TestHook(); + hookA.beforeEvalImpl = (_context, data) => { + beforeCalledOrder.push('a'); + return data; + }; + + hookA.afterEvalImpl = (_context, data, _detail) => { + afterCalledOrder.push('a'); + return data; + }; + + const hookB = new TestHook(); + hookB.beforeEvalImpl = (_context, data) => { + beforeCalledOrder.push('b'); + return data; + }; + hookB.afterEvalImpl = (_context, data, _detail) => { + afterCalledOrder.push('b'); + return data; + }; + + const hookC = new TestHook(); + hookC.beforeEvalImpl = (_context, data) => { + beforeCalledOrder.push('c'); + return data; + }; + + hookC.afterEvalImpl = (_context, data, _detail) => { + afterCalledOrder.push('c'); + return data; + }; + + const logger = new TestLogger(); + const runner = new HookRunner(logger, [hookA, hookB]); + + const testHook = new TestHook(); + runner.addHook(testHook); + runner.addHook(hookC); + await runner.withEvaluationSeries( + 'flagKey', + defaultUser, + false, + 'LDClient.variation', + async () => ({ + value: false, + reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' }, + variationIndex: null, + }), + ); + + expect(beforeCalledOrder).toEqual(['a', 'b', 'c']); + expect(afterCalledOrder).toEqual(['c', 'b', 'a']); +}); + +it('can return custom data', async () => { + const runner = new HookRunner(undefined, []); + const res = await runner.withEvaluationSeriesExtraDetail( + 'flagKey', + defaultUser, + false, + 'LDClient.variation', + async () => ({ + detail: { + value: false, + reason: { kind: 'ERROR', errorKind: 'FLAG_NOT_FOUND' }, + variationIndex: null, + }, + test: 'test', + }), + ); + expect(res.test).toEqual('test'); +}); diff --git a/packages/shared/sdk-server/__tests__/hooks/TestHook.ts b/packages/shared/sdk-server/__tests__/hooks/TestHook.ts new file mode 100644 index 0000000000..6ba2555b89 --- /dev/null +++ b/packages/shared/sdk-server/__tests__/hooks/TestHook.ts @@ -0,0 +1,67 @@ +import { integrations, LDEvaluationDetail } from '../../src'; + +export type EvalCapture = { + method: string; + hookContext: integrations.EvaluationSeriesContext; + hookData: integrations.EvaluationSeriesData; + detail?: LDEvaluationDetail; +}; + +export class TestHook implements integrations.Hook { + captureBefore: EvalCapture[] = []; + captureAfter: EvalCapture[] = []; + + getMetadataImpl: () => integrations.HookMetadata = () => ({ name: 'LaunchDarkly Test Hook' }); + + getMetadata(): integrations.HookMetadata { + return this.getMetadataImpl(); + } + + verifyBefore( + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, + ) { + expect(this.captureBefore).toHaveLength(1); + expect(this.captureBefore[0].hookContext).toEqual(hookContext); + expect(this.captureBefore[0].hookData).toEqual(data); + } + + verifyAfter( + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, + detail: LDEvaluationDetail, + ) { + expect(this.captureAfter).toHaveLength(1); + expect(this.captureAfter[0].hookContext).toEqual(hookContext); + expect(this.captureAfter[0].hookData).toEqual(data); + expect(this.captureAfter[0].detail).toEqual(detail); + } + + beforeEvalImpl: ( + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, + ) => integrations.EvaluationSeriesData = (_hookContext, data) => data; + + afterEvalImpl: ( + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, + detail: LDEvaluationDetail, + ) => integrations.EvaluationSeriesData = (_hookContext, data, _detail) => data; + + beforeEvaluation?( + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, + ): integrations.EvaluationSeriesData { + this.captureBefore.push({ method: 'beforeEvaluation', hookContext, hookData: data }); + return this.beforeEvalImpl(hookContext, data); + } + + afterEvaluation?( + hookContext: integrations.EvaluationSeriesContext, + data: integrations.EvaluationSeriesData, + detail: LDEvaluationDetail, + ): integrations.EvaluationSeriesData { + this.captureAfter.push({ method: 'afterEvaluation', hookContext, hookData: data, detail }); + return this.afterEvalImpl(hookContext, data, detail); + } +} diff --git a/packages/shared/sdk-server/src/LDClientImpl.ts b/packages/shared/sdk-server/src/LDClientImpl.ts index 024e792370..fedbc171e3 100644 --- a/packages/shared/sdk-server/src/LDClientImpl.ts +++ b/packages/shared/sdk-server/src/LDClientImpl.ts @@ -24,7 +24,7 @@ import { LDMigrationVariation, LDOptions, } from './api'; -import { EvaluationSeriesContext, EvaluationSeriesData, Hook } from './api/integrations/Hook'; +import { Hook } from './api/integrations/Hook'; import { BigSegmentStoreMembership } from './api/interfaces'; import BigSegmentsManager from './BigSegmentsManager'; import BigSegmentStoreStatusProvider from './BigSegmentStatusProviderImpl'; @@ -43,6 +43,7 @@ import ContextDeduplicator from './events/ContextDeduplicator'; import EventFactory from './events/EventFactory'; import isExperiment from './events/isExperiment'; import FlagsStateBuilder from './FlagsStateBuilder'; +import HookRunner from './hooks/HookRunner'; import MigrationOpEventToInputEvent from './MigrationOpEventConversion'; import MigrationOpTracker from './MigrationOpTracker'; import Configuration from './options/Configuration'; @@ -79,10 +80,6 @@ const STRING_VARIATION_DETAIL_METHOD_NAME = 'LDClient.stringVariationDetail'; const JSON_VARIATION_DETAIL_METHOD_NAME = 'LDClient.jsonVariationDetail'; const VARIATION_METHOD_DETAIL_NAME = 'LDClient.variationDetail'; -const BEFORE_EVALUATION_STAGE_NAME = 'beforeEvaluation'; -const AFTER_EVALUATION_STAGE_NAME = 'afterEvaluation'; -const UNKNOWN_HOOK_NAME = 'unknown hook'; - /** * @ignore */ @@ -123,7 +120,7 @@ export default class LDClientImpl implements LDClient { private diagnosticsManager?: internal.DiagnosticsManager; - private hooks: Hook[]; + private hookRunner: HookRunner; /** * Intended for use by platform specific client implementations. @@ -148,7 +145,7 @@ export default class LDClientImpl implements LDClient { const { onUpdate, hasEventListeners } = callbacks; const config = new Configuration(options, internalOptions); - this.hooks = config.hooks || []; + this.hookRunner = new HookRunner(config.logger, config.hooks || []); if (!sdkKey && !config.offline) { throw new Error('You must configure the client with an SDK key'); @@ -291,21 +288,23 @@ export default class LDClientImpl implements LDClient { defaultValue: any, callback?: (err: any, res: any) => void, ): Promise { - return this.withHooks( - key, - context, - defaultValue, - VARIATION_METHOD_NAME, - () => - new Promise((resolve) => { - this.evaluateIfPossible(key, context, defaultValue, this.eventFactoryDefault, (res) => { - resolve(res.detail); - }); - }), - ).then((detail) => { - callback?.(null, detail.value); - return detail.value; - }); + return this.hookRunner + .withEvaluationSeries( + key, + context, + defaultValue, + VARIATION_METHOD_NAME, + () => + new Promise((resolve) => { + this.evaluateIfPossible(key, context, defaultValue, this.eventFactoryDefault, (res) => { + resolve(res.detail); + }); + }), + ) + .then((detail) => { + callback?.(null, detail.value); + return detail.value; + }); } variationDetail( @@ -314,7 +313,7 @@ export default class LDClientImpl implements LDClient { defaultValue: any, callback?: (err: any, res: LDEvaluationDetail) => void, ): Promise { - return this.withHooks( + return this.hookRunner.withEvaluationSeries( key, context, defaultValue, @@ -343,7 +342,7 @@ export default class LDClientImpl implements LDClient { methodName: string, typeChecker: (value: unknown) => [boolean, string], ): Promise { - return this.withHooks( + return this.hookRunner.withEvaluationSeries( key, context, defaultValue, @@ -409,18 +408,20 @@ export default class LDClientImpl implements LDClient { } jsonVariation(key: string, context: LDContext, defaultValue: unknown): Promise { - return this.withHooks( - key, - context, - defaultValue, - JSON_VARIATION_METHOD_NAME, - () => - new Promise((resolve) => { - this.evaluateIfPossible(key, context, defaultValue, this.eventFactoryDefault, (res) => { - resolve(res.detail); - }); - }), - ).then((detail) => detail.value); + return this.hookRunner + .withEvaluationSeries( + key, + context, + defaultValue, + JSON_VARIATION_METHOD_NAME, + () => + new Promise((resolve) => { + this.evaluateIfPossible(key, context, defaultValue, this.eventFactoryDefault, (res) => { + resolve(res.detail); + }); + }), + ) + .then((detail) => detail.value); } boolVariationDetail( @@ -473,7 +474,7 @@ export default class LDClientImpl implements LDClient { context: LDContext, defaultValue: unknown, ): Promise> { - return this.withHooks( + return this.hookRunner.withEvaluationSeries( key, context, defaultValue, @@ -499,73 +500,60 @@ export default class LDClientImpl implements LDClient { defaultValue: LDMigrationStage, ): Promise<{ detail: LDEvaluationDetail; migration: LDMigrationVariation }> { const convertedContext = Context.fromLDContext(context); - return new Promise<{ detail: LDEvaluationDetail; migration: LDMigrationVariation }>( - (resolve) => { - this.evaluateIfPossible( - key, - context, - defaultValue, - this.eventFactoryWithReasons, - ({ detail }, flag) => { - const contextKeys = convertedContext.valid ? convertedContext.kindsAndKeys : {}; - const checkRatio = flag?.migration?.checkRatio; - const samplingRatio = flag?.samplingRatio; - - if (!IsMigrationStage(detail.value)) { - const error = new Error( - `Unrecognized MigrationState for "${key}"; returning default value.`, - ); - this.onError(error); - const reason = { - kind: 'ERROR', - errorKind: ErrorKinds.WrongType, - }; - resolve({ - detail: { - value: defaultValue, - reason, - }, - migration: { - value: defaultValue, - tracker: new MigrationOpTracker( - key, - contextKeys, - defaultValue, - defaultValue, - reason, - checkRatio, - undefined, - flag?.version, - samplingRatio, - this.logger, - ), - }, - }); - return; - } + const res = await new Promise<{ detail: LDEvaluationDetail; flag?: Flag }>((resolve) => { + this.evaluateIfPossible( + key, + context, + defaultValue, + this.eventFactoryWithReasons, + ({ detail }, flag) => { + if (!IsMigrationStage(detail.value)) { + const error = new Error( + `Unrecognized MigrationState for "${key}"; returning default value.`, + ); + this.onError(error); + const reason = { + kind: 'ERROR', + errorKind: ErrorKinds.WrongType, + }; resolve({ - detail, - migration: { - value: detail.value as LDMigrationStage, - tracker: new MigrationOpTracker( - key, - contextKeys, - defaultValue, - detail.value, - detail.reason, - checkRatio, - // Can be null for compatibility reasons. - detail.variationIndex === null ? undefined : detail.variationIndex, - flag?.version, - samplingRatio, - this.logger, - ), + detail: { + value: defaultValue, + reason, }, + flag, }); - }, - ); + return; + } + resolve({ detail, flag }); + }, + ); + }); + + const { detail, flag } = res; + const contextKeys = convertedContext.valid ? convertedContext.kindsAndKeys : {}; + const checkRatio = flag?.migration?.checkRatio; + const samplingRatio = flag?.samplingRatio; + + return { + detail, + migration: { + value: detail.value as LDMigrationStage, + tracker: new MigrationOpTracker( + key, + contextKeys, + defaultValue, + detail.value, + detail.reason, + checkRatio, + // Can be null for compatibility reasons. + detail.variationIndex === null ? undefined : detail.variationIndex, + flag?.version, + samplingRatio, + this.logger, + ), }, - ); + }; } async migrationVariation( @@ -573,11 +561,14 @@ export default class LDClientImpl implements LDClient { context: LDContext, defaultValue: LDMigrationStage, ): Promise { - const { hooks, hookContext }: { hooks: Hook[]; hookContext: EvaluationSeriesContext } = - this.prepareHooks(key, context, defaultValue, MIGRATION_VARIATION_METHOD_NAME); - const hookData = this.executeBeforeEvaluation(hooks, hookContext); - const res = await this.migrationVariationInternal(key, context, defaultValue); - this.executeAfterEvaluation(hooks, hookContext, hookData, res.detail); + const res = await this.hookRunner.withEvaluationSeriesExtraDetail( + key, + context, + defaultValue, + MIGRATION_VARIATION_METHOD_NAME, + () => this.migrationVariationInternal(key, context, defaultValue), + ); + return res.migration; } @@ -727,7 +718,7 @@ export default class LDClientImpl implements LDClient { } addHook(hook: Hook): void { - this.hooks.push(hook); + this.hookRunner.addHook(hook); } private variationInternal( @@ -867,100 +858,4 @@ export default class LDClientImpl implements LDClient { this.onReady(); } } - - private async withHooks( - key: string, - context: LDContext, - defaultValue: unknown, - methodName: string, - method: () => Promise, - ): Promise { - if (this.hooks.length === 0) { - return method(); - } - const { hooks, hookContext }: { hooks: Hook[]; hookContext: EvaluationSeriesContext } = - this.prepareHooks(key, context, defaultValue, methodName); - const hookData = this.executeBeforeEvaluation(hooks, hookContext); - const result = await method(); - this.executeAfterEvaluation(hooks, hookContext, hookData, result); - return result; - } - - private tryExecuteStage( - method: string, - hookName: string, - stage: () => EvaluationSeriesData, - ): EvaluationSeriesData { - try { - return stage(); - } catch (err) { - this.logger?.error( - `An error was encountered in "${method}" of the "${hookName}" hook: ${err}`, - ); - return {}; - } - } - - private hookName(hook?: Hook): string { - try { - return hook?.getMetadata().name ?? UNKNOWN_HOOK_NAME; - } catch { - this.logger?.error(`Exception thrown getting metadata for hook. Unable to get hook name.`); - return UNKNOWN_HOOK_NAME; - } - } - - private executeAfterEvaluation( - 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] ?? {}; - this.tryExecuteStage( - AFTER_EVALUATION_STAGE_NAME, - this.hookName(hook), - () => hook?.afterEvaluation?.(hookContext, data, result) ?? {}, - ); - } - } - - private executeBeforeEvaluation( - hooks: Hook[], - hookContext: EvaluationSeriesContext, - ): EvaluationSeriesData[] { - return hooks.map((hook) => - this.tryExecuteStage( - BEFORE_EVALUATION_STAGE_NAME, - this.hookName(hook), - () => hook?.beforeEvaluation?.(hookContext, {}) ?? {}, - ), - ); - } - - private prepareHooks( - key: string, - context: LDContext, - defaultValue: unknown, - methodName: string, - ): { - hooks: Hook[]; - hookContext: EvaluationSeriesContext; - } { - // Copy the hooks to use a consistent set during evaluation. Hooks could be added and we want - // to ensure all correct stages for any give hook execute. Not for instance the afterEvaluation - // stage without beforeEvaluation having been called on that hook. - const hooks: Hook[] = [...this.hooks]; - const hookContext: EvaluationSeriesContext = { - flagKey: key, - context, - defaultValue, - method: methodName, - }; - return { hooks, hookContext }; - } } diff --git a/packages/shared/sdk-server/src/hooks/HookRunner.ts b/packages/shared/sdk-server/src/hooks/HookRunner.ts new file mode 100644 index 0000000000..5e9e531eda --- /dev/null +++ b/packages/shared/sdk-server/src/hooks/HookRunner.ts @@ -0,0 +1,147 @@ +import { LDContext, LDEvaluationDetail, LDLogger } from '@launchdarkly/js-sdk-common'; + +import { EvaluationSeriesContext, EvaluationSeriesData, Hook } from '../integrations'; + +const BEFORE_EVALUATION_STAGE_NAME = 'beforeEvaluation'; +const AFTER_EVALUATION_STAGE_NAME = 'afterEvaluation'; +const UNKNOWN_HOOK_NAME = 'unknown hook'; + +export default class HookRunner { + private readonly hooks: Hook[] = []; + + constructor( + private readonly logger: LDLogger | undefined, + hooks: Hook[], + ) { + this.hooks.push(...hooks); + } + + public async withEvaluationSeries( + key: string, + context: LDContext, + defaultValue: unknown, + methodName: string, + method: () => Promise, + ): Promise { + // This early return is here to avoid the extra async/await associated with + // using withHooksDataWithDetail. + if (this.hooks.length === 0) { + return method(); + } + + return this.withEvaluationSeriesExtraDetail( + key, + context, + defaultValue, + methodName, + async () => { + const detail = await method(); + return { detail }; + }, + ).then(({ detail }) => detail); + } + + /** + * This function allows extra information to be returned with the detail for situations like + * migrations where a tracker is returned with the detail. + */ + public async withEvaluationSeriesExtraDetail( + key: string, + context: LDContext, + defaultValue: unknown, + methodName: string, + method: () => Promise<{ detail: LDEvaluationDetail; [index: string]: any }>, + ): Promise<{ detail: LDEvaluationDetail; [index: string]: any }> { + if (this.hooks.length === 0) { + return method(); + } + const { hooks, hookContext }: { hooks: Hook[]; hookContext: EvaluationSeriesContext } = + this.prepareHooks(key, context, defaultValue, methodName); + const hookData = this.executeBeforeEvaluation(hooks, hookContext); + const result = await method(); + this.executeAfterEvaluation(hooks, hookContext, hookData, result.detail); + return result; + } + + private tryExecuteStage( + method: string, + hookName: string, + stage: () => EvaluationSeriesData, + ): EvaluationSeriesData { + try { + return stage(); + } catch (err) { + this.logger?.error( + `An error was encountered in "${method}" of the "${hookName}" hook: ${err}`, + ); + return {}; + } + } + + private hookName(hook?: Hook): string { + try { + return hook?.getMetadata().name ?? UNKNOWN_HOOK_NAME; + } catch { + this.logger?.error(`Exception thrown getting metadata for hook. Unable to get hook name.`); + return UNKNOWN_HOOK_NAME; + } + } + + private executeAfterEvaluation( + 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] ?? {}; + this.tryExecuteStage( + AFTER_EVALUATION_STAGE_NAME, + this.hookName(hook), + () => hook?.afterEvaluation?.(hookContext, data, result) ?? {}, + ); + } + } + + private executeBeforeEvaluation( + hooks: Hook[], + hookContext: EvaluationSeriesContext, + ): EvaluationSeriesData[] { + return hooks.map((hook) => + this.tryExecuteStage( + BEFORE_EVALUATION_STAGE_NAME, + this.hookName(hook), + () => hook?.beforeEvaluation?.(hookContext, {}) ?? {}, + ), + ); + } + + private prepareHooks( + key: string, + context: LDContext, + defaultValue: unknown, + methodName: string, + ): { + hooks: Hook[]; + hookContext: EvaluationSeriesContext; + } { + // Copy the hooks to use a consistent set during evaluation. Hooks could be added and we want + // to ensure all correct stages for any give hook execute. Not for instance the afterEvaluation + // stage without beforeEvaluation having been called on that hook. + const hooks: Hook[] = [...this.hooks]; + const hookContext: EvaluationSeriesContext = { + flagKey: key, + context, + defaultValue, + method: methodName, + }; + return { hooks, hookContext }; + } + + addHook(hook: Hook): void { + this.hooks.push(hook); + } +}