Skip to content
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
5 changes: 5 additions & 0 deletions .changeset/tiny-taxis-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@clerk/clerk-js': patch
---

Update token refresh mechanism to handle network failures without raising an error
78 changes: 66 additions & 12 deletions packages/clerk-js/src/core/resources/Session.test.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import type { SessionJSON } from '@clerk/types';

import { eventBus } from '../events';
import { clerkMock, createUser } from '../test/fixtures';
import { createFapiClient } from '../fapiClient';
import { clerkMock, createUser, mockDevClerkInstance, mockJwt, mockNetworkFailedFetch } from '../test/fixtures';
import { BaseResource, Session } from './internal';

export const mockJwt =
'eyJhbGciOiJSUzI1NiIsImtpZCI6Imluc18yR0lvUWhiVXB5MGhYN0IyY1ZrdVRNaW5Yb0QiLCJ0eXAiOiJKV1QifQ.eyJhenAiOiJodHRwczovL2FjY291bnRzLmluc3BpcmVkLnB1bWEtNzQubGNsLmRldiIsImV4cCI6MTY2NjY0ODMxMCwiaWF0IjoxNjY2NjQ4MjUwLCJpc3MiOiJodHRwczovL2NsZXJrLmluc3BpcmVkLnB1bWEtNzQubGNsLmRldiIsIm5iZiI6MTY2NjY0ODI0MCwic2lkIjoic2Vzc18yR2JEQjRlbk5kQ2E1dlMxenBDM1h6Zzl0SzkiLCJzdWIiOiJ1c2VyXzJHSXBYT0VwVnlKdzUxcmtabjlLbW5jNlN4ciJ9.n1Usc-DLDftqA0Xb-_2w8IGs4yjCmwc5RngwbSRvwevuZOIuRoeHmE2sgCdEvjfJEa7ewL6EVGVcM557TWPW--g_J1XQPwBy8tXfz7-S73CEuyRFiR97L2AHRdvRtvGtwR-o6l8aHaFxtlmfWbQXfg4kFJz2UGe9afmh3U9-f_4JOZ5fa3mI98UMy1-bo20vjXeWQ9aGrqaxHQxjnzzC-1Kpi5LdPvhQ16H0dPB8MHRTSM5TAuLKTpPV7wqixmbtcc2-0k6b9FKYZNqRVTaIyV-lifZloBvdzlfOF8nW1VVH_fx-iW5Q3hovHFcJIULHEC1kcAYTubbxzpgeVQepGg';

