From 2afd0e6a0ebcce7c013d86432152a0399143517f Mon Sep 17 00:00:00 2001 From: Stefanos Anagnostou Date: Fri, 21 Mar 2025 12:51:27 +0200 Subject: [PATCH 1/6] fix(clerk-js): Handle empty responses from FAPI --- .changeset/ninety-bobcats-train.md | 5 +++++ packages/clerk-js/src/core/fapiClient.ts | 7 ++++++- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 .changeset/ninety-bobcats-train.md diff --git a/.changeset/ninety-bobcats-train.md b/.changeset/ninety-bobcats-train.md new file mode 100644 index 00000000000..5fa274239d9 --- /dev/null +++ b/.changeset/ninety-bobcats-train.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': patch +--- + +Handle the empty response diff --git a/packages/clerk-js/src/core/fapiClient.ts b/packages/clerk-js/src/core/fapiClient.ts index f996ccd8db4..dfe60e0aee0 100644 --- a/packages/clerk-js/src/core/fapiClient.ts +++ b/packages/clerk-js/src/core/fapiClient.ts @@ -250,7 +250,12 @@ export function createFapiClient(options: FapiClientOptions): FapiClient { clerkNetworkError(urlStr, e); } - const json: FapiResponseJSON = await response.json(); + let json: FapiResponseJSON | null = null; + try { + json = await response.json(); + } catch { + json = null; + } const fapiResponse: FapiResponse = Object.assign(response, { payload: json }); await runAfterResponseCallbacks(requestInit, fapiResponse); return fapiResponse; From 70a42cfa7d4e524e551ecf9c72a4b08e84bfaeff Mon Sep 17 00:00:00 2001 From: Stefanos Anagnostou Date: Fri, 21 Mar 2025 13:14:34 +0200 Subject: [PATCH 2/6] update changeset --- .changeset/ninety-bobcats-train.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/ninety-bobcats-train.md b/.changeset/ninety-bobcats-train.md index 5fa274239d9..4f9087a5aa0 100644 --- a/.changeset/ninety-bobcats-train.md +++ b/.changeset/ninety-bobcats-train.md @@ -2,4 +2,4 @@ '@clerk/clerk-js': patch --- -Handle the empty response +Handle the empty body on 2xx responses from FAPI From 1461385d4b0a3d64508054e4058d8a38587d8677 Mon Sep 17 00:00:00 2001 From: Stefanos Anagnostou Date: Fri, 21 Mar 2025 16:10:43 +0200 Subject: [PATCH 3/6] handle empty body on 204 status --- .changeset/ninety-bobcats-train.md | 2 +- packages/clerk-js/src/core/fapiClient.ts | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/.changeset/ninety-bobcats-train.md b/.changeset/ninety-bobcats-train.md index 4f9087a5aa0..55b10049983 100644 --- a/.changeset/ninety-bobcats-train.md +++ b/.changeset/ninety-bobcats-train.md @@ -2,4 +2,4 @@ '@clerk/clerk-js': patch --- -Handle the empty body on 2xx responses from FAPI +Handle the empty body on 204 responses from FAPI diff --git a/packages/clerk-js/src/core/fapiClient.ts b/packages/clerk-js/src/core/fapiClient.ts index dfe60e0aee0..a030ffaf4bb 100644 --- a/packages/clerk-js/src/core/fapiClient.ts +++ b/packages/clerk-js/src/core/fapiClient.ts @@ -251,9 +251,10 @@ export function createFapiClient(options: FapiClientOptions): FapiClient { } let json: FapiResponseJSON | null = null; - try { + // 204 No Content responses do not have a body so we should not try to parse it + if (response.status !== 204) { json = await response.json(); - } catch { + } else { json = null; } const fapiResponse: FapiResponse = Object.assign(response, { payload: json }); From 381b44a9cd876d942166eab67a93222fa1a3bda2 Mon Sep 17 00:00:00 2001 From: Stefanos Anagnostou Date: Fri, 21 Mar 2025 16:25:01 +0200 Subject: [PATCH 4/6] simplify the logic --- packages/clerk-js/src/core/fapiClient.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/packages/clerk-js/src/core/fapiClient.ts b/packages/clerk-js/src/core/fapiClient.ts index a030ffaf4bb..37f9dedafe5 100644 --- a/packages/clerk-js/src/core/fapiClient.ts +++ b/packages/clerk-js/src/core/fapiClient.ts @@ -250,13 +250,8 @@ export function createFapiClient(options: FapiClientOptions): FapiClient { clerkNetworkError(urlStr, e); } - let json: FapiResponseJSON | null = null; // 204 No Content responses do not have a body so we should not try to parse it - if (response.status !== 204) { - json = await response.json(); - } else { - json = null; - } + const json: FapiResponseJSON | null = response.status !== 204 ? await response.json() : null; const fapiResponse: FapiResponse = Object.assign(response, { payload: json }); await runAfterResponseCallbacks(requestInit, fapiResponse); return fapiResponse; From 0937f4cf75955124c4156010490e031d377f9044 Mon Sep 17 00:00:00 2001 From: Stefanos Anagnostou Date: Wed, 26 Mar 2025 13:18:55 +0200 Subject: [PATCH 5/6] Add test --- .../src/core/__tests__/fapiClient.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/clerk-js/src/core/__tests__/fapiClient.test.ts b/packages/clerk-js/src/core/__tests__/fapiClient.test.ts index cc93181cb5d..5e4ef4723f4 100644 --- a/packages/clerk-js/src/core/__tests__/fapiClient.test.ts +++ b/packages/clerk-js/src/core/__tests__/fapiClient.test.ts @@ -200,6 +200,23 @@ describe('request', () => { expect(Array.isArray(resp.payload)).toEqual(true); }); + it('handles the empty body on 204 response, returning null', async () => { + (global.fetch as jest.Mock).mockResolvedValueOnce( + Promise.resolve>({ + status: 204, + json: () => { + throw new Error('json should not be called on 204 response'); + }, + }), + ); + + const resp = await fapiClient.request({ + path: '/foo', + }); + + expect(resp.payload).toEqual(null); + }); + describe('for production instances', () => { it.todo('does not append the __clerk_db_jwt cookie value to the query string'); it.todo('does not set the __clerk_db_jwt cookie from the response Clerk-Cookie header'); From 4c69badb60b6a982ea2ac0bf1f3af0d04a7c0697 Mon Sep 17 00:00:00 2001 From: Stefanos Anagnostou Date: Wed, 26 Mar 2025 13:38:52 +0200 Subject: [PATCH 6/6] Add test on sendCaptchaToken --- .../core/resources/__tests__/Client.test.ts | 83 +++++++++++++------ 1 file changed, 58 insertions(+), 25 deletions(-) diff --git a/packages/clerk-js/src/core/resources/__tests__/Client.test.ts b/packages/clerk-js/src/core/resources/__tests__/Client.test.ts index 635a85adcc0..fada67f9f5f 100644 --- a/packages/clerk-js/src/core/resources/__tests__/Client.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/Client.test.ts @@ -4,33 +4,66 @@ import { createSession, createSignIn, createSignUp, createUser } from '../../tes import { BaseResource, Client } from '../internal'; describe('Client Singleton', () => { - it('sends captcha token', async () => { - const user = createUser({ first_name: 'John', last_name: 'Doe', id: 'user_1' }); - const session = createSession({ id: 'session_1' }, user); - const clientObjectJSON: ClientJSON = { - object: 'client', - id: 'test_id', - status: 'active', - last_active_session_id: 'test_session_id', - sign_in: createSignIn({ id: 'test_sign_in_id' }, user), - sign_up: createSignUp({ id: 'test_sign_up_id' }), // This is only for testing purposes, this will never happen - sessions: [session], - created_at: jest.now() - 1000, - updated_at: jest.now(), - } as any; + describe('sendCaptchaToken', () => { + it('sends captcha token', async () => { + const user = createUser({ first_name: 'John', last_name: 'Doe', id: 'user_1' }); + const session = createSession({ id: 'session_1' }, user); + const clientObjectJSON: ClientJSON = { + object: 'client', + id: 'test_id', + status: 'active', + last_active_session_id: 'test_session_id', + sign_in: createSignIn({ id: 'test_sign_in_id' }, user), + sign_up: createSignUp({ id: 'test_sign_up_id' }), // This is only for testing purposes, this will never happen + sessions: [session], + created_at: jest.now() - 1000, + updated_at: jest.now(), + } as any; - // @ts-expect-error This is a private method that we are mocking - BaseResource._baseFetch = jest.fn(); + // @ts-expect-error This is a private method that we are mocking + BaseResource._baseFetch = jest.fn(); - const client = Client.getOrCreateInstance().fromJSON(clientObjectJSON); - await client.sendCaptchaToken({ captcha_token: 'test_captcha_token' }); - // @ts-expect-error This is a private method that we are mocking - expect(BaseResource._baseFetch).toHaveBeenCalledWith({ - method: 'POST', - path: `/client/verify`, - body: { - captcha_token: 'test_captcha_token', - }, + const client = Client.getOrCreateInstance().fromJSON(clientObjectJSON); + await client.sendCaptchaToken({ captcha_token: 'test_captcha_token' }); + // @ts-expect-error This is a private method that we are mocking + expect(BaseResource._baseFetch).toHaveBeenCalledWith({ + method: 'POST', + path: `/client/verify`, + body: { + captcha_token: 'test_captcha_token', + }, + }); + }); + + it('ignores null response payload', async () => { + const user = createUser({ first_name: 'John', last_name: 'Doe', id: 'user_1' }); + const session = createSession({ id: 'session_1' }, user); + const clientObjectJSON: ClientJSON = { + object: 'client', + id: 'test_id', + status: 'active', + last_active_session_id: 'test_session_id', + sign_in: createSignIn({ id: 'test_sign_in_id' }, user), + sign_up: createSignUp({ id: 'test_sign_up_id' }), // This is only for testing purposes, this will never happen + sessions: [session], + created_at: jest.now() - 1000, + updated_at: jest.now(), + } as any; + + // @ts-expect-error This is a private method that we are mocking + BaseResource._baseFetch = jest.fn().mockResolvedValueOnce(Promise.resolve(null)); + + const client = Client.getOrCreateInstance().fromJSON(clientObjectJSON); + await client.sendCaptchaToken({ captcha_token: 'test_captcha_token' }); + // @ts-expect-error This is a private method that we are mocking + expect(BaseResource._baseFetch).toHaveBeenCalledWith({ + method: 'POST', + path: `/client/verify`, + body: { + captcha_token: 'test_captcha_token', + }, + }); + expect(client.id).toBe('test_id'); }); });