diff --git a/.changeset/mighty-pugs-knock.md b/.changeset/mighty-pugs-knock.md new file mode 100644 index 00000000000..e2f52fc4ed8 --- /dev/null +++ b/.changeset/mighty-pugs-knock.md @@ -0,0 +1,17 @@ +--- +'@clerk/clerk-sdk-node': major +'@clerk/backend': major +'@clerk/nextjs': major +--- + +Change the response payload of Backend API requests to return `{ data, errors }` instead of return the data and throwing on error response. +Code example to keep the same behavior: +```typescript +import { users } from '@clerk/backend'; +import { ClerkAPIResponseError } from '@clerk/shared/error'; + +const { data, errors, clerkTraceId, status, statusText } = await users.getUser('user_deadbeef'); +if(errors){ + throw new ClerkAPIResponseError(statusText, { data: errors, status, clerkTraceId }); +} +``` \ No newline at end of file diff --git a/packages/backend/src/api/factory.test.ts b/packages/backend/src/api/factory.test.ts index 8210c6d0416..88be8073ebd 100644 --- a/packages/backend/src/api/factory.test.ts +++ b/packages/backend/src/api/factory.test.ts @@ -4,6 +4,7 @@ import sinon from 'sinon'; import emailJson from '../fixtures/responses/email.json'; import userJson from '../fixtures/responses/user.json'; import runtime from '../runtime'; +import { assertErrorResponse, assertResponse } from '../util/assertResponse'; import { jsonError, jsonNotOk, jsonOk } from '../util/mockFetch'; import { createBackendApiClient } from './factory'; @@ -26,14 +27,10 @@ export default (QUnit: QUnit) => { fakeFetch = sinon.stub(runtime, 'fetch'); fakeFetch.onCall(0).returns(jsonOk(userJson)); - const payload = await apiClient.users.getUser('user_deadbeef'); + const response = await apiClient.users.getUser('user_deadbeef'); - if (!payload) { - // eslint-disable-next-line qunit/no-conditional-assertions - assert.false(true, 'This assertion should never fail. We need to check for payload to make TS happy.'); - // eslint-disable-next-line qunit/no-early-return - return; - } + assertResponse(assert, response); + const { data: payload } = response; assert.equal(payload.firstName, 'John'); assert.equal(payload.lastName, 'Doe'); @@ -41,7 +38,6 @@ export default (QUnit: QUnit) => { assert.equal(payload.phoneNumbers[0].phoneNumber, '+311-555-2368'); assert.equal(payload.externalAccounts[0].emailAddress, 'john.doe@clerk.test'); assert.equal(payload.publicMetadata.zodiac_sign, 'leo'); - // assert.equal(payload.errors, null); assert.ok( fakeFetch.calledOnceWith('https://api.clerk.test/v1/users/user_deadbeef', { @@ -59,14 +55,9 @@ export default (QUnit: QUnit) => { fakeFetch = sinon.stub(runtime, 'fetch'); fakeFetch.onCall(0).returns(jsonOk([userJson])); - const payload = await apiClient.users.getUserList({ offset: 2, limit: 5 }); - - if (!payload) { - // eslint-disable-next-line qunit/no-conditional-assertions - assert.false(true, 'This assertion should never fail. We need to check for payload to make TS happy.'); - // eslint-disable-next-line qunit/no-early-return - return; - } + const response = await apiClient.users.getUserList({ offset: 2, limit: 5 }); + assertResponse(assert, response); + const { data: payload } = response; assert.equal(payload[0].firstName, 'John'); assert.equal(payload[0].lastName, 'Doe'); @@ -74,7 +65,6 @@ export default (QUnit: QUnit) => { assert.equal(payload[0].phoneNumbers[0].phoneNumber, '+311-555-2368'); assert.equal(payload[0].externalAccounts[0].emailAddress, 'john.doe@clerk.test'); assert.equal(payload[0].publicMetadata.zodiac_sign, 'leo'); - // assert.equal(payload.errors, null); assert.ok( fakeFetch.calledOnceWith('https://api.clerk.test/v1/users?offset=2&limit=5', { @@ -100,14 +90,10 @@ export default (QUnit: QUnit) => { }; const requestBody = '{"from_email_name":"foobar123","email_address_id":"test@test.dev","body":"this is a test","subject":"this is a test"}'; - const payload = await apiClient.emails.createEmail(body); - - if (!payload) { - // eslint-disable-next-line qunit/no-conditional-assertions - assert.false(true, 'This assertion should never fail. We need to check for payload to make TS happy.'); - // eslint-disable-next-line qunit/no-early-return - return; - } + const response = await apiClient.emails.createEmail(body); + assertResponse(assert, response); + const { data: payload } = response; + assert.equal(JSON.stringify(payload.data), '{}'); assert.equal(payload.id, 'ema_2PHa2N3bS7D6NPPQ5mpHEg0waZQ'); @@ -126,15 +112,19 @@ export default (QUnit: QUnit) => { test('executes a successful backend API request to create a new resource', async assert => { fakeFetch = sinon.stub(runtime, 'fetch'); - fakeFetch.onCall(0).returns(jsonOk([userJson])); + fakeFetch.onCall(0).returns(jsonOk(userJson)); - await apiClient.users.createUser({ + const response = await apiClient.users.createUser({ firstName: 'John', lastName: 'Doe', publicMetadata: { star_sign: 'Leon', }, }); + assertResponse(assert, response); + const { data: payload } = response; + + assert.equal(payload.firstName, 'John'); assert.ok( fakeFetch.calledOnceWith('https://api.clerk.test/v1/users', { @@ -161,14 +151,13 @@ export default (QUnit: QUnit) => { fakeFetch = sinon.stub(runtime, 'fetch'); fakeFetch.onCall(0).returns(jsonNotOk({ errors: [mockErrorPayload], clerk_trace_id: traceId })); - try { - await apiClient.users.getUser('user_deadbeef'); - } catch (e: any) { - assert.equal(e.clerkTraceId, traceId); - assert.true(e.clerkError); - assert.equal(e.status, 422); - assert.equal(e.errors[0].code, 'whatever_error'); - } + const response = await apiClient.users.getUser('user_deadbeef'); + assertErrorResponse(assert, response); + + assert.equal(response.clerkTraceId, traceId); + assert.equal(response.status, 422); + assert.equal(response.statusText, '422'); + assert.equal(response.errors[0].code, 'whatever_error'); assert.ok( fakeFetch.calledOnceWith('https://api.clerk.test/v1/users/user_deadbeef', { @@ -186,13 +175,12 @@ export default (QUnit: QUnit) => { fakeFetch = sinon.stub(runtime, 'fetch'); fakeFetch.onCall(0).returns(jsonError({ errors: [] })); - try { - await apiClient.users.getUser('user_deadbeef'); - } catch (e: any) { - assert.true(e.clerkError); - assert.equal(e.status, 500); - assert.equal(e.clerkTraceId, 'mock_cf_ray'); - } + const response = await apiClient.users.getUser('user_deadbeef'); + assertErrorResponse(assert, response); + + assert.equal(response.status, 500); + assert.equal(response.statusText, '500'); + assert.equal(response.clerkTraceId, 'mock_cf_ray'); assert.ok( fakeFetch.calledOnceWith('https://api.clerk.test/v1/users/user_deadbeef', { diff --git a/packages/backend/src/api/request.ts b/packages/backend/src/api/request.ts index f7c566a0a1d..b6ad827024a 100644 --- a/packages/backend/src/api/request.ts +++ b/packages/backend/src/api/request.ts @@ -1,4 +1,3 @@ -import { ClerkAPIResponseError } from '@clerk/shared/error'; import type { ClerkAPIError, ClerkAPIErrorJSON } from '@clerk/types'; import snakecaseKeys from 'snakecase-keys'; @@ -36,32 +35,11 @@ export type ClerkBackendApiResponse = data: null; errors: ClerkAPIError[]; clerkTraceId?: string; + status?: number; + statusText?: string; }; export type RequestFunction = ReturnType; -type LegacyRequestFunction = (requestOptions: ClerkBackendApiRequestOptions) => Promise; - -/** - * Switching to the { data, errors } format is a breaking change, so we will skip it for now - * until we release v5 of the related SDKs. - * This HOF wraps the request helper and transforms the new return to the legacy return. - * TODO: Simply remove this wrapper and the ClerkAPIResponseError before the v5 release. - */ -const withLegacyReturn = - (cb: any): LegacyRequestFunction => - async (...args) => { - // @ts-ignore - const { data, errors, status, statusText, clerkTraceId } = await cb(...args); - if (errors === null) { - return data; - } else { - throw new ClerkAPIResponseError(statusText || '', { - data: errors, - status: status || '', - clerkTraceId, - }); - } - }; type BuildRequestOptions = { /* Secret Key */ @@ -73,9 +51,8 @@ type BuildRequestOptions = { /* Library/SDK name */ userAgent?: string; }; - export function buildRequest(options: BuildRequestOptions) { - const request = async (requestOptions: ClerkBackendApiRequestOptions): Promise> => { + return async (requestOptions: ClerkBackendApiRequestOptions): Promise> => { const { secretKey, apiUrl = API_URL, apiVersion = API_VERSION, userAgent = USER_AGENT } = options; const { path, method, queryParams, headerParams, bodyParams, formData } = requestOptions; @@ -133,7 +110,13 @@ export function buildRequest(options: BuildRequestOptions) { const data = await (isJSONResponse ? res.json() : res.text()); if (!res.ok) { - throw data; + return { + data: null, + errors: data?.errors || data, + status: res?.status, + statusText: res?.statusText, + clerkTraceId: getTraceId(data, res?.headers), + }; } return { @@ -157,16 +140,12 @@ export function buildRequest(options: BuildRequestOptions) { return { data: null, errors: parseErrors(err), - // TODO: To be removed with withLegacyReturn - // @ts-expect-error status: res?.status, statusText: res?.statusText, clerkTraceId: getTraceId(err, res?.headers), }; } }; - - return withLegacyReturn(request); } // Returns either clerk_trace_id if present in response json, otherwise defaults to CF-Ray header diff --git a/packages/backend/src/tokens/authStatus.ts b/packages/backend/src/tokens/authStatus.ts index b39c53fa540..d87143643f8 100644 --- a/packages/backend/src/tokens/authStatus.ts +++ b/packages/backend/src/tokens/authStatus.ts @@ -149,12 +149,9 @@ export async function signedIn( loadOrganization && orgId ? organizations.getOrganization({ organizationId: orgId }) : Promise.resolve(undefined), ]); - const session = sessionResp; - const user = userResp; - const organization = organizationResp; - // const session = sessionResp && !sessionResp.errors ? sessionResp.data : undefined; - // const user = userResp && !userResp.errors ? userResp.data : undefined; - // const organization = organizationResp && !organizationResp.errors ? organizationResp.data : undefined; + const session = sessionResp && !sessionResp.errors ? sessionResp.data : undefined; + const user = userResp && !userResp.errors ? userResp.data : undefined; + const organization = organizationResp && !organizationResp.errors ? organizationResp.data : undefined; const authObject = signedInAuthObject( sessionClaims, diff --git a/packages/backend/src/util/assertResponse.ts b/packages/backend/src/util/assertResponse.ts new file mode 100644 index 00000000000..8cdb760ff28 --- /dev/null +++ b/packages/backend/src/util/assertResponse.ts @@ -0,0 +1,9 @@ +type ApiResponse = { data: T | null; errors: null | any[] }; +type SuccessApiResponse = { data: T; errors: null }; +type ErrorApiResponse = { data: null; errors: any[]; clerkTraceId: string; status: number; statusText: string }; +export function assertResponse(assert: Assert, resp: ApiResponse): asserts resp is SuccessApiResponse { + assert.equal(resp.errors, null); +} +export function assertErrorResponse(assert: Assert, resp: ApiResponse): asserts resp is ErrorApiResponse { + assert.notEqual(resp.errors, null); +} diff --git a/packages/backend/src/util/mockFetch.ts b/packages/backend/src/util/mockFetch.ts index 89cf9f5e686..ada6a78f2ab 100644 --- a/packages/backend/src/util/mockFetch.ts +++ b/packages/backend/src/util/mockFetch.ts @@ -5,6 +5,7 @@ export function jsonOk(body: unknown, status = 200) { const mockResponse = { ok: true, status, + statusText: status.toString(), headers: { get: mockHeadersGet }, json() { return Promise.resolve(body); @@ -19,6 +20,7 @@ export function jsonNotOk(body: unknown) { const mockResponse = { ok: false, status: 422, + statusText: 422, headers: { get: mockHeadersGet }, json() { return Promise.resolve(body); @@ -32,7 +34,8 @@ export function jsonError(body: unknown, status = 500) { // Mock response object that satisfies the window.Response interface const mockResponse = { ok: false, - status: status, + status, + statusText: status.toString(), headers: { get: mockHeadersGet }, json() { return Promise.resolve(body); diff --git a/packages/nextjs/src/app-router/server/currentUser.ts b/packages/nextjs/src/app-router/server/currentUser.ts index 6684ac8f90f..adcf72e2860 100644 --- a/packages/nextjs/src/app-router/server/currentUser.ts +++ b/packages/nextjs/src/app-router/server/currentUser.ts @@ -5,5 +5,10 @@ import { auth } from './auth'; export async function currentUser(): Promise { const { userId } = auth(); - return userId ? clerkClient.users.getUser(userId) : null; + if (!userId) return null; + + const { data, errors } = await clerkClient.users.getUser(userId); + if (errors) return null; + + return data; } diff --git a/packages/sdk-node/examples/express/src/runtime-keys-middleware.ts b/packages/sdk-node/examples/express/src/runtime-keys-middleware.ts index 6f10d179648..11a442f7122 100644 --- a/packages/sdk-node/examples/express/src/runtime-keys-middleware.ts +++ b/packages/sdk-node/examples/express/src/runtime-keys-middleware.ts @@ -21,8 +21,12 @@ app.use(clerk.expressWithAuth()); app.get('/', async (req: WithAuthProp, res: Response) => { const { userId, debug } = req.auth; console.log(debug()); - const user = userId ? await clerk.users.getUser(userId) : null; - res.json({ auth: req.auth, user }); + if (!userId) return res.json({ auth: req.auth, user: null }); + + const { data, errors } = await clerk.users.getUser(userId); + if (errors) return res.json({ auth: req.auth, user: null }); + + return res.json({ auth: req.auth, user: data });; }); // @ts-ignore diff --git a/packages/sdk-node/examples/node/src/organizations.ts b/packages/sdk-node/examples/node/src/organizations.ts index 3661d2f5c47..d225b83c3ca 100644 --- a/packages/sdk-node/examples/node/src/organizations.ts +++ b/packages/sdk-node/examples/node/src/organizations.ts @@ -1,18 +1,29 @@ import { organizations, users } from '@clerk/clerk-sdk-node'; console.log('Get user to create organization'); -const [creator] = await users.getUserList(); +const { data, errors } = await users.getUserList(); +if (errors) { + throw new Error(errors); +} + +const creator = data[0]; console.log('Create organization'); -const organization = await organizations.createOrganization({ +const { data: organization } = await organizations.createOrganization({ name: 'test-organization', createdBy: creator.id, }); console.log(organization); console.log('Update organization metadata'); -const updatedOrganizationMetadata = - await organizations.updateOrganizationMetadata(organization.id, { +const { data: updatedOrganizationMetadata, errors: uomErrors } = await organizations.updateOrganizationMetadata( + organization.id, + { publicMetadata: { test: 1 }, - }); + }, +); +if (uomErrors) { + throw new Error(uomErrors); +} + console.log(updatedOrganizationMetadata); diff --git a/packages/sdk-node/examples/node/src/sessions.ts b/packages/sdk-node/examples/node/src/sessions.ts index 2f52e4287f8..1b4c3e56fcd 100644 --- a/packages/sdk-node/examples/node/src/sessions.ts +++ b/packages/sdk-node/examples/node/src/sessions.ts @@ -9,33 +9,25 @@ const sessionIdtoRevoke = process.env.SESSION_ID_TO_REVOKE || ''; const sessionToken = process.env.SESSION_TOKEN || ''; console.log('Get session list'); -const sessionList = await sessions.getSessionList(); +const { data: sessionList } = await sessions.getSessionList(); console.log(sessionList); console.log('Get session list filtered by userId'); -const filteredSessions1 = await sessions.getSessionList({ userId }); +const { data: filteredSessions1 } = await sessions.getSessionList({ userId }); console.log(filteredSessions1); console.log('Get session list filtered by clientId'); -const filteredSessions2 = await sessions.getSessionList({ clientId }); +const { data: filteredSessions2 } = await sessions.getSessionList({ clientId }); console.log(filteredSessions2); console.log('Get single session'); -const session = await sessions.getSession(sessionId); +const { data: session } = await sessions.getSession(sessionId); console.log(session); -try { - console.log('Revoke session'); - const revokedSession = await sessions.revokeSession(sessionIdtoRevoke); - console.log(revokedSession); -} catch (error) { - console.log(error); -} +console.log('Revoke session'); +const { data: revokedSession } = await sessions.revokeSession(sessionIdtoRevoke); +console.log(revokedSession); -try { - console.log('Verify session'); - const verifiedSession = await sessions.verifySession(sessionId, sessionToken); - console.log(verifiedSession); -} catch (error) { - console.log(error); -} +console.log('Verify session'); +const { data: verifiedSession } = await sessions.verifySession(sessionId, sessionToken); +console.log(verifiedSession); diff --git a/packages/sdk-node/examples/node/src/users.ts b/packages/sdk-node/examples/node/src/users.ts index 0e9a354b4b6..9ee3a2f0f15 100644 --- a/packages/sdk-node/examples/node/src/users.ts +++ b/packages/sdk-node/examples/node/src/users.ts @@ -3,7 +3,7 @@ import { users } from '@clerk/clerk-sdk-node'; console.log('Create user'); -const createdUser = await users.createUser({ +const { data: createdUser, errors: createUserErrors } = await users.createUser({ emailAddress: ['test@example.com'], phoneNumber: ['+15555555555'], externalId: 'a-unique-id', @@ -21,36 +21,46 @@ const createdUser = await users.createUser({ }, password: '123456+ABCd', }); +if (createUserErrors) { + throw new Error(createUserErrors); +} console.log(createdUser); const createdUserId = createdUser.id as string; console.log('Get single user'); -const user = await users.getUser(createdUserId); +const { data: user, errors: userErrors } = await users.getUser(createdUserId); +if (userErrors) { + throw new Error(userErrors); +} console.log(user); await users.deleteUser(createdUserId); console.log('Get user list'); -const userList = await users.getUserList(); +const { data: userList, errors: userListErrors } = await users.getUserList(); +if (userListErrors) { + throw new Error(userListErrors); +} console.log(userList); -try { - console.log('Update user'); - - const updatedUser = await users.updateUser(createdUserId, { - firstName: 'Kyle', - lastName: 'Reese', - publicMetadata: { - zodiac_sign: 'leo', - ascendant: 'scorpio', - }, - }); +console.log('Update user'); - console.log(updatedUser); -} catch (error) { - console.log(error); +const { user: updatedUser, errors: updateUserErrors } = await users.updateUser(createdUserId, { + firstName: 'Kyle', + lastName: 'Reese', + publicMetadata: { + zodiac_sign: 'leo', + ascendant: 'scorpio', + }, +}); +if (updateUserErrors) { + throw new Error(updateUserErrors); } +console.log(updatedUser); console.log('Get total count of users'); -const count = await users.getCount(); +const { data: count, errors: countErrors } = await users.getCount(); +if (countErrors) { + throw new Error(countErrors); +} console.log(count); diff --git a/packages/sdk-node/src/__tests__/middleware.test.ts b/packages/sdk-node/src/__tests__/middleware.test.ts index dd62716deb4..d35fe2c0966 100644 --- a/packages/sdk-node/src/__tests__/middleware.test.ts +++ b/packages/sdk-node/src/__tests__/middleware.test.ts @@ -87,7 +87,7 @@ describe('ClerkExpressWithAuth', () => { isUnknown: false, toAuth: () => ({ sessionId: '1' }), } as unknown as RequestState); - clerkClient.remotePrivateInterstitial.mockReturnValue('interstitial'); + clerkClient.remotePrivateInterstitial.mockReturnValue({ data: 'interstitial', errors: null }); await createClerkExpressWithAuth({ clerkClient })()(req, res, mockNext as NextFunction); @@ -189,7 +189,7 @@ describe('ClerkExpressRequireAuth', () => { isUnknown: false, toAuth: () => ({ sessionId: '1' }), } as unknown as RequestState); - clerkClient.remotePrivateInterstitial.mockReturnValue('interstitial'); + clerkClient.remotePrivateInterstitial.mockReturnValue({ data: 'interstitial', errors: null }); await createClerkExpressRequireAuth({ clerkClient })()(req, res, mockNext as NextFunction); diff --git a/packages/sdk-node/src/authenticateRequest.ts b/packages/sdk-node/src/authenticateRequest.ts index 383bb065557..ce0eb02bfba 100644 --- a/packages/sdk-node/src/authenticateRequest.ts +++ b/packages/sdk-node/src/authenticateRequest.ts @@ -20,7 +20,7 @@ export async function loadInterstitial({ * and avoid the extra network call */ if (requestState.publishableKey) { - return clerkClient.localInterstitial({ + const data = clerkClient.localInterstitial({ publishableKey: requestState.publishableKey, proxyUrl: requestState.proxyUrl, signInUrl: requestState.signInUrl, @@ -29,8 +29,14 @@ export async function loadInterstitial({ clerkJSVersion, clerkJSUrl, }); + + return { + data, + errors: null, + }; } - return await clerkClient.remotePrivateInterstitial(); + + return clerkClient.remotePrivateInterstitial(); } export const authenticateRequest = (opts: AuthenticateRequestParams) => { diff --git a/packages/sdk-node/src/clerkExpressRequireAuth.ts b/packages/sdk-node/src/clerkExpressRequireAuth.ts index 884ad1a9038..6bb3b22c677 100644 --- a/packages/sdk-node/src/clerkExpressRequireAuth.ts +++ b/packages/sdk-node/src/clerkExpressRequireAuth.ts @@ -38,7 +38,12 @@ export const createClerkExpressRequireAuth = (createOpts: CreateClerkExpressMidd clerkClient, requestState, }); - return handleInterstitialCase(res, requestState, interstitial); + if (interstitial.errors) { + // TODO(@dimkl): return interstitial errors ? + next(new Error('Unauthenticated')); + return; + } + return handleInterstitialCase(res, requestState, interstitial.data); } if (requestState.isSignedIn) { diff --git a/packages/sdk-node/src/clerkExpressWithAuth.ts b/packages/sdk-node/src/clerkExpressWithAuth.ts index 5d59ed3cf85..2407a624f89 100644 --- a/packages/sdk-node/src/clerkExpressWithAuth.ts +++ b/packages/sdk-node/src/clerkExpressWithAuth.ts @@ -28,7 +28,12 @@ export const createClerkExpressWithAuth = (createOpts: CreateClerkExpressMiddlew clerkClient, requestState, }); - return handleInterstitialCase(res, requestState, interstitial); + if (interstitial.errors) { + // TODO(@dimkl): return interstitial errors ? + next(new Error('Unauthenticated')); + return; + } + return handleInterstitialCase(res, requestState, interstitial.data); } (req as WithAuthProp).auth = {