Skip to content

Commit 39a684d

Browse files
committed
refactor(clerk-js): streamline degraded load recovery
Why: The degraded-load recovery carried a client-generation counter to discard a background /client response that raced a newer client update. It added an instance field, an increment in the hot updateClient path, and a low-level fetch that reached past the resource API. Dropped it: the background retry now applies the /client response directly, and the rare stale overwrite it guarded against is self-correcting via the poller. Also collapsed the mint fallback so recovery derives one client from a single jwt (minted, or the cookie jwt on mint failure) rather than branching on a separate localClient.
1 parent e0de525 commit 39a684d

2 files changed

Lines changed: 14 additions & 72 deletions

File tree

packages/clerk-js/src/core/__tests__/clerk.test.ts

Lines changed: 4 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,6 @@ import type { DisplayConfig, Organization } from '../resources/internal';
2222
import { BaseResource, Client, Environment, SignIn, SignUp } from '../resources/internal';
2323

2424
const mockClientFetch = vi.fn();
25-
// Static raw-JSON fetch used by the background /client retry after a degraded load.
26-
const mockClientStaticFetch = vi.fn();
2725
const mockEnvironmentFetch = vi.fn(() => Promise.resolve({}));
2826

2927
vi.mock('../resources/Client');
@@ -44,9 +42,8 @@ vi.mock('../auth/devBrowser', () => ({
4442
}));
4543

4644
Client.getOrCreateInstance = vi.fn().mockImplementation(() => {
47-
return { fetch: mockClientFetch, fromJSON: (data: any) => data };
45+
return { fetch: mockClientFetch };
4846
});
49-
Client._fetch = mockClientStaticFetch as any;
5047
Environment.getInstance = vi.fn().mockImplementation(() => {
5148
return { fetch: mockEnvironmentFetch };
5249
});
@@ -870,10 +867,6 @@ describe('Clerk singleton', () => {
870867
.spyOn(AuthCookieService.prototype, 'stopPollingForToken')
871868
.mockImplementation(() => void callLog.push('stopPoll'));
872869
mockClientFetch.mockClear();
873-
// Client._fetch must return a promise; default to a no-op null so degraded paths that don't
874-
// assert on the background retry don't hit undefined.then. The two background tests override this.
875-
mockClientStaticFetch.mockReset();
876-
mockClientStaticFetch.mockResolvedValue(null);
877870
mockCreateClientFromJwt.mockReturnValue({ signedInSessions: [], lastActiveSessionId: null });
878871
});
879872

@@ -979,48 +972,23 @@ describe('Clerk singleton', () => {
979972
const realSession = { id: 'sess_1', status: 'active', user: { id: 'user_real' } };
980973
const realClient = { id: 'client_real', signedInSessions: [realSession], lastActiveSessionId: 'sess_1' };
981974
const refetch = createDeferredPromise();
982-
mockClientFetch.mockRejectedValueOnce(new Error('client fetch failed'));
983-
mockClientStaticFetch.mockReturnValueOnce(refetch.promise);
975+
mockClientFetch.mockRejectedValueOnce(new Error('client fetch failed')).mockReturnValueOnce(refetch.promise);
984976
mockCreateClientFromJwt.mockReturnValue(sessionClient(stubSession));
985977

986978
const sut = new Clerk(productionPublishableKey);
987979
await pumpUntilSettled(sut.load());
988980

989981
expect(sut.status).toBe('degraded');
990-
// The primary /client fetch is Client.fetch(); the background retry is the raw Client._fetch().
991-
expect(mockClientFetch).toHaveBeenCalledTimes(1);
992-
expect(mockClientStaticFetch).toHaveBeenCalledTimes(1);
982+
expect(mockClientFetch).toHaveBeenCalledTimes(2);
993983

994984
// The background retry is not bounded by INITIALIZATION_TIMEOUT_MS.
995985
await vi.advanceTimersByTimeAsync(60_000);
996-
refetch.resolve({ response: realClient });
986+
refetch.resolve(realClient);
997987
await vi.advanceTimersByTimeAsync(0);
998988

999989
expect(sut.client).toBe(realClient);
1000990
expect(sut.session).toBe(realSession);
1001991
});
1002-
1003-
it('discards the background /client response when something else updated the client while it was in flight', async () => {
1004-
const stubSession = makeSession();
1005-
const refetch = createDeferredPromise();
1006-
mockClientFetch.mockRejectedValueOnce(new Error('client fetch failed'));
1007-
mockClientStaticFetch.mockReturnValueOnce(refetch.promise);
1008-
mockCreateClientFromJwt.mockReturnValue(sessionClient(stubSession));
1009-
1010-
const sut = new Clerk(productionPublishableKey);
1011-
await sut.load();
1012-
1013-
// Something newer lands while the background retry is still in flight, e.g. a sign-out
1014-
// or a mutation's piggybacked client.
1015-
const interimClient = { id: 'client_interim', signedInSessions: [], lastActiveSessionId: null };
1016-
sut.updateClient(interimClient as any);
1017-
1018-
const staleClient = { id: 'client_stale', signedInSessions: [stubSession], lastActiveSessionId: 'sess_1' };
1019-
refetch.resolve({ response: staleClient });
1020-
await new Promise(resolve => setTimeout(resolve, 0));
1021-
1022-
expect(sut.client).toBe(interimClient);
1023-
});
1024992
});
1025993
});
1026994

packages/clerk-js/src/core/clerk.ts

