Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(backend,nextjs,clerk-sdk-node): Drop legacy return response in BAPI responses #2126

Merged
merged 1 commit into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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.
dimkl marked this conversation as resolved.
Show resolved Hide resolved
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;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertResponse / assertErrorResponse helpers were introduced to drop the !payload check.

assertResponse(assert, response);
const { data: payload } = response;

assert.equal(payload.firstName, 'John');
assert.equal(payload.lastName, 'Doe');
assert.equal(payload.emailAddresses[0].emailAddress, '[email protected]');
assert.equal(payload.phoneNumbers[0].phoneNumber, '+311-555-2368');
assert.equal(payload.externalAccounts[0].emailAddress, '[email protected]');
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, '[email protected]');
assert.equal(payload[0].phoneNumbers[0].phoneNumber, '+311-555-2368');
assert.equal(payload[0].externalAccounts[0].emailAddress, '[email protected]');
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":"[email protected]","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
Loading