Skip to content

Commit

Permalink
chore(backend,nextjs,clerk-sdk-node): Drop legacy return response in …
Browse files Browse the repository at this point in the history
…BAPI responses (#2126)
  • Loading branch information
dimkl committed Nov 17, 2023
1 parent 4e844ef commit dd57030
Show file tree
Hide file tree
Showing 15 changed files with 161 additions and 130 deletions.
17 changes: 17 additions & 0 deletions .changeset/mighty-pugs-knock.md
Original file line number Diff line number Diff line change
@@ -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 });
}
```
72 changes: 30 additions & 42 deletions packages/backend/src/api/factory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand All @@ -26,22 +27,17 @@ 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');
assert.equal(payload.emailAddresses[0].emailAddress, 'john.doe@clerk.test');
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', {
Expand All @@ -59,22 +55,16 @@ 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');
assert.equal(payload[0].emailAddresses[0].emailAddress, 'john.doe@clerk.test');
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', {
Expand All @@ -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');

Expand All @@ -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', {
Expand All @@ -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', {
Expand All @@ -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', {
Expand Down
41 changes: 10 additions & 31 deletions packages/backend/src/api/request.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import { ClerkAPIResponseError } from '@clerk/shared/error';
import type { ClerkAPIError, ClerkAPIErrorJSON } from '@clerk/types';
import snakecaseKeys from 'snakecase-keys';

Expand Down Expand Up @@ -36,32 +35,11 @@ export type ClerkBackendApiResponse<T> =
data: null;
errors: ClerkAPIError[];
clerkTraceId?: string;
status?: number;
statusText?: string;
};

export type RequestFunction = ReturnType<typeof buildRequest>;
type LegacyRequestFunction = <T>(requestOptions: ClerkBackendApiRequestOptions) => Promise<T>;

/**
* 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<T>(...args);
if (errors === null) {
return data;
} else {
throw new ClerkAPIResponseError(statusText || '', {
data: errors,
status: status || '',
clerkTraceId,
});
}
};

type BuildRequestOptions = {
/* Secret Key */
Expand All @@ -73,9 +51,8 @@ type BuildRequestOptions = {
/* Library/SDK name */
userAgent?: string;
};

export function buildRequest(options: BuildRequestOptions) {
const request = async <T>(requestOptions: ClerkBackendApiRequestOptions): Promise<ClerkBackendApiResponse<T>> => {
return async <T>(requestOptions: ClerkBackendApiRequestOptions): Promise<ClerkBackendApiResponse<T>> => {
const { secretKey, apiUrl = API_URL, apiVersion = API_VERSION, userAgent = USER_AGENT } = options;
const { path, method, queryParams, headerParams, bodyParams, formData } = requestOptions;

Expand Down Expand Up @@ -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 {
Expand All @@ -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
Expand Down
9 changes: 3 additions & 6 deletions packages/backend/src/tokens/authStatus.ts
Original file line number Diff line number Diff line change
Expand Up @@ -149,12 +149,9 @@ export async function signedIn<T extends AuthStatusOptionsType>(
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,
Expand Down
9 changes: 9 additions & 0 deletions packages/backend/src/util/assertResponse.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
type ApiResponse<T> = { data: T | null; errors: null | any[] };
type SuccessApiResponse<T> = { data: T; errors: null };
type ErrorApiResponse = { data: null; errors: any[]; clerkTraceId: string; status: number; statusText: string };
export function assertResponse<T>(assert: Assert, resp: ApiResponse<T>): asserts resp is SuccessApiResponse<T> {
assert.equal(resp.errors, null);
}
export function assertErrorResponse<T>(assert: Assert, resp: ApiResponse<T>): asserts resp is ErrorApiResponse {
assert.notEqual(resp.errors, null);
}
5 changes: 4 additions & 1 deletion packages/backend/src/util/mockFetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand All @@ -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);
Expand Down
7 changes: 6 additions & 1 deletion packages/nextjs/src/app-router/server/currentUser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,10 @@ import { auth } from './auth';

export async function currentUser(): Promise<User | null> {
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;
}
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,12 @@ app.use(clerk.expressWithAuth());
app.get('/', async (req: WithAuthProp<Request>, 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
Expand Down
21 changes: 16 additions & 5 deletions packages/sdk-node/examples/node/src/organizations.ts
Original file line number Diff line number Diff line change
@@ -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);
Loading

0 comments on commit dd57030

Please sign in to comment.