fix: fixes race between setUserId and trigger (#7498)

Co-authored-by: Dhruwang <dhruwangjariwala18@gmail.com>

packages/js-core/src/lib/survey/tests/widget.test.ts

@@ -43,6 +43,17 @@ vi.mock("@/lib/common/utils", () => ({
   handleHiddenFields: vi.fn(),
 }));
 
+const mockUpdateQueue = {
+  hasPendingWork: vi.fn().mockReturnValue(false),
+  waitForPendingWork: vi.fn().mockResolvedValue(true),
+};
+
+vi.mock("@/lib/user/update-queue", () => ({
+  UpdateQueue: {
+    getInstance: vi.fn(() => mockUpdateQueue),
+  },
+}));
+
 describe("widget-file", () => {
   let getInstanceConfigMock: MockInstance<() => Config>;
   let getInstanceLoggerMock: MockInstance<() => Logger>;
@@ -249,4 +260,265 @@ describe("widget-file", () => {
     widget.removeWidgetContainer();
     expect(document.getElementById("formbricks-container")).toBeFalsy();
   });
+
+  test("renderWidget waits for pending identification before rendering", async () => {
+    mockUpdateQueue.hasPendingWork.mockReturnValue(true);
+    mockUpdateQueue.waitForPendingWork.mockResolvedValue(true);
+
+    const mockConfigValue = {
+      get: vi.fn().mockReturnValue({
+        appUrl: "https://fake.app",
+        environmentId: "env_123",
+        environment: {
+          data: {
+            project: {
+              clickOutsideClose: true,
+              overlay: "none",
+              placement: "bottomRight",
+              inAppSurveyBranding: true,
+            },
+          },
+        },
+        user: {
+          data: {
+            userId: "user_abc",
+            contactId: "contact_abc",
+            displays: [],
+            responses: [],
+            lastDisplayAt: null,
+            language: "en",
+          },
+        },
+      }),
+      update: vi.fn(),
+    };
+
+    getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
+    widget.setIsSurveyRunning(false);
+
+    // @ts-expect-error -- mock window.formbricksSurveys
+    window.formbricksSurveys = {
+      renderSurvey: vi.fn(),
+    };
+
+    vi.useFakeTimers();
+
+    await widget.renderWidget({
+      ...mockSurvey,
+      delay: 0,
+    } as unknown as TEnvironmentStateSurvey);
+
+    expect(mockUpdateQueue.hasPendingWork).toHaveBeenCalled();
+    expect(mockUpdateQueue.waitForPendingWork).toHaveBeenCalled();
+
+    vi.advanceTimersByTime(0);
+
+    expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalledWith(
+      expect.objectContaining({
+        contactId: "contact_abc",
+      })
+    );
+
+    vi.useRealTimers();
+  });
+
+  test("renderWidget does not wait when no identification is pending", async () => {
+    mockUpdateQueue.hasPendingWork.mockReturnValue(false);
+
+    const mockConfigValue = {
+      get: vi.fn().mockReturnValue({
+        appUrl: "https://fake.app",
+        environmentId: "env_123",
+        environment: {
+          data: {
+            project: {
+              clickOutsideClose: true,
+              overlay: "none",
+              placement: "bottomRight",
+              inAppSurveyBranding: true,
+            },
+          },
+        },
+        user: {
+          data: {
+            userId: "user_abc",
+            contactId: "contact_abc",
+            displays: [],
+            responses: [],
+            lastDisplayAt: null,
+            language: "en",
+          },
+        },
+      }),
+      update: vi.fn(),
+    };
+
+    getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
+    widget.setIsSurveyRunning(false);
+
+    // @ts-expect-error -- mock window.formbricksSurveys
+    window.formbricksSurveys = {
+      renderSurvey: vi.fn(),
+    };
+
+    vi.useFakeTimers();
+
+    await widget.renderWidget({
+      ...mockSurvey,
+      delay: 0,
+    } as unknown as TEnvironmentStateSurvey);
+
+    expect(mockUpdateQueue.hasPendingWork).toHaveBeenCalled();
+    expect(mockUpdateQueue.waitForPendingWork).not.toHaveBeenCalled();
+
+    vi.advanceTimersByTime(0);
+    expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalled();
+
+    vi.useRealTimers();
+  });
+
+  test("renderWidget reads contactId after identification wait completes", async () => {
+    let callCount = 0;
+    const mockConfigValue = {
+      get: vi.fn().mockImplementation(() => {
+        callCount++;
+        return {
+          appUrl: "https://fake.app",
+          environmentId: "env_123",
+          environment: {
+            data: {
+              project: {
+                clickOutsideClose: true,
+                overlay: "none",
+                placement: "bottomRight",
+                inAppSurveyBranding: true,
+              },
+            },
+          },
+          user: {
+            data: {
+              // Simulate contactId becoming available after identification
+              userId: "user_abc",
+              contactId: callCount > 2 ? "contact_after_identification" : undefined,
+              displays: [],
+              responses: [],
+              lastDisplayAt: null,
+              language: "en",
+            },
+          },
+        };
+      }),
+      update: vi.fn(),
+    };
+
+    getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
+    mockUpdateQueue.hasPendingWork.mockReturnValue(true);
+    mockUpdateQueue.waitForPendingWork.mockResolvedValue(true);
+    widget.setIsSurveyRunning(false);
+
+    // @ts-expect-error -- mock window.formbricksSurveys
+    window.formbricksSurveys = {
+      renderSurvey: vi.fn(),
+    };
+
+    vi.useFakeTimers();
+
+    await widget.renderWidget({
+      ...mockSurvey,
+      delay: 0,
+    } as unknown as TEnvironmentStateSurvey);
+
+    vi.advanceTimersByTime(0);
+
+    // The contactId passed to renderSurvey should be read after the wait
+    expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalledWith(
+      expect.objectContaining({
+        contactId: "contact_after_identification",
+      })
+    );
+
+    vi.useRealTimers();
+  });
+
+  test("renderWidget skips survey when identification fails and survey has segment filters", async () => {
+    mockUpdateQueue.hasPendingWork.mockReturnValue(true);
+    mockUpdateQueue.waitForPendingWork.mockResolvedValue(false);
+
+    widget.setIsSurveyRunning(false);
+
+    // @ts-expect-error -- mock window.formbricksSurveys
+    window.formbricksSurveys = {
+      renderSurvey: vi.fn(),
+    };
+
+    await widget.renderWidget({
+      ...mockSurvey,
+      delay: 0,
+      segment: { id: "seg_1", filters: [{ type: "attribute", value: "plan" }] },
+    } as unknown as TEnvironmentStateSurvey);
+
+    expect(mockUpdateQueue.waitForPendingWork).toHaveBeenCalled();
+    expect(mockLogger.debug).toHaveBeenCalledWith(
+      "User identification failed. Skipping survey with segment filters."
+    );
+    expect(window.formbricksSurveys.renderSurvey).not.toHaveBeenCalled();
+  });
+
+  test("renderWidget proceeds when identification fails but survey has no segment filters", async () => {
+    mockUpdateQueue.hasPendingWork.mockReturnValue(true);
+    mockUpdateQueue.waitForPendingWork.mockResolvedValue(false);
+
+    const mockConfigValue = {
+      get: vi.fn().mockReturnValue({
+        appUrl: "https://fake.app",
+        environmentId: "env_123",
+        environment: {
+          data: {
+            project: {
+              clickOutsideClose: true,
+              overlay: "none",
+              placement: "bottomRight",
+              inAppSurveyBranding: true,
+            },
+          },
+        },
+        user: {
+          data: {
+            userId: null,
+            contactId: null,
+            displays: [],
+            responses: [],
+            lastDisplayAt: null,
+            language: "en",
+          },
+        },
+      }),
+      update: vi.fn(),
+    };
+
+    getInstanceConfigMock.mockReturnValue(mockConfigValue as unknown as Config);
+    widget.setIsSurveyRunning(false);
+
+    // @ts-expect-error -- mock window.formbricksSurveys
+    window.formbricksSurveys = {
+      renderSurvey: vi.fn(),
+    };
+
+    vi.useFakeTimers();
+
+    await widget.renderWidget({
+      ...mockSurvey,
+      delay: 0,
+      segment: undefined,
+    } as unknown as TEnvironmentStateSurvey);
+
+    expect(mockLogger.debug).toHaveBeenCalledWith(
+      "User identification failed but survey has no segment filters. Proceeding."
+    );
+
+    vi.advanceTimersByTime(0);
+    expect(window.formbricksSurveys.renderSurvey).toHaveBeenCalled();
+
+    vi.useRealTimers();
+  });
 });

packages/js-core/src/lib/survey/widget.ts

@@ -11,6 +11,7 @@ import {
   handleHiddenFields,
   shouldDisplayBasedOnPercentage,
 } from "@/lib/common/utils";
+import { UpdateQueue } from "@/lib/user/update-queue";
 import { type TEnvironmentStateSurvey, type TUserState } from "@/types/config";
 import { type TTrackProperties } from "@/types/survey";
 
@@ -60,6 +61,24 @@ export const renderWidget = async (
 
   setIsSurveyRunning(true);
 
+  // Wait for pending user identification to complete before rendering
+  const updateQueue = UpdateQueue.getInstance();
+  if (updateQueue.hasPendingWork()) {
+    logger.debug("Waiting for pending user identification before rendering survey");
+    const identificationSucceeded = await updateQueue.waitForPendingWork();
+    if (!identificationSucceeded) {
+      const hasSegmentFilters = Array.isArray(survey.segment?.filters) && survey.segment.filters.length > 0;
+
+      if (hasSegmentFilters) {
+        logger.debug("User identification failed. Skipping survey with segment filters.");
+        setIsSurveyRunning(false);
+        return;
+      }
+
+      logger.debug("User identification failed but survey has no segment filters. Proceeding.");
+    }
+  }
+
   if (survey.delay) {
     logger.debug(`Delaying survey "${survey.name}" by ${survey.delay.toString()} seconds.`);
   }

packages/js-core/src/lib/user/tests/update-queue.test.ts

@@ -169,4 +169,104 @@ describe("UpdateQueue", () => {
       "Formbricks can't set attributes without a userId! Please set a userId first with the setUserId function"
     );
   });
+
+  test("hasPendingWork returns false when no updates and no flush in flight", () => {
+    expect(updateQueue.hasPendingWork()).toBe(false);
+  });
+
+  test("hasPendingWork returns true when updates are queued", () => {
+    updateQueue.updateUserId(mockUserId1);
+    expect(updateQueue.hasPendingWork()).toBe(true);
+  });
+
+  test("hasPendingWork returns true while processUpdates flush is in flight", () => {
+    (sendUpdates as Mock).mockReturnValue({
+      ok: true,
+      data: { hasWarnings: false },
+    });
+
+    updateQueue.updateUserId(mockUserId1);
+    // Start processing but don't await — the debounce means the flush is in-flight
+    void updateQueue.processUpdates();
+
+    expect(updateQueue.hasPendingWork()).toBe(true);
+  });
+
+  test("waitForPendingWork returns true immediately when no pending work", async () => {
+    const result = await updateQueue.waitForPendingWork();
+    expect(result).toBe(true);
+  });
+
+  test("waitForPendingWork returns true when processUpdates succeeds", async () => {
+    (sendUpdates as Mock).mockReturnValue({
+      ok: true,
+      data: { hasWarnings: false },
+    });
+
+    updateQueue.updateUserId(mockUserId1);
+    void updateQueue.processUpdates();
+
+    const result = await updateQueue.waitForPendingWork();
+
+    expect(result).toBe(true);
+    expect(updateQueue.hasPendingWork()).toBe(false);
+    expect(sendUpdates).toHaveBeenCalled();
+  });
+
+  test("waitForPendingWork returns false when processUpdates rejects", async () => {
+    loggerMock.mockReturnValue(mockLogger as unknown as Logger);
+    (sendUpdates as Mock).mockRejectedValue(new Error("network error"));
+
+    updateQueue.updateUserId(mockUserId1);
+    // eslint-disable-next-line @typescript-eslint/no-empty-function -- intentionally swallowing rejection to avoid unhandled promise
+    const processPromise = updateQueue.processUpdates().catch(() => {});
+
+    const result = await updateQueue.waitForPendingWork();
+    expect(result).toBe(false);
+    await processPromise;
+  });
+
+  test("waitForPendingWork returns false when flush hangs past timeout", async () => {
+    vi.useFakeTimers();
+
+    // sendUpdates returns a promise that never resolves, simulating a network hang
+    // eslint-disable-next-line @typescript-eslint/no-empty-function -- intentionally never-resolving promise
+    (sendUpdates as Mock).mockReturnValue(new Promise(() => {}));
+
+    updateQueue.updateUserId(mockUserId1);
+    void updateQueue.processUpdates();
+
+    const resultPromise = updateQueue.waitForPendingWork();
+
+    // Advance past the debounce delay (500ms) so the handler fires and hangs on sendUpdates
+    await vi.advanceTimersByTimeAsync(500);
+    // Advance past the pending work timeout (5000ms)
+    await vi.advanceTimersByTimeAsync(5000);
+
+    const result = await resultPromise;
+    expect(result).toBe(false);
+
+    vi.useRealTimers();
+  });
+
+  test("processUpdates reuses pending flush instead of creating orphaned promises", async () => {
+    (sendUpdates as Mock).mockReturnValue({
+      ok: true,
+      data: { hasWarnings: false },
+    });
+
+    updateQueue.updateUserId(mockUserId1);
+
+    // First call creates the flush promise
+    const firstPromise = updateQueue.processUpdates();
+
+    // Second call while first is still pending should not create a new flush
+    updateQueue.updateAttributes({ name: mockAttributes.name });
+    const secondPromise = updateQueue.processUpdates();
+
+    // Both promises should resolve (second is not orphaned)
+    await Promise.all([firstPromise, secondPromise]);
+
+    expect(updateQueue.hasPendingWork()).toBe(false);
+  });
 });