describe('Session', () => {
describe('getToken()', () => {
describe('creating new session', () => {
let dispatchSpy;

beforeEach(() => {
Expand All @@ -19,42 +17,98 @@ describe('Session', () => {
afterEach(() => {
dispatchSpy?.mockRestore();
BaseResource.clerk = null as any;
// @ts-ignore
global.fetch?.mockClear();
});

it('dispatches token:update event on getToken', async () => {
const session = new Session({
it('dispatches token:update event on initilization with lastActiveToken', () => {
new Session({
status: 'active',
id: 'session_1',
object: 'session',
user: createUser({}),
last_active_organization_id: 'activeOrganization',
last_active_token: { object: 'token', jwt: mockJwt },
actor: null,
created_at: new Date().getTime(),
updated_at: new Date().getTime(),
} as SessionJSON);

await session.getToken();

expect(dispatchSpy).toBeCalledTimes(1);
expect(dispatchSpy.mock.calls[0]).toMatchSnapshot();
});
});

it('dispatches token:update event on initilization with lastActiveToken', () => {
new Session({
describe('getToken()', () => {
let dispatchSpy;

beforeEach(() => {
dispatchSpy = jest.spyOn(eventBus, 'dispatch');
BaseResource.clerk = clerkMock() as any;
});

afterEach(() => {
dispatchSpy?.mockRestore();
BaseResource.clerk = null as any;
});

it('dispatches token:update event on getToken', async () => {
const session = new Session({
status: 'active',
id: 'session_1',
object: 'session',
user: createUser({}),
last_active_organization_id: 'activeOrganization',
last_active_token: { object: 'token', jwt: mockJwt },
actor: null,
created_at: new Date().getTime(),
updated_at: new Date().getTime(),
} as SessionJSON);

await session.getToken();

expect(dispatchSpy).toBeCalledTimes(1);
expect(dispatchSpy.mock.calls[0]).toMatchSnapshot();
});

describe('with offline browser and network failure', () => {
let warnSpy;
beforeEach(() => {
Object.defineProperty(window.navigator, 'onLine', {
writable: true,
value: false,
});
warnSpy = jest.spyOn(console, 'warn').mockReturnValue();
});

afterEach(() => {
Object.defineProperty(window.navigator, 'onLine', {
writable: true,
value: true,
});
warnSpy.mockRestore();
});

it('returns null', async () => {
const session = new Session({
status: 'active',
id: 'session_1',
object: 'session',
user: createUser({}),
last_active_organization_id: 'activeOrganization',
actor: null,
created_at: new Date().getTime(),
updated_at: new Date().getTime(),
} as SessionJSON);

mockNetworkFailedFetch();
BaseResource.clerk = { getFapiClient: () => createFapiClient(mockDevClerkInstance) } as any;

const token = await session.getToken();

expect(dispatchSpy).toBeCalledTimes(1);
expect(token).toEqual(null);
});
});
});

describe('isAuthorized()', () => {
Expand Down
8 changes: 5 additions & 3 deletions packages/clerk-js/src/core/resources/Session.ts
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,12 @@ export class Session extends BaseResource implements SessionResource {
const cachedEntry = skipCache ? undefined : SessionTokenCache.get({ tokenId }, leewayInSeconds);

if (cachedEntry) {
const cachedToken = await cachedEntry.tokenResolver.then(res => res);
const cachedToken = await cachedEntry.tokenResolver;
if (!template) {
eventBus.dispatch(events.TokenUpdate, { token: cachedToken });
}
return cachedToken.getRawString();
// Return null when raw string is empty to indicate that there it's signed-out
return cachedToken.getRawString() || null;
}
const path = template ? `${this.path()}/tokens/${template}` : `${this.path()}/tokens`;
const tokenResolver = Token.create(path);
Expand All @@ -174,7 +175,8 @@ export class Session extends BaseResource implements SessionResource {
if (!template) {
eventBus.dispatch(events.TokenUpdate, { token });
}
return token.getRawString();
// Return null when raw string is empty to indicate that there it's signed-out
return token.getRawString() || null;
});
}
}
98 changes: 98 additions & 0 deletions packages/clerk-js/src/core/resources/Token.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
import { createFapiClient } from '../fapiClient';
import { mockDevClerkInstance, mockFetch, mockNetworkFailedFetch } from '../test/fixtures';
import { BaseResource } from './internal';
import { Token } from './Token';

describe('Token', () => {
describe('create', () => {
afterEach(() => {
// @ts-ignore
global.fetch?.mockClear();
BaseResource.clerk = null as any;
});

it('with http 500 throws error', async () => {
mockFetch(false, 500);
BaseResource.clerk = { getFapiClient: () => createFapiClient(mockDevClerkInstance) } as any;

await expect(Token.create('/path/to/tokens')).rejects.toMatchObject({
message: '500',
});

expect(global.fetch).toHaveBeenCalledWith(
'https://clerk.example.com/v1/path/to/tokens?_clerk_js_version=test-0.0.0',
// TODO(dimkl): omit extra params from fetch request (eg path, url) - remove expect.objectContaining
expect.objectContaining({
method: 'POST',
body: '',
credentials: 'include',
headers: new Headers(),
}),
);
});

describe('with offline browser and network failure', () => {
let warnSpy;

beforeEach(() => {
Object.defineProperty(window.navigator, 'onLine', {
writable: true,
value: false,
});
warnSpy = jest.spyOn(console, 'warn').mockReturnValue();
});

afterEach(() => {
Object.defineProperty(window.navigator, 'onLine', {
writable: true,
value: true,
});
warnSpy.mockRestore();
});

it('create returns empty raw string', async () => {
mockNetworkFailedFetch();
BaseResource.clerk = { getFapiClient: () => createFapiClient(mockDevClerkInstance) } as any;

const token = await Token.create('/path/to/tokens');

expect(global.fetch).toHaveBeenCalledWith(
'https://clerk.example.com/v1/path/to/tokens?_clerk_js_version=test-0.0.0',
// TODO(dimkl): omit extra params from fetch request (eg path, url) - remove expect.objectContaining
expect.objectContaining({
method: 'POST',
body: '',
credentials: 'include',
headers: new Headers(),
}),
);

expect(token.getRawString()).toEqual('');
expect(warnSpy).toBeCalled();
});
});

describe('with online browser and network failure', () => {
it('throws error', async () => {
mockNetworkFailedFetch();
BaseResource.clerk = { getFapiClient: () => createFapiClient(mockDevClerkInstance) } as any;

await expect(Token.create('/path/to/tokens')).rejects.toMatchObject({
message:
'ClerkJS: Network error at "https://clerk.example.com/v1/path/to/tokens?_clerk_js_version=test-0.0.0" - TypeError: Failed to fetch. Please try again.',
});

expect(global.fetch).toHaveBeenCalledWith(
'https://clerk.example.com/v1/path/to/tokens?_clerk_js_version=test-0.0.0',
// TODO(dimkl): omit extra params from fetch request (eg path, url) - remove expect.objectContaining
expect.objectContaining({
method: 'POST',
body: '',
credentials: 'include',
headers: new Headers(),
}),
);
});
});
});
});
10 changes: 6 additions & 4 deletions packages/clerk-js/src/core/resources/Token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { BaseResource } from './internal';
export class Token extends BaseResource implements TokenResource {
pathRoot = 'tokens';

jwt: JWT;
jwt?: JWT;

static async create(path: string, body: any = {}): Promise<TokenResource> {
const json = (await BaseResource._fetch<TokenJSON>({
Expand All @@ -18,18 +18,20 @@ export class Token extends BaseResource implements TokenResource {
return new Token(json, path);
}

constructor(data: TokenJSON, pathRoot?: string) {
constructor(data: TokenJSON | null, pathRoot?: string) {
super();

if (pathRoot) {
this.pathRoot = pathRoot;
}

this.jwt = decode(data.jwt);
if (data?.jwt) {
this.jwt = decode(data.jwt);
}
}

getRawString = (): string => {
return this.jwt?.claims.__raw;
return this.jwt?.claims.__raw || '';
};

protected fromJSON(data: TokenJSON | null): this {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`Session getToken() dispatches token:update event on getToken 1`] = `
exports[`Session creating new session dispatches token:update event on initilization with lastActiveToken 1`] = `
[
"token:update",
{
Expand Down Expand Up @@ -28,13 +28,13 @@ exports[`Session getToken() dispatches token:update event on getToken 1`] = `
"typ": "JWT",
},
},
"pathRoot": "/client/sessions/session_1/tokens",
"pathRoot": "tokens",
},
},
]
`;

exports[`Session getToken() dispatches token:update event on initilization with lastActiveToken 1`] = `
exports[`Session getToken() dispatches token:update event on getToken 1`] = `
[
"token:update",
{
Expand Down Expand Up @@ -62,7 +62,7 @@ exports[`Session getToken() dispatches token:update event on initilization with
"typ": "JWT",
},
},
"pathRoot": "tokens",
"pathRoot": "/client/sessions/session_1/tokens",
},
},
]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ export class SessionCookieService {
}

private updateSessionCookie(token: TokenResource | string | undefined | null) {
if (token) {
return setSessionCookie(typeof token === 'string' ? token : token.getRawString());
const rawToken = typeof token === 'string' ? token : token?.getRawString();

if (rawToken) {
return setSessionCookie(rawToken);
}
return removeSessionCookie();
}
Expand Down
31 changes: 31 additions & 0 deletions packages/clerk-js/src/core/test/fixtures.ts
Original file line number Diff line number Diff line change
Expand Up @@ -251,3 +251,34 @@ export const clerkMock = () => {
}),
};
};

type RecursivePartial<T> = {
[P in keyof T]?: RecursivePartial<T[P]>;
};

export const mockFetch = (ok = true, status = 200, responsePayload = {}) => {
// @ts-ignore
global.fetch = jest.fn(() => {
return Promise.resolve<RecursivePartial<Response>>({
status,
statusText: status.toString(),
ok,
json: () => Promise.resolve(responsePayload),
});
});
};

export const mockNetworkFailedFetch = () => {
// @ts-ignore
global.fetch = jest.fn(() => {
return Promise.reject(new TypeError('Failed to fetch'));
});
};

export const mockDevClerkInstance = {
frontendApi: 'clerk.example.com',
instanceType: 'development',
isSatellite: false,
version: 'test-0.0.0',
domain: '',
};
4 changes: 4 additions & 0 deletions packages/clerk-js/src/core/tokenCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,10 @@ const MemoryTokenCache = (prefix = KEY_PREFIX): TokenCache => {

entry.tokenResolver
.then(newToken => {
if (!newToken.jwt) {
return deleteKey();
}
Comment on lines +83 to +85

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Was this issue caused by an error thrown inside the tokenResolver promise?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This change attempts to fix the case where the network is offline causing the Token.create to return a newToken : null. Currently an error related to Cannot read properties of null (reading 'jwt') is thrown due to code in the following lines. To mitigate that (with the code above) I've added this guard to delete the key (if it exists) and return since there is no cookie to set.
Does this make any sense?


const expiresAt = newToken.jwt.claims.exp;
const issuedAt = newToken.jwt.claims.iat;
const expiresIn: Seconds = expiresAt - issuedAt;
Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,6 @@ import type { JWT } from './jwt';
import type { ClerkResource } from './resource';

export interface TokenResource extends ClerkResource {
jwt: JWT;
jwt?: JWT;
getRawString: () => string;
}