Skip to content

Commit 1253b2a

Browse files
wobsorianowobsoriano
committed
refactor(expo): drop request abort handling and trim token read tests
The only request clerk-js aborts is the initial client fetch on load timeout. Its hook already reads a valid token once the app is active, and fetch rejects the aborted request before it is sent, so threading the signal through the activation wait is unnecessary. Share setup across the token read tests, collapse the token cache app state tables into one, and tighten the changeset wording.
1 parent dac76d1 commit 1253b2a

6 files changed

Lines changed: 59 additions & 301 deletions

File tree

‎.changeset/keychain-read-keeps-client.md‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
'@clerk/expo': minor
33
---
44

5-
Fix iOS users being signed out when the keychain can't be read for a moment, for example when the app is launched in the background before the device has been unlocked since a restart.
5+
Fix iOS users being signed out when the keychain can't be read for a moment, for example when the app launches in the background before the device has been unlocked since a restart.
66

7-
- On iOS, `getToken()` from the `@clerk/expo/token-cache` `tokenCache` now rejects with the keychain error unless the read both starts and fails while the app is active, instead of deleting the stored client token and returning `null`. This also preserves tokens in app extensions. A read that both starts and fails while the app is active still clears the token, as before.
8-
- When reading the client token fails, from this `tokenCache` or a custom one, Clerk now waits until the app is active and reads it again before sending the request. If this happens while the app is in the background, those requests wait until the app is opened. If a custom `tokenCache` still fails, the request fails with its error, as before.
9-
- A custom `tokenCache` that deletes the token and returns `null` whenever a read fails will still sign users out in this situation. To avoid that, rethrow the error unless `AppState.currentState` is `active` both when the read starts and when it fails. A custom `tokenCache` that always rethrows makes Clerk requests fail until the token can be read again.
7+
- On iOS, `getToken()` from the `tokenCache` in `@clerk/expo/token-cache` no longer deletes the stored client token when the keychain read fails. It rejects with the keychain error instead, which also keeps tokens in app extensions. The one exception is a read that starts and fails while the app is active. That read still deletes the token and returns `null`, as before.
8+
- When reading the client token fails, from this `tokenCache` or a custom one, Clerk waits until the app is active and reads it once more before sending the request. Requests made in the background wait until the app is opened. If the second read also fails, the request fails with that error.
9+
- A custom `tokenCache` that deletes the token and returns `null` whenever a read fails still signs users out in this case. To avoid that, rethrow the error unless `AppState.currentState` is `active` both when the read starts and when it fails. A custom `tokenCache` that always rethrows makes Clerk requests fail until the token can be read again.

‎packages/expo/src/provider/__tests__/ClerkProvider.nativeClientSync.test.tsx‎

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -557,8 +557,7 @@ describe('ClerkProvider native client sync', () => {
557557
consoleError.mockRestore();
558558
});
559559

560-
test('finishes bootstrap with the cached token once the app is active when the token cache cannot be read in the background', async () => {
561-
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined);
560+
test('configures native with the cached token once a background launch becomes active', async () => {
562561
setAppState('background');
563562
mocks.tokenCache.getToken.mockImplementation(rejectUnlessAppIsActive('client-token'));
564563
mocks.getClientToken.mockResolvedValue('client-token');
@@ -599,11 +598,9 @@ describe('ClerkProvider native client sync', () => {
599598
await waitFor(() =>
600599
expect(mocks.tokenCache.saveToken).toHaveBeenCalledWith(CLERK_CLIENT_JWT_KEY, 'native-client-token'),
601600
);
602-
consoleError.mockRestore();
603601
});
604602

605-
test('applies a native client change once the app is active when the token cache cannot be read in the background', async () => {
606-
const consoleError = vi.spyOn(console, 'error').mockImplementation(() => undefined);
603+
test('applies a native client change from the background once the app becomes active', async () => {
607604
mocks.tokenCache.getToken.mockImplementation(rejectUnlessAppIsActive('client-token'));
608605
mocks.getClientToken.mockResolvedValue('client-token');
609606

@@ -615,9 +612,6 @@ describe('ClerkProvider native client sync', () => {
615612
);
616613

617614
await waitFor(() => expect(mocks.configure).toHaveBeenCalledWith('pk_test_123', 'client-token'));
618-
await act(async () => {
619-
await Promise.resolve();
620-
});
621615

622616
setAppState('background');
623617
mocks.nativeClientEvent = {
@@ -645,7 +639,6 @@ describe('ClerkProvider native client sync', () => {
645639
expect(mocks.tokenCache.saveToken).toHaveBeenCalledWith(CLERK_CLIENT_JWT_KEY, 'native-client-token'),
646640
);
647641
expect(mocks.clerkInstance.__internal_reloadInitialResources).toHaveBeenCalled();
648-
consoleError.mockRestore();
649642
});
650643

651644
test('disables synchronization when switching publishable keys fails to configure native', async () => {

‎packages/expo/src/provider/singleton/__tests__/createClerkInstance.test.ts‎

Lines changed: 28 additions & 213 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,24 @@ const loadCreateClerkInstance = async () => {
9191
return mod.createClerkInstance;
9292
};
9393

94+
const setUpBeforeRequest = async (tokenCache: TokenCache) => {
95+
const createClerkInstance = await loadCreateClerkInstance();
96+
const getClerkInstance = createClerkInstance(MockClerk as unknown as typeof Clerk);
97+
const clerk = getClerkInstance({
98+
publishableKey: 'pk_test_123',
99+
tokenCache,
100+
}) as unknown as MockClerk;
101+
102+
return {
103+
beforeRequest: clerk.__internal_onBeforeRequest.mock.calls[0][0],
104+
getClerkInstance,
105+
requestInit: {
106+
headers: new Headers(),
107+
url: new URL('https://clerk.example.com/v1/client'),
108+
},
109+
};
110+
};
111+
94112
describe('createClerkInstance', () => {
95113
beforeEach(() => {
96114
vi.resetModules();
@@ -374,23 +392,7 @@ describe('createClerkInstance', () => {
374392
.fn<TokenCache['getToken']>()
375393
.mockRejectedValueOnce(new Error('User interaction is not allowed.'))
376394
.mockResolvedValueOnce('cached-token');
377-
const tokenCache: TokenCache = {
378-
getToken,
379-
saveToken: vi.fn(() => Promise.resolve()),
380-
};
381-
382-
const createClerkInstance = await loadCreateClerkInstance();
383-
const getClerkInstance = createClerkInstance(MockClerk as unknown as typeof Clerk);
384-
const clerk = getClerkInstance({
385-
publishableKey: 'pk_test_123',
386-
tokenCache,
387-
}) as unknown as MockClerk;
388-
389-
const beforeRequest = clerk.__internal_onBeforeRequest.mock.calls[0][0];
390-
const requestInit = {
391-
headers: new Headers(),
392-
url: new URL('https://clerk.example.com/v1/client'),
393-
};
395+
const { beforeRequest, requestInit } = await setUpBeforeRequest({ getToken, saveToken: vi.fn() });
394396

395397
let settled = false;
396398
const pending = beforeRequest(requestInit).finally(() => {
@@ -415,245 +417,58 @@ describe('createClerkInstance', () => {
415417
.fn<TokenCache['getToken']>()
416418
.mockRejectedValueOnce(new Error('Keychain is busy.'))
417419
.mockResolvedValueOnce('cached-token');
418-
const clearToken = vi.fn(() => Promise.resolve());
419-
const tokenCache: TokenCache = {
420-
getToken,
421-
saveToken: vi.fn(() => Promise.resolve()),
422-
clearToken,
423-
};
424-
425-
const createClerkInstance = await loadCreateClerkInstance();
426-
const getClerkInstance = createClerkInstance(MockClerk as unknown as typeof Clerk);
427-
const clerk = getClerkInstance({
428-
publishableKey: 'pk_test_123',
429-
tokenCache,
430-
}) as unknown as MockClerk;
420+
const { beforeRequest, requestInit } = await setUpBeforeRequest({ getToken, saveToken: vi.fn() });
431421

432-
const beforeRequest = clerk.__internal_onBeforeRequest.mock.calls[0][0];
433-
const requestInit = {
434-
headers: new Headers(),
435-
url: new URL('https://clerk.example.com/v1/client'),
436-
};
437422
await beforeRequest(requestInit);
438423

439424
expect(getToken).toHaveBeenCalledTimes(2);
440-
expect(clearToken).not.toHaveBeenCalled();
441425
expect(requestInit.headers.get('authorization')).toBe('cached-token');
442426
expect(mocks.appStateListeners.size).toBe(0);
443427
});
444428

445-
test('rejects with the token cache error without clearing it when the read still fails once the app is active', async () => {
429+
test('rejects with the token cache error when the read still fails once the app is active', async () => {
446430
setAppState('background');
447431
const readError = new Error('Biometric authentication was cancelled.');
448432
const getToken = vi.fn<TokenCache['getToken']>(() => Promise.reject(readError));
449-
const clearToken = vi.fn(() => Promise.resolve());
450-
const tokenCache: TokenCache = {
451-
getToken,
452-
saveToken: vi.fn(() => Promise.resolve()),
453-
clearToken,
454-
};
455-
456-
const createClerkInstance = await loadCreateClerkInstance();
457-
const getClerkInstance = createClerkInstance(MockClerk as unknown as typeof Clerk);
458-
const clerk = getClerkInstance({
459-
publishableKey: 'pk_test_123',
460-
tokenCache,
461-
}) as unknown as MockClerk;
462-
463-
const beforeRequest = clerk.__internal_onBeforeRequest.mock.calls[0][0];
464-
const requestInit = {
465-
headers: new Headers(),
466-
url: new URL('https://clerk.example.com/v1/client'),
467-
};
433+
const { beforeRequest, requestInit } = await setUpBeforeRequest({ getToken, saveToken: vi.fn() });
468434

469435
const pending = beforeRequest(requestInit).catch((err: unknown) => err);
470436
await vi.waitFor(() => expect(mocks.appStateListeners.size).toBe(1));
471-
472437
setAppState('active');
473438

474439
await expect(pending).resolves.toBe(readError);
475440
expect(getToken).toHaveBeenCalledTimes(2);
476-
expect(clearToken).not.toHaveBeenCalled();
477441
expect(requestInit.headers.has('authorization')).toBe(false);
478442
});
479443

480-
test('stops waiting for the app to become active when the request is aborted', async () => {
481-
setAppState('background');
482-
const getToken = vi.fn<TokenCache['getToken']>(() => Promise.reject(new Error('User interaction is not allowed.')));
483-
const tokenCache: TokenCache = {
484-
getToken,
485-
saveToken: vi.fn(() => Promise.resolve()),
486-
};
487-
488-
const createClerkInstance = await loadCreateClerkInstance();
489-
const getClerkInstance = createClerkInstance(MockClerk as unknown as typeof Clerk);
490-
const clerk = getClerkInstance({
491-
publishableKey: 'pk_test_123',
492-
tokenCache,
493-
}) as unknown as MockClerk;
494-
495-
const beforeRequest = clerk.__internal_onBeforeRequest.mock.calls[0][0];
496-
const controller = new AbortController();
497-
const abortReason = new Error('client fetch timed out');
498-
const requestInit = {
499-
headers: new Headers(),
500-
signal: controller.signal,
501-
url: new URL('https://clerk.example.com/v1/client'),
502-
};
503-
504-
const pending = beforeRequest(requestInit).catch((err: unknown) => err);
505-
await vi.waitFor(() => expect(mocks.appStateListeners.size).toBe(1));
506-
507-
controller.abort(abortReason);
508-
509-
await expect(pending).resolves.toBe(abortReason);
510-
expect(mocks.appStateListeners.size).toBe(0);
511-
512-
setAppState('active');
513-
await Promise.resolve();
514-
expect(getToken).toHaveBeenCalledTimes(1);
515-
});
516-
517-
test('does not wait for the app to become active when the request was already aborted', async () => {
518-
setAppState('background');
519-
const getToken = vi.fn<TokenCache['getToken']>(() => Promise.reject(new Error('User interaction is not allowed.')));
520-
const tokenCache: TokenCache = {
521-
getToken,
522-
saveToken: vi.fn(() => Promise.resolve()),
523-
};
524-
525-
const createClerkInstance = await loadCreateClerkInstance();
526-
const getClerkInstance = createClerkInstance(MockClerk as unknown as typeof Clerk);
527-
const clerk = getClerkInstance({
528-
publishableKey: 'pk_test_123',
529-
tokenCache,
530-
}) as unknown as MockClerk;
531-
532-
const beforeRequest = clerk.__internal_onBeforeRequest.mock.calls[0][0];
533-
const controller = new AbortController();
534-
const abortReason = new Error('client fetch timed out');
535-
controller.abort(abortReason);
536-
const requestInit = {
537-
headers: new Headers(),
538-
signal: controller.signal,
539-
url: new URL('https://clerk.example.com/v1/client'),
540-
};
541-
542-
await expect(beforeRequest(requestInit)).rejects.toBe(abortReason);
543-
expect(mocks.appStateListeners.size).toBe(0);
544-
expect(getToken).toHaveBeenCalledTimes(1);
545-
});
546-
547-
test('rejects with an AbortError when the aborted signal has no reason', async () => {
548-
setAppState('background');
549-
const tokenCache: TokenCache = {
550-
getToken: vi.fn(() => Promise.reject(new Error('User interaction is not allowed.'))),
551-
saveToken: vi.fn(() => Promise.resolve()),
552-
};
553-
554-
const createClerkInstance = await loadCreateClerkInstance();
555-
const getClerkInstance = createClerkInstance(MockClerk as unknown as typeof Clerk);
556-
const clerk = getClerkInstance({
557-
publishableKey: 'pk_test_123',
558-
tokenCache,
559-
}) as unknown as MockClerk;
560-
561-
const beforeRequest = clerk.__internal_onBeforeRequest.mock.calls[0][0];
562-
const abortListeners = new Set<() => void>();
563-
const polyfilledSignal = {
564-
aborted: false,
565-
reason: undefined,
566-
addEventListener: (_type: 'abort', listener: () => void) => abortListeners.add(listener),
567-
removeEventListener: (_type: 'abort', listener: () => void) => abortListeners.delete(listener),
568-
};
569-
const requestInit = {
570-
headers: new Headers(),
571-
signal: polyfilledSignal,
572-
url: new URL('https://clerk.example.com/v1/client'),
573-
};
574-
575-
const pending = beforeRequest(requestInit).catch((err: unknown) => err);
576-
await vi.waitFor(() => expect(abortListeners.size).toBe(1));
577-
578-
polyfilledSignal.aborted = true;
579-
for (const listener of [...abortListeners]) {
580-
listener();
581-
}
582-
583-
const error = await pending;
584-
expect(error).toBeInstanceOf(Error);
585-
expect((error as Error).name).toBe('AbortError');
586-
expect(mocks.appStateListeners.size).toBe(0);
587-
});
588-
589444
test('does not wait for the app to become active inside an app extension', async () => {
590445
setAppState('extension');
591446
const readError = new Error('Unable to decode the provided data.');
592447
const getToken = vi.fn<TokenCache['getToken']>(() => Promise.reject(readError));
593-
const clearToken = vi.fn(() => Promise.resolve());
594-
const tokenCache: TokenCache = {
595-
getToken,
596-
saveToken: vi.fn(() => Promise.resolve()),
597-
clearToken,
598-
};
599-
600-
const createClerkInstance = await loadCreateClerkInstance();
601-
const getClerkInstance = createClerkInstance(MockClerk as unknown as typeof Clerk);
602-
const clerk = getClerkInstance({
603-
publishableKey: 'pk_test_123',
604-
tokenCache,
605-
}) as unknown as MockClerk;
448+
const { beforeRequest, requestInit } = await setUpBeforeRequest({ getToken, saveToken: vi.fn() });
606449

607-
const beforeRequest = clerk.__internal_onBeforeRequest.mock.calls[0][0];
608-
const requestInit = {
609-
headers: new Headers(),
610-
url: new URL('https://clerk.example.com/v1/client'),
611-
};
612450
await expect(beforeRequest(requestInit)).rejects.toBe(readError);
613-
614451
expect(getToken).toHaveBeenCalledTimes(2);
615-
expect(clearToken).not.toHaveBeenCalled();
616452
expect(mocks.appStateListeners.size).toBe(0);
617453
});
618454

619455
test('retries with the latest tokenCache when it changes while waiting for the app to become active', async () => {
620456
setAppState('background');
621-
const initialTokenCache: TokenCache = {
622-
getToken: vi.fn(() => Promise.reject(new Error('User interaction is not allowed.'))),
623-
saveToken: vi.fn(() => Promise.resolve()),
624-
clearToken: vi.fn(() => Promise.resolve()),
625-
};
626-
const latestTokenCache: TokenCache = {
627-
getToken: vi.fn(() => Promise.resolve('latest-token')),
628-
saveToken: vi.fn(() => Promise.resolve()),
629-
clearToken: vi.fn(() => Promise.resolve()),
630-
};
631-
632-
const createClerkInstance = await loadCreateClerkInstance();
633-
const getClerkInstance = createClerkInstance(MockClerk as unknown as typeof Clerk);
634-
const clerk = getClerkInstance({
635-
publishableKey: 'pk_test_123',
636-
tokenCache: initialTokenCache,
637-
}) as unknown as MockClerk;
457+
const { beforeRequest, getClerkInstance, requestInit } = await setUpBeforeRequest({
458+
getToken: () => Promise.reject(new Error('User interaction is not allowed.')),
459+
saveToken: vi.fn(),
460+
});
638461

639-
const beforeRequest = clerk.__internal_onBeforeRequest.mock.calls[0][0];
640-
const requestInit = {
641-
headers: new Headers(),
642-
url: new URL('https://clerk.example.com/v1/client'),
643-
};
644462
const pending = beforeRequest(requestInit);
645463
await vi.waitFor(() => expect(mocks.appStateListeners.size).toBe(1));
646-
647464
getClerkInstance({
648465
publishableKey: 'pk_test_123',
649-
tokenCache: latestTokenCache,
466+
tokenCache: { getToken: () => Promise.resolve('latest-token'), saveToken: vi.fn() },
650467
});
651468
setAppState('active');
652469
await pending;
653470

654471
expect(requestInit.headers.get('authorization')).toBe('latest-token');
655-
expect(initialTokenCache.clearToken).not.toHaveBeenCalled();
656-
expect(latestTokenCache.clearToken).not.toHaveBeenCalled();
657472
});
658473

659474
test('uses the latest explicit tokenCache for request authorization when the singleton is reused', async () => {

‎packages/expo/src/provider/singleton/createClerkInstance.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,11 +62,11 @@ let __internal_cancelResourceRetries: (() => void) | undefined;
6262
// Token IO can change without recreating the native singleton.
6363
let __internal_tokenCache: TokenCache = MemoryTokenCache;
6464

65-
async function readClientJwt(signal?: AbortSignal): Promise<string | null | undefined> {
65+
async function readClientJwt(): Promise<string | null | undefined> {
6666
try {
6767
return await __internal_tokenCache.getToken(CLERK_CLIENT_JWT_KEY);
6868
} catch {
69-
await waitUntilAppIsActive(signal);
69+
await waitUntilAppIsActive();
7070
}
7171
return __internal_tokenCache.getToken(CLERK_CLIENT_JWT_KEY);
7272
}
@@ -298,7 +298,7 @@ export function createClerkInstance(ClerkClass: typeof Clerk) {
298298
// Instructs the backend to parse the api token from the Authorization header.
299299
requestInit.url?.searchParams.append('_is_native', '1');
300300

301-
const jwt = await readClientJwt(requestInit.signal ?? undefined);
301+
const jwt = await readClientJwt();
302302
(requestInit.headers as Headers).set('authorization', jwt || '');
303303

304304
// Instructs the backend that the request is from a mobile device.

0 commit comments

Comments
 (0)