packages/js-core/src/lib/user/update-queue.ts

@@ -8,7 +8,9 @@ export class UpdateQueue {
   private static instance: UpdateQueue | null = null;
   private updates: TUpdates | null = null;
   private debounceTimeout: NodeJS.Timeout | null = null;
+  private pendingFlush: Promise<void> | null = null;
   private readonly DEBOUNCE_DELAY = 500;
+  private readonly PENDING_WORK_TIMEOUT = 5000;
 
   private constructor() {}
 
@@ -63,17 +65,45 @@ export class UpdateQueue {
     return !this.updates;
   }
 
+  public hasPendingWork(): boolean {
+    return this.updates !== null || this.pendingFlush !== null;
+  }
+
+  public async waitForPendingWork(): Promise<boolean> {
+    if (!this.hasPendingWork()) return true;
+
+    const flush = this.pendingFlush ?? this.processUpdates();
+    try {
+      const succeeded = await Promise.race([
+        flush.then(() => true as const),
+        new Promise<false>((resolve) => {
+          setTimeout(() => {
+            resolve(false);
+          }, this.PENDING_WORK_TIMEOUT);
+        }),
+      ]);
+      return succeeded;
+    } catch {
+      return false;
+    }
+  }
+
   public async processUpdates(): Promise<void> {
     const logger = Logger.getInstance();
     if (!this.updates) {
       return;
     }
 
+    // If a flush is already in flight, reuse it instead of creating a new promise
+    if (this.pendingFlush) {
+      return this.pendingFlush;
+    }
+
     if (this.debounceTimeout) {
       clearTimeout(this.debounceTimeout);
     }
 
-    return new Promise((resolve, reject) => {
+    const flushPromise = new Promise<void>((resolve, reject) => {
       const handler = async (): Promise<void> => {
         try {
           let currentUpdates = { ...this.updates };
@@ -147,8 +177,10 @@ export class UpdateQueue {
           }
 
           this.clearUpdates();
+          this.pendingFlush = null;
           resolve();
         } catch (error: unknown) {
+          this.pendingFlush = null;
           logger.error(
             `Failed to process updates: ${error instanceof Error ? error.message : "Unknown error"}`
           );
@@ -158,5 +190,8 @@ export class UpdateQueue {
 
       this.debounceTimeout = setTimeout(() => void handler(), this.DEBOUNCE_DELAY);
     });
+
+    this.pendingFlush = flushPromise;
+    return flushPromise;
   }
 }