chore(clerk-js): Improve Token.create to handle error due to network …

…failure (#3035) Error resolved by returning an empty string or null instead of trying to decode the `jwt` property from the response data. After this fix an `token.getRawString()` will return an empty string and `session.getToken()` will return `null` on Network failure and when request fails and browser is offline. Current error message example: ``` Error: ClerkJS: Token refresh failed (error='Cannot read properties of null (reading 'jwt')') ```
clerk · Mar 28, 2024 · c3dccfc · c3dccfc
1 parent d1dc44c
commit c3dccfc
Show file tree

Hide file tree

Showing 10 changed files with 224 additions and 26 deletions.
diff --git a/.changeset/tiny-taxis-smell.md b/.changeset/tiny-taxis-smell.md
@@ -0,0 +1,5 @@
+---
+'@clerk/clerk-js': patch
+---
+
+Update token refresh mechanism to handle network failures without raising an error
diff --git a/packages/clerk-js/src/core/resources/Session.test.ts b/packages/clerk-js/src/core/resources/Session.test.ts
@@ -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(() => {
@@ -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()', () => {

diff --git a/packages/clerk-js/src/core/resources/Session.ts b/packages/clerk-js/src/core/resources/Session.ts
@@ -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);
@@ -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;
     });
   }
 }
diff --git a/packages/clerk-js/src/core/resources/Token.test.ts b/packages/clerk-js/src/core/resources/Token.test.ts
@@ -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(),
+          }),
+        );
+      });
+    });
+  });
+});
diff --git a/packages/clerk-js/src/core/resources/Token.ts b/packages/clerk-js/src/core/resources/Token.ts
@@ -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>({
@@ -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 {

diff --git a/packages/clerk-js/src/core/resources/__snapshots__/Session.test.ts.snap b/packages/clerk-js/src/core/resources/__snapshots__/Session.test.ts.snap
@@ -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",
   {
@@ -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",
   {
@@ -62,7 +62,7 @@ exports[`Session getToken() dispatches token:update event on initilization with
           "typ": "JWT",
         },
       },
-      "pathRoot": "tokens",
+      "pathRoot": "/client/sessions/session_1/tokens",
     },
   },
 ]

diff --git a/packages/clerk-js/src/core/services/authentication/SessionCookieService.ts b/packages/clerk-js/src/core/services/authentication/SessionCookieService.ts
@@ -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();
   }

diff --git a/packages/clerk-js/src/core/test/fixtures.ts b/packages/clerk-js/src/core/test/fixtures.ts
@@ -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: '',
+};
diff --git a/packages/clerk-js/src/core/tokenCache.ts b/packages/clerk-js/src/core/tokenCache.ts
@@ -80,6 +80,10 @@ const MemoryTokenCache = (prefix = KEY_PREFIX): TokenCache => {
 
     entry.tokenResolver
       .then(newToken => {
+        if (!newToken.jwt) {
+          return deleteKey();
+        }
+
         const expiresAt = newToken.jwt.claims.exp;
         const issuedAt = newToken.jwt.claims.iat;
         const expiresIn: Seconds = expiresAt - issuedAt;

diff --git a/packages/types/src/token.ts b/packages/types/src/token.ts
@@ -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;
 }