Lines changed: 10 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,6 @@ import type {
7777
ClerkAPIError,
7878
ClerkAuthenticateWithWeb3Params,
7979
ClerkOptions,
80-
ClientJSON,
8180
ClientJSONSnapshot,
8281
ClientResource,
8382
ConfigureSSOProps,
@@ -216,8 +215,6 @@ const CANNOT_RENDER_API_KEYS_ORG_DISABLED_ERROR_CODE = 'cannot_render_api_keys_o
216215
const CANNOT_RENDER_SELF_SERVE_SSO_DISABLED_ERROR_CODE = 'cannot_render_self_serve_sso_disabled';
217216
const CANNOT_RENDER_CONFIGURE_SSO_EMAIL_ADDRESS_DISABLED_ERROR_CODE =
218217
'cannot_render_configure_sso_email_address_disabled';
219-
// Bounds a single origin request at load; a slow response gets no fapiClient retry,
220-
// so this needs to sit above real-world /client latency including cold-mobile DNS+TLS.
221218
const INITIALIZATION_TIMEOUT_MS = 7_000;
222219
const defaultOptions: ClerkOptions = {
223220
polling: true,
@@ -271,7 +268,6 @@ export class Clerk implements ClerkInterface {
271268
#fapiClient: FapiClient;
272269
#instanceType?: InstanceType;
273270
#status: ClerkInterface['status'] = 'loading';
274-
#clientUpdateGeneration = 0;
275271
#listeners: Array<(emission: Resources) => void> = [];
276272
#navigationListeners: Array<() => void> = [];
277273
#options: ClerkOptions = {};
@@ -2981,7 +2977,6 @@ export class Clerk implements ClerkInterface {
29812977
// and emitting, library consumers that both read state directly and set up listeners
29822978
// could end up in a inconsistent state.
29832979
updateClient = (newClient: ClientResource, options?: { __internal_dangerouslySkipEmit?: boolean }): void => {
2984-
this.#clientUpdateGeneration++;
29852980
if (!this.client) {
29862981
// This is the first time client is being
29872982
// set, so we also need to set session
@@ -3251,8 +3246,7 @@ export class Clerk implements ClerkInterface {
32513246
});
32523247

32533248
const initClient = async () => {
3254-
// Abort the /client request on timeout so it stops running instead of settling on a
3255-
// detached instance later; the background retry below owns recovery from then on.
3249+
// Abort the /client request on timeout so clerkjs loading does not hang
32563250
const clientFetchController = new AbortController();
32573251
return timeLimit(
32583252
Client.getOrCreateInstance().fetch({ abortSignal: clientFetchController.signal }),
@@ -3279,46 +3273,26 @@ export class Clerk implements ClerkInterface {
32793273

32803274
try {
32813275
const jwtInCookie = this.#authService?.getSessionCookie();
3282-
const localClient = createClientFromJwt(jwtInCookie);
3283-
const session = this.#defaultSession(localClient);
3276+
const session = this.#defaultSession(createClientFromJwt(jwtInCookie));
32843277

32853278
if (session) {
3286-
// Prefer minting a fresh token for the degraded identity: the minter can serve it
3287-
// during an origin outage and its claims are fresher than the cookie's. Fall back
3288-
// to the cookie identity only when the mint fails or times out.
3279+
// Prefer minting a fresh token as its claims are fresher than the cookie's
32893280
session.clearCache();
3290-
const freshJwt = await timeLimit(session.getToken(), INITIALIZATION_TIMEOUT_MS).catch(() => {
3291-
// On timeout the recovery getToken is still in flight with a pending resolver in the cache;
3292-
// clear it so the poller's next getToken starts fresh instead of awaiting the abandoned one.
3281+
const jwt = await timeLimit(session.getToken(), INITIALIZATION_TIMEOUT_MS).catch(() => {
32933282
session.clearCache();
3294-
return null;
3283+
return jwtInCookie;
32953284
});
3296-
this.updateClient(freshJwt ? createClientFromJwt(freshJwt) : localClient);
3285+
this.updateClient(createClientFromJwt(jwt));
32973286
} else {
3298-
this.updateClient(localClient);
3287+
this.updateClient(createClientFromJwt(jwtInCookie));
32993288
}
33003289
} finally {
3301-
// Always restart the poller, no matter what in the recovery block above threw (a failed
3302-
// mint, a throwing updateClient listener). Otherwise the session-refresh poller would stay
3303-
// stopped until a full page reload.
33043290
this.#authService?.startPollingForToken();
33053291
}
33063292

3307-
// Retry /client in the background (network-level retries, no time limit) now that load no
3308-
// longer blocks on it. We fetch the raw JSON and apply it only if nothing else updated the
3309-
// client while it was in flight; anything newer (a sign-out, a mutation's piggybacked client)
3310-
// must win. Fetching the JSON with Client._fetch instead of Client.fetch() means a superseded
3311-
// response is dropped without ever mutating the shared Client instance in place.
3312-
// A 5xx failure is not retried (fapiClient only retries network errors); in that case the
3313-
// client heals via the piggybacked client of the next mutation.
3314-
const clientGenerationAtDispatch = this.#clientUpdateGeneration;
3315-
void Client._fetch<ClientJSON>({ method: 'GET', path: '/client' })
3316-
.then(res => {
3317-
const clientJson = res?.response;
3318-
if (clientJson && this.#clientUpdateGeneration === clientGenerationAtDispatch) {
3319-
this.updateClient(Client.getOrCreateInstance().fromJSON(clientJson));
3320-
}
3321-
})
3293+
void Client.getOrCreateInstance()
3294+
.fetch()
3295+
.then(res => this.updateClient(res))
33223296
.catch(noop);
33233297

33243298
return null;

0 commit comments

Comments
 (0)