diff --git a/.changeset/handle-guide-engagement-errors.md b/.changeset/handle-guide-engagement-errors.md new file mode 100644 index 000000000..a80e11e5f --- /dev/null +++ b/.changeset/handle-guide-engagement-errors.md @@ -0,0 +1,8 @@ +--- +"@knocklabs/client": patch +"@knocklabs/react": patch +--- + +fix(KNO-14505): prevent guide engagement API failures from becoming unhandled promise rejections + +Guide seen, interacted, and archived events continue to update local state optimistically while failed background requests are caught and logged. API request errors now retain their original identity, HTTP status, and response data for diagnostics. diff --git a/packages/client/src/clients/guide/client.ts b/packages/client/src/clients/guide/client.ts index 5abd97a02..a284c8199 100644 --- a/packages/client/src/clients/guide/client.ts +++ b/packages/client/src/clients/guide/client.ts @@ -1022,10 +1022,16 @@ export class KnockGuideClient { data: this.targetParams.data, }; - this.knock.user.markGuideStepAs( - "seen", - params, - ); + void this.knock.user + .markGuideStepAs("seen", params) + .catch((error: unknown) => { + const message = error instanceof Error ? error.message : String(error); + + this.knock.log( + `[Guide] Failed to mark guide step as seen: ${message}`, + true, + ); + }); return updatedStep; } @@ -1065,10 +1071,19 @@ export class KnockGuideClient { metadata, }; - this.knock.user.markGuideStepAs< - MarkAsInteractedParams, - MarkGuideAsResponse - >("interacted", params); + void this.knock.user + .markGuideStepAs< + MarkAsInteractedParams, + MarkGuideAsResponse + >("interacted", params) + .catch((error: unknown) => { + const message = error instanceof Error ? error.message : String(error); + + this.knock.log( + `[Guide] Failed to mark guide step as interacted: ${message}`, + true, + ); + }); return updatedStep; } @@ -1100,13 +1115,19 @@ export class KnockGuideClient { const params = this.buildEngagementEventBaseParams(guide, updatedStep); - this.knock.user.markGuideStepAs( - "archived", - { + void this.knock.user + .markGuideStepAs("archived", { ...params, unthrottled: guide.bypass_global_group_limit, - }, - ); + }) + .catch((error: unknown) => { + const message = error instanceof Error ? error.message : String(error); + + this.knock.log( + `[Guide] Failed to mark guide step as archived: ${message}`, + true, + ); + }); return updatedStep; } diff --git a/packages/client/src/clients/users/index.ts b/packages/client/src/clients/users/index.ts index 0cdab95f2..93b0ef3e1 100644 --- a/packages/client/src/clients/users/index.ts +++ b/packages/client/src/clients/users/index.ts @@ -153,7 +153,8 @@ class UserClient { private handleResponse(response: ApiResponse) { if (response.statusCode === "error") { - throw new Error(response.error || response.body); + const error = response.error || response.body; + throw error instanceof Error ? error : new Error(error); } return response.body as T; diff --git a/packages/client/test/clients/guide/guide.test.ts b/packages/client/test/clients/guide/guide.test.ts index c03eae834..c8d631aee 100644 --- a/packages/client/test/clients/guide/guide.test.ts +++ b/packages/client/test/clients/guide/guide.test.ts @@ -901,6 +901,125 @@ describe("KnockGuideClient", () => { }); }); + test.each([ + { + status: "seen", + timestamp: "seen_at", + engage: ( + client: KnockGuideClient, + guide: KnockGuide, + step: KnockGuideStep, + ) => client.markAsSeen(guide, step), + }, + { + status: "interacted", + timestamp: "interacted_at", + engage: ( + client: KnockGuideClient, + guide: KnockGuide, + step: KnockGuideStep, + ) => client.markAsInteracted(guide, step), + }, + { + status: "archived", + timestamp: "archived_at", + engage: ( + client: KnockGuideClient, + guide: KnockGuide, + step: KnockGuideStep, + ) => client.markAsArchived(guide, step), + }, + ] as const)( + "handles rejected $status requests after applying the optimistic update", + async ({ status, timestamp, engage }) => { + const error = new Error(`${status} request failed`); + vi.mocked(mockKnock.user.markGuideStepAs).mockRejectedValueOnce(error); + + const freshStep = { + ...mockStep, + message: { + ...mockStep.message, + seen_at: null, + read_at: null, + interacted_at: null, + archived_at: null, + }, + } as KnockGuideStep; + const freshGuide = { + ...mockGuide, + steps: [freshStep], + getStep: vi.fn().mockReturnValue(freshStep), + } as KnockGuide; + const stateWithGuides = { + guideGroups: [mockDefaultGroup], + guideGroupDisplayLogs: {}, + guides: { [freshGuide.key]: freshGuide }, + ineligibleGuides: {}, + previewGuides: {}, + queries: {}, + location: undefined, + counter: 0, + debug: { forcedGuideKey: null, previewSessionId: null }, + }; + mockStore.state = stateWithGuides; + mockStore.getState.mockReturnValue(stateWithGuides); + + const result = await engage( + new KnockGuideClient(mockKnock, channelId), + freshGuide, + freshStep, + ); + await Promise.resolve(); + + expect(result?.message[timestamp]).toEqual(expect.any(String)); + expect(mockKnock.log).toHaveBeenCalledWith( + `[Guide] Failed to mark guide step as ${status}: ${error.message}`, + true, + ); + }, + ); + + test("does not wait for the engagement request before resolving", async () => { + const pendingRequest = new Promise(() => {}); + vi.mocked(mockKnock.user.markGuideStepAs).mockReturnValueOnce( + pendingRequest, + ); + + const freshStep = { + ...mockStep, + message: { + ...mockStep.message, + interacted_at: null, + read_at: null, + }, + } as KnockGuideStep; + const freshGuide = { + ...mockGuide, + steps: [freshStep], + getStep: vi.fn().mockReturnValue(freshStep), + } as KnockGuide; + const stateWithGuides = { + guideGroups: [mockDefaultGroup], + guideGroupDisplayLogs: {}, + guides: { [freshGuide.key]: freshGuide }, + ineligibleGuides: {}, + previewGuides: {}, + queries: {}, + location: undefined, + counter: 0, + debug: { forcedGuideKey: null, previewSessionId: null }, + }; + mockStore.state = stateWithGuides; + mockStore.getState.mockReturnValue(stateWithGuides); + + const result = await new KnockGuideClient( + mockKnock, + channelId, + ).markAsInteracted(freshGuide, freshStep); + + expect(result?.message.interacted_at).toEqual(expect.any(String)); + }); + test("marks guide step as archived with bypass_global_group_limit true", async () => { // Create a fresh mock step for this test const freshMockStep = { diff --git a/packages/client/test/clients/users/users.test.ts b/packages/client/test/clients/users/users.test.ts index 5d84b1a02..5d2fb53df 100644 --- a/packages/client/test/clients/users/users.test.ts +++ b/packages/client/test/clients/users/users.test.ts @@ -993,5 +993,43 @@ describe("User Client", () => { }), ).rejects.toThrow("Guide step not found"); }); + + test("preserves the original API error and its response details", async () => { + const { knock, mockApiClient } = getTestSetup(); + const responseBody = { + error: "Invalid guide engagement", + field: "guide_step_ref", + }; + const apiError = Object.assign( + new Error("Request failed with status code 400"), + { + name: "ApiRequestError", + response: { + status: 400, + data: responseBody, + }, + }, + ); + + mockApiClient.makeRequest.mockResolvedValue({ + statusCode: "error", + status: 400, + error: apiError, + body: responseBody, + }); + + const request = knock.user.markGuideStepAs("interacted", { + guide_key: "onboarding_guide", + guide_id: "guide_456", + guide_step_ref: "step_1", + channel_id: "channel_456", + }); + + await expect(request).rejects.toBe(apiError); + expect(apiError.response).toEqual({ + status: 400, + data: responseBody, + }); + }); }); });