mirror of
https://github.com/formbricks/formbricks.git
synced 2026-04-19 02:10:33 -05:00
fix: await user sync completion in setUserId and setAttributes
Previously, setUserId() and setAttributes() returned immediately after queuing the update, but the API sync (which sets contactId) happened asynchronously with a 500ms debounce. This caused a race condition where surveys could be displayed as anonymous if shown before the sync completed. Changes: - Make setUserId() and setAttributes() await processUpdates() completion - Add queue.wait() to public API functions (setUserId, setAttribute, setAttributes, setEmail, setLanguage) to ensure commands execute - Add error handling for network failures during sync - Update tests to reflect new async behavior This ensures that when customers await these functions, the contactId is properly set before the promise resolves, preventing anonymous survey responses.
This commit is contained in:
@@ -42,22 +42,27 @@ const setup = async (setupConfig: TConfigInput): Promise<void> => {
|
||||
|
||||
const setUserId = async (userId: string): Promise<void> => {
|
||||
await queue.add(User.setUserId, CommandType.UserAction, true, userId);
|
||||
await queue.wait();
|
||||
};
|
||||
|
||||
const setEmail = async (email: string): Promise<void> => {
|
||||
await queue.add(Attribute.setAttributes, CommandType.UserAction, true, { email });
|
||||
await queue.wait();
|
||||
};
|
||||
|
||||
const setAttribute = async (key: string, value: string): Promise<void> => {
|
||||
await queue.add(Attribute.setAttributes, CommandType.UserAction, true, { [key]: value });
|
||||
await queue.wait();
|
||||
};
|
||||
|
||||
const setAttributes = async (attributes: Record<string, string>): Promise<void> => {
|
||||
await queue.add(Attribute.setAttributes, CommandType.UserAction, true, attributes);
|
||||
await queue.wait();
|
||||
};
|
||||
|
||||
const setLanguage = async (language: string): Promise<void> => {
|
||||
await queue.add(Attribute.setAttributes, CommandType.UserAction, true, { language });
|
||||
await queue.wait();
|
||||
};
|
||||
|
||||
const logout = async (): Promise<void> => {
|
||||
|
||||
@@ -1,12 +1,25 @@
|
||||
import { Logger } from "@/lib/common/logger";
|
||||
import { UpdateQueue } from "@/lib/user/update-queue";
|
||||
import { type NetworkError, type Result, okVoid } from "@/types/error";
|
||||
import { type NetworkError, type Result, err, okVoid } from "@/types/error";
|
||||
|
||||
export const setAttributes = async (
|
||||
attributes: Record<string, string>
|
||||
// eslint-disable-next-line @typescript-eslint/require-await -- we want to use promises here
|
||||
): Promise<Result<void, NetworkError>> => {
|
||||
const logger = Logger.getInstance();
|
||||
const updateQueue = UpdateQueue.getInstance();
|
||||
updateQueue.updateAttributes(attributes);
|
||||
void updateQueue.processUpdates();
|
||||
return okVoid();
|
||||
try {
|
||||
await updateQueue.processUpdates();
|
||||
return okVoid();
|
||||
} catch (error) {
|
||||
logger.error(
|
||||
`Failed to process attribute updates: ${error instanceof Error ? error.message : "Unknown error"}`
|
||||
);
|
||||
return err({
|
||||
code: "network_error",
|
||||
message: "Failed to sync attributes",
|
||||
responseMessage: error instanceof Error ? error.message : "Unknown error",
|
||||
status: 500,
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
@@ -17,6 +17,16 @@ vi.mock("@/lib/user/update-queue", () => ({
|
||||
},
|
||||
}));
|
||||
|
||||
// Mock the Logger
|
||||
vi.mock("@/lib/common/logger", () => ({
|
||||
Logger: {
|
||||
getInstance: vi.fn(() => ({
|
||||
error: vi.fn(),
|
||||
debug: vi.fn(),
|
||||
})),
|
||||
},
|
||||
}));
|
||||
|
||||
describe("User Attributes", () => {
|
||||
const mockUpdateQueue = {
|
||||
updateAttributes: vi.fn(),
|
||||
@@ -32,6 +42,8 @@ describe("User Attributes", () => {
|
||||
|
||||
describe("setAttributes", () => {
|
||||
test("successfully updates attributes and triggers processing", async () => {
|
||||
mockUpdateQueue.processUpdates.mockResolvedValue(undefined);
|
||||
|
||||
const result = await setAttributes(mockAttributes);
|
||||
|
||||
// Verify UpdateQueue methods were called correctly
|
||||
@@ -43,6 +55,8 @@ describe("User Attributes", () => {
|
||||
});
|
||||
|
||||
test("processes multiple attribute updates", async () => {
|
||||
mockUpdateQueue.processUpdates.mockResolvedValue(undefined);
|
||||
|
||||
const firstAttributes = { name: mockAttributes.name };
|
||||
const secondAttributes = { email: mockAttributes.email };
|
||||
|
||||
@@ -55,22 +69,35 @@ describe("User Attributes", () => {
|
||||
expect(mockUpdateQueue.processUpdates).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
test("processes updates asynchronously", async () => {
|
||||
test("waits for processUpdates to complete", async () => {
|
||||
const attributes = { name: mockAttributes.name };
|
||||
let processUpdatesResolved = false;
|
||||
|
||||
// Mock processUpdates to be async
|
||||
// Mock processUpdates to be async and set a flag when resolved
|
||||
mockUpdateQueue.processUpdates.mockImplementation(
|
||||
() =>
|
||||
new Promise((resolve) => {
|
||||
setTimeout(resolve, 100);
|
||||
setTimeout(() => {
|
||||
processUpdatesResolved = true;
|
||||
resolve(undefined);
|
||||
}, 100);
|
||||
})
|
||||
);
|
||||
|
||||
const result = await setAttributes(attributes);
|
||||
const resultPromise = setAttributes(attributes);
|
||||
|
||||
expect(result.ok).toBe(true);
|
||||
// Verify processUpdates was called
|
||||
expect(mockUpdateQueue.processUpdates).toHaveBeenCalled();
|
||||
// The function returns before processUpdates completes due to void operator
|
||||
|
||||
// Verify the function hasn't resolved yet
|
||||
expect(processUpdatesResolved).toBe(false);
|
||||
|
||||
// Wait for setAttributes to complete
|
||||
const result = await resultPromise;
|
||||
|
||||
// Verify it completed after processUpdates
|
||||
expect(processUpdatesResolved).toBe(true);
|
||||
expect(result.ok).toBe(true);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -101,7 +101,7 @@ describe("user.ts", () => {
|
||||
|
||||
const mockUpdateQueue = {
|
||||
updateUserId: vi.fn(),
|
||||
processUpdates: vi.fn(),
|
||||
processUpdates: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
|
||||
getInstanceConfigMock.mockReturnValue(mockConfig as unknown as Config);
|
||||
@@ -113,6 +113,42 @@ describe("user.ts", () => {
|
||||
expect(mockUpdateQueue.updateUserId).toHaveBeenCalledWith(mockUserId);
|
||||
expect(mockUpdateQueue.processUpdates).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test("returns error if processUpdates fails", async () => {
|
||||
const mockConfig = {
|
||||
get: vi.fn().mockReturnValue({
|
||||
user: {
|
||||
data: {
|
||||
userId: null,
|
||||
},
|
||||
},
|
||||
}),
|
||||
};
|
||||
|
||||
const mockLogger = {
|
||||
debug: vi.fn(),
|
||||
error: vi.fn(),
|
||||
};
|
||||
|
||||
const mockUpdateQueue = {
|
||||
updateUserId: vi.fn(),
|
||||
processUpdates: vi.fn().mockRejectedValue(new Error("Network error")),
|
||||
};
|
||||
|
||||
getInstanceConfigMock.mockReturnValue(mockConfig as unknown as Config);
|
||||
getInstanceLoggerMock.mockReturnValue(mockLogger as unknown as Logger);
|
||||
getInstanceUpdateQueueMock.mockReturnValue(mockUpdateQueue as unknown as UpdateQueue);
|
||||
const result = await setUserId(mockUserId);
|
||||
|
||||
expect(result.ok).toBe(false);
|
||||
if (!result.ok) {
|
||||
expect(result.error.code).toBe("network_error");
|
||||
expect(result.error.status).toBe(500);
|
||||
}
|
||||
expect(mockUpdateQueue.updateUserId).toHaveBeenCalledWith(mockUserId);
|
||||
expect(mockUpdateQueue.processUpdates).toHaveBeenCalled();
|
||||
expect(mockLogger.error).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe("logout", () => {
|
||||
|
||||
@@ -4,7 +4,6 @@ import { tearDown } from "@/lib/common/setup";
|
||||
import { UpdateQueue } from "@/lib/user/update-queue";
|
||||
import { type ApiErrorResponse, type Result, err, okVoid } from "@/types/error";
|
||||
|
||||
// eslint-disable-next-line @typescript-eslint/require-await -- we want to use promises here
|
||||
export const setUserId = async (userId: string): Promise<Result<void, ApiErrorResponse>> => {
|
||||
const appConfig = Config.getInstance();
|
||||
const logger = Logger.getInstance();
|
||||
@@ -27,8 +26,20 @@ export const setUserId = async (userId: string): Promise<Result<void, ApiErrorRe
|
||||
}
|
||||
|
||||
updateQueue.updateUserId(userId);
|
||||
void updateQueue.processUpdates();
|
||||
return okVoid();
|
||||
try {
|
||||
await updateQueue.processUpdates();
|
||||
return okVoid();
|
||||
} catch (error) {
|
||||
logger.error(
|
||||
`Failed to process userId update: ${error instanceof Error ? error.message : "Unknown error"}`
|
||||
);
|
||||
return err({
|
||||
code: "network_error",
|
||||
message: "Failed to sync userId",
|
||||
responseMessage: error instanceof Error ? error.message : "Unknown error",
|
||||
status: 500,
|
||||
});
|
||||
}
|
||||
};
|
||||
|
||||
export const logout = (): Result<void> => {
|
||||
|
||||
Reference in New Issue
Block a user