diff --git a/packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts b/packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts index 17a88b61fc..b8491cc4c1 100644 --- a/packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts +++ b/packages/sdk/browser/contract-tests/entity/src/ClientEntity.ts @@ -190,9 +190,7 @@ function makeSdkConfig(options: SDKConfigParams, tag: string) { } if (options.hooks) { - cf.hooks = options.hooks.hooks.map( - (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors), - ); + cf.hooks = TestHook.forClient(options.hooks.hooks); } cf.fetchGoals = false; diff --git a/packages/sdk/browser/contract-tests/suppressions.txt b/packages/sdk/browser/contract-tests/suppressions.txt index 89c15dc29e..a4b3ab831b 100644 --- a/packages/sdk/browser/contract-tests/suppressions.txt +++ b/packages/sdk/browser/contract-tests/suppressions.txt @@ -15,6 +15,4 @@ tags/stream requests/{"applicationId":"._-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKL tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":null} tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":""} tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":"._-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ012345678"} -tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":"________________________________________________________________"} -hooks/evaluation/executes beforeEvaluation hooks in registration order -hooks/track/executes afterTrack hooks in registration order \ No newline at end of file +tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":"________________________________________________________________"} \ No newline at end of file diff --git a/packages/sdk/electron/contract-tests/entity/src/ClientEntity.ts b/packages/sdk/electron/contract-tests/entity/src/ClientEntity.ts index 657301ae71..e966cb5596 100644 --- a/packages/sdk/electron/contract-tests/entity/src/ClientEntity.ts +++ b/packages/sdk/electron/contract-tests/entity/src/ClientEntity.ts @@ -87,9 +87,7 @@ function makeSdkConfig(options: SDKConfigParams, tag: string) { } if (options.hooks) { - cf.hooks = options.hooks.hooks.map( - (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors), - ); + cf.hooks = TestHook.forClient(options.hooks.hooks); } if (options.tls) { diff --git a/packages/sdk/electron/contract-tests/suppressions.txt b/packages/sdk/electron/contract-tests/suppressions.txt index 2f16ccf42c..e69de29bb2 100644 --- a/packages/sdk/electron/contract-tests/suppressions.txt +++ b/packages/sdk/electron/contract-tests/suppressions.txt @@ -1,2 +0,0 @@ -hooks/evaluation/executes beforeEvaluation hooks in registration order -hooks/track/executes afterTrack hooks in registration order \ No newline at end of file diff --git a/packages/sdk/react-native/contract-tests/entity/src/ClientEntity.ts b/packages/sdk/react-native/contract-tests/entity/src/ClientEntity.ts index 19d87944d9..2ff7f5c3ac 100644 --- a/packages/sdk/react-native/contract-tests/entity/src/ClientEntity.ts +++ b/packages/sdk/react-native/contract-tests/entity/src/ClientEntity.ts @@ -77,9 +77,7 @@ function makeSdkConfig(options: SDKConfigParams, tag: string) { } if (options.hooks) { - cf.hooks = options.hooks.hooks.map( - (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors), - ); + cf.hooks = TestHook.forClient(options.hooks.hooks); } return cf; diff --git a/packages/sdk/react/contract-tests/app/ClientEntity.ts b/packages/sdk/react/contract-tests/app/ClientEntity.ts index 41f4a88572..a7273488bc 100644 --- a/packages/sdk/react/contract-tests/app/ClientEntity.ts +++ b/packages/sdk/react/contract-tests/app/ClientEntity.ts @@ -70,9 +70,7 @@ export function makeSdkConfig(options: SDKConfigParams, tag: string): LDOptions } if (options.hooks) { - cf.hooks = options.hooks.hooks.map( - (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors), - ); + cf.hooks = TestHook.forClient(options.hooks.hooks); } cf.fetchGoals = false; diff --git a/packages/sdk/react/contract-tests/suppressions.txt b/packages/sdk/react/contract-tests/suppressions.txt index 0b8ddcb95a..18abe27603 100644 --- a/packages/sdk/react/contract-tests/suppressions.txt +++ b/packages/sdk/react/contract-tests/suppressions.txt @@ -16,5 +16,3 @@ tags/stream requests/{"applicationId":"_________________________________________ tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":""} tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":"._-abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ012345678"} tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":"________________________________________________________________"} -hooks/evaluation/executes beforeEvaluation hooks in registration order -hooks/track/executes afterTrack hooks in registration order diff --git a/packages/sdk/server-node/contract-tests/src/sdkClientEntity.ts b/packages/sdk/server-node/contract-tests/src/sdkClientEntity.ts index 9854507005..f45cdc888c 100644 --- a/packages/sdk/server-node/contract-tests/src/sdkClientEntity.ts +++ b/packages/sdk/server-node/contract-tests/src/sdkClientEntity.ts @@ -88,9 +88,7 @@ export function makeSdkConfig(options: ServerSDKConfigParams, tag: string): LDOp } if (options.hooks) { - cf.hooks = options.hooks.hooks.map( - (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors), - ); + cf.hooks = TestHook.forClient(options.hooks.hooks); } if (options.wrapper) { diff --git a/packages/sdk/server-node/contract-tests/testharness-suppressions-fdv2.txt b/packages/sdk/server-node/contract-tests/testharness-suppressions-fdv2.txt index bcb934d8d2..22c1f2830c 100644 --- a/packages/sdk/server-node/contract-tests/testharness-suppressions-fdv2.txt +++ b/packages/sdk/server-node/contract-tests/testharness-suppressions-fdv2.txt @@ -11,5 +11,3 @@ streaming/fdv2/reconnection state management/replaces previously known state streaming/fdv2/reconnection state management/updates previously known state streaming/fdv2/ignores model version streaming/fdv2/can discard partial events on errors -hooks/evaluation/executes beforeEvaluation hooks in registration order -hooks/track/executes afterTrack hooks in registration order diff --git a/packages/sdk/server-node/contract-tests/testharness-suppressions.txt b/packages/sdk/server-node/contract-tests/testharness-suppressions.txt index 555cedac1a..81013bcf9f 100644 --- a/packages/sdk/server-node/contract-tests/testharness-suppressions.txt +++ b/packages/sdk/server-node/contract-tests/testharness-suppressions.txt @@ -3,6 +3,4 @@ streaming/validation/drop and reconnect if stream event has well-formed JSON not streaming/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has no trailing slash/GET streaming/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has a trailing slash/GET polling/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has no trailing slash/GET -polling/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has a trailing slash/GET -hooks/evaluation/executes beforeEvaluation hooks in registration order -hooks/track/executes afterTrack hooks in registration order \ No newline at end of file +polling/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has a trailing slash/GET \ No newline at end of file diff --git a/packages/shared/sdk-client/__tests__/hooks/HookRunner.test.ts b/packages/shared/sdk-client/__tests__/hooks/HookRunner.test.ts index 7ecca84ff2..3a2c6a7288 100644 --- a/packages/shared/sdk-client/__tests__/hooks/HookRunner.test.ts +++ b/packages/shared/sdk-client/__tests__/hooks/HookRunner.test.ts @@ -421,6 +421,6 @@ describe('given a hook runner and test hook', () => { expect(afterIdentifyOrder).toEqual(['c', 'b', 'a']); // Verify track hooks order - expect(afterTrackOrder).toEqual(['c', 'b', 'a']); + expect(afterTrackOrder).toEqual(['a', 'b', 'c']); }); }); diff --git a/packages/shared/sdk-client/src/HookRunner.ts b/packages/shared/sdk-client/src/HookRunner.ts index 87bda82cdd..93459052c3 100644 --- a/packages/shared/sdk-client/src/HookRunner.ts +++ b/packages/shared/sdk-client/src/HookRunner.ts @@ -117,9 +117,7 @@ function executeAfterIdentify( } function executeAfterTrack(logger: LDLogger, hooks: Hook[], hookContext: TrackSeriesContext) { - // This iterates in reverse, versus reversing a shallow copy of the hooks, - // for efficiency. - for (let hookIndex = hooks.length - 1; hookIndex >= 0; hookIndex -= 1) { + for (let hookIndex = 0; hookIndex < hooks.length; hookIndex += 1) { const hook = hooks[hookIndex]; tryExecuteStage( logger, diff --git a/packages/tooling/contract-test-utils/src/client-side/TestHook.ts b/packages/tooling/contract-test-utils/src/client-side/TestHook.ts index b05b35da1c..64b4b74ef0 100644 --- a/packages/tooling/contract-test-utils/src/client-side/TestHook.ts +++ b/packages/tooling/contract-test-utils/src/client-side/TestHook.ts @@ -8,8 +8,15 @@ import { } from '@launchdarkly/js-client-sdk-common'; import { BaseTestHook } from '../shared/BaseTestHook.js'; +import { HookPostQueue } from '../shared/HookPostQueue.js'; +import { SDKConfigHookInstance } from '../types/ConfigParams.js'; export default class TestHook extends BaseTestHook implements Hook { + static forClient(configs: ReadonlyArray): TestHook[] { + const queue = new HookPostQueue(); + return configs.map((c) => new TestHook(c.name, c.callbackUri, c.data, c.errors, queue)); + } + protected async safePost(body: unknown): Promise { try { await fetch(this.endpoint, { @@ -52,7 +59,7 @@ export default class TestHook extends BaseTestHook implements Hook { if (this.hookErrors?.afterTrack) { throw new Error(this.hookErrors.afterTrack); } - this.safePost({ + this.enqueuePost({ trackSeriesContext: hookContext, stage: 'afterTrack', }); diff --git a/packages/tooling/contract-test-utils/src/server-side/TestHook.ts b/packages/tooling/contract-test-utils/src/server-side/TestHook.ts index e5c1f9de34..51b8d6f8aa 100644 --- a/packages/tooling/contract-test-utils/src/server-side/TestHook.ts +++ b/packages/tooling/contract-test-utils/src/server-side/TestHook.ts @@ -1,8 +1,15 @@ import { integrations, LDEvaluationDetail } from '@launchdarkly/js-server-sdk-common'; import { BaseTestHook } from '../shared/BaseTestHook.js'; +import { HookPostQueue } from '../shared/HookPostQueue.js'; +import { SDKConfigHookInstance } from '../types/ConfigParams.js'; export default class TestHook extends BaseTestHook implements integrations.Hook { + static forClient(configs: ReadonlyArray): TestHook[] { + const queue = new HookPostQueue(); + return configs.map((c) => new TestHook(c.name, c.callbackUri, c.data, c.errors, queue)); + } + protected async safePost(body: unknown): Promise { try { await fetch(this.endpoint, { diff --git a/packages/tooling/contract-test-utils/src/shared/BaseTestHook.ts b/packages/tooling/contract-test-utils/src/shared/BaseTestHook.ts index 724edb50b7..363770e0f0 100644 --- a/packages/tooling/contract-test-utils/src/shared/BaseTestHook.ts +++ b/packages/tooling/contract-test-utils/src/shared/BaseTestHook.ts @@ -1,20 +1,34 @@ import { HookData, HookErrors } from '../types/CommandParams.js'; +import { HookPostQueue } from './HookPostQueue.js'; + export abstract class BaseTestHook { protected readonly hookName: string; protected readonly endpoint: string; protected readonly hookData?: HookData; protected readonly hookErrors?: HookErrors; + private readonly _postQueue: HookPostQueue; - constructor(name: string, endpoint: string, data?: HookData, errors?: HookErrors) { + constructor( + name: string, + endpoint: string, + data?: HookData, + errors?: HookErrors, + postQueue?: HookPostQueue, + ) { this.hookName = name; this.endpoint = endpoint; this.hookData = data; this.hookErrors = errors; + this._postQueue = postQueue ?? new HookPostQueue(); } protected abstract safePost(body: unknown): Promise; + protected enqueuePost(body: unknown): void { + this._postQueue.enqueue(() => this.safePost(body)); + } + getMetadata() { return { name: this.hookName }; } @@ -26,7 +40,7 @@ export abstract class BaseTestHook { if (this.hookErrors?.beforeEvaluation) { throw new Error(this.hookErrors.beforeEvaluation); } - this.safePost({ + this.enqueuePost({ evaluationSeriesContext: hookContext, evaluationSeriesData: data, stage: 'beforeEvaluation', @@ -42,7 +56,7 @@ export abstract class BaseTestHook { if (this.hookErrors?.afterEvaluation) { throw new Error(this.hookErrors.afterEvaluation); } - this.safePost({ + this.enqueuePost({ evaluationSeriesContext: hookContext, evaluationSeriesData: data, stage: 'afterEvaluation', diff --git a/packages/tooling/contract-test-utils/src/shared/HookPostQueue.ts b/packages/tooling/contract-test-utils/src/shared/HookPostQueue.ts new file mode 100644 index 0000000000..6316878cc1 --- /dev/null +++ b/packages/tooling/contract-test-utils/src/shared/HookPostQueue.ts @@ -0,0 +1,9 @@ +// The test harness records hook callbacks in network-arrival order, so +// parallel fetches break ordering assertions. Chain them instead. +export class HookPostQueue { + private _chain: Promise = Promise.resolve(); + + enqueue(fn: () => Promise): void { + this._chain = this._chain.then(fn).catch(() => {}); + } +}