ci: fixing contract test breaking due to async hooks (#1391)

joker23 · web-flow · commit 58e063bad465 · 2026-05-26T18:10:32.000-04:00
BEGIN_COMMIT_OVERRIDE ci: fixing contract test breaking due to async hooks fix(sdk-client): `executeAfterTrack` ordering END_COMMIT_OVERRIDE  --- > [!NOTE] > **Medium Risk** > Changes runtime hook invocation order for afterTrack (a behavioral API contract); impact is limited to apps using multiple track hooks, but ordering semantics are intentional for harness alignment. > > **Overview** > Fixes contract-test hook ordering failures by aligning **`afterTrack`** execution with registration order and serializing harness callback posts. > > In **`HookRunner`**, **`executeAfterTrack`** now iterates hooks forward (same as **`beforeEvaluation`** / **`beforeIdentify`**) instead of reverse like **`afterEvaluation`**. **`HookRunner`** unit expectations for track hook order are updated accordingly. > > Contract-test **`TestHook`** instances now share a **`HookPostQueue`** via **`TestHook.forClient`**, chaining async **`fetch`** callbacks so the harness sees stable arrival order; **`BaseTestHook`** routes evaluation/track posts through **`enqueuePost`**. Browser, Electron, React, React Native, and server-node contract entities use the factory. Hook-order-related lines are removed from several **`suppressions.txt`** files now that behavior matches the harness. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 6b89ddf. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>
diff --git 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
@@ -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
+tags/stream requests/{"applicationId":"________________________________________________________________","applicationVersion":"________________________________________________________________"}
diff --git 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
@@ -1,2 +0,0 @@
-hooks/evaluation/executes beforeEvaluation hooks in registration order
-hooks/track/executes afterTrack hooks in registration order
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
+polling/requests/URL path is computed correctly/environment_filter_key="encoding_not_necessary"/base URI has a trailing slash/GET
diff --git 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
@@ -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
@@ -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<SDKConfigHookInstance>): 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<void> {
     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
@@ -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<SDKConfigHookInstance>): 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<void> {
     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
@@ -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<void>;
 
+  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
@@ -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<void> = Promise.resolve();
+
+  enqueue(fn: () => Promise<void>): void {
+    this._chain = this._chain.then(fn).catch(() => {});
+  }
+}

Original file line number	Diff line number	Diff line change
`@@ -190,9 +190,7 @@ function makeSdkConfig(options: SDKConfigParams, tag: string) {`
`190`	`190`	`}`
`191`	`191`
`192`	`192`	`if (options.hooks) {`
`193`		`- cf.hooks = options.hooks.hooks.map(`
`194`		`- (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors),`
`195`		`- );`
	`193`	`+ cf.hooks = TestHook.forClient(options.hooks.hooks);`
`196`	`194`	`}`
`197`	`195`
`198`	`196`	`cf.fetchGoals = false;`
Original file line number	Diff line number	Diff line change
`@@ -87,9 +87,7 @@ function makeSdkConfig(options: SDKConfigParams, tag: string) {`
`87`	`87`	`}`
`88`	`88`
`89`	`89`	`if (options.hooks) {`
`90`		`- cf.hooks = options.hooks.hooks.map(`
`91`		`- (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors),`
`92`		`- );`
	`90`	`+ cf.hooks = TestHook.forClient(options.hooks.hooks);`
`93`	`91`	`}`
`94`	`92`
`95`	`93`	`if (options.tls) {`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +0,0 @@`
`1`		`-hooks/evaluation/executes beforeEvaluation hooks in registration order`
`2`		`-hooks/track/executes afterTrack hooks in registration order`
Original file line number	Diff line number	Diff line change
`@@ -77,9 +77,7 @@ function makeSdkConfig(options: SDKConfigParams, tag: string) {`
`77`	`77`	`}`
`78`	`78`
`79`	`79`	`if (options.hooks) {`
`80`		`- cf.hooks = options.hooks.hooks.map(`
`81`		`- (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors),`
`82`		`- );`
	`80`	`+ cf.hooks = TestHook.forClient(options.hooks.hooks);`
`83`	`81`	`}`
`84`	`82`
`85`	`83`	`return cf;`
Original file line number	Diff line number	Diff line change
`@@ -70,9 +70,7 @@ export function makeSdkConfig(options: SDKConfigParams, tag: string): LDOptions`
`70`	`70`	`}`
`71`	`71`
`72`	`72`	`if (options.hooks) {`
`73`		`- cf.hooks = options.hooks.hooks.map(`
`74`		`- (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors),`
`75`		`- );`
	`73`	`+ cf.hooks = TestHook.forClient(options.hooks.hooks);`
`76`	`74`	`}`
`77`	`75`
`78`	`76`	`cf.fetchGoals = false;`
Original file line number	Diff line number	Diff line change
`@@ -88,9 +88,7 @@ export function makeSdkConfig(options: ServerSDKConfigParams, tag: string): LDOp`
`88`	`88`	`}`
`89`	`89`
`90`	`90`	`if (options.hooks) {`
`91`		`- cf.hooks = options.hooks.hooks.map(`
`92`		`- (hook) => new TestHook(hook.name, hook.callbackUri, hook.data, hook.errors),`
`93`		`- );`
	`91`	`+ cf.hooks = TestHook.forClient(options.hooks.hooks);`
`94`	`92`	`}`
`95`	`93`
`96`	`94`	`if (options.wrapper) {`