diff --git a/.changeset/three-ads-fold.md b/.changeset/three-ads-fold.md new file mode 100644 index 00000000000..9161e7e7d66 --- /dev/null +++ b/.changeset/three-ads-fold.md @@ -0,0 +1,5 @@ +--- +'@clerk/clerk-js': patch +--- + +Fix random sign-outs when the browser temporarily loses network connectivity. diff --git a/packages/clerk-js/bundlewatch.config.json b/packages/clerk-js/bundlewatch.config.json index 8cbc8199d3a..a268b1c02dd 100644 --- a/packages/clerk-js/bundlewatch.config.json +++ b/packages/clerk-js/bundlewatch.config.json @@ -1,6 +1,6 @@ { "files": [ - { "path": "./dist/clerk.js", "maxSize": "539KB" }, + { "path": "./dist/clerk.js", "maxSize": "540KB" }, { "path": "./dist/clerk.browser.js", "maxSize": "66KB" }, { "path": "./dist/clerk.legacy.browser.js", "maxSize": "108KB" }, { "path": "./dist/clerk.no-rhc.js", "maxSize": "307KB" }, diff --git a/packages/clerk-js/src/core/resources/Session.ts b/packages/clerk-js/src/core/resources/Session.ts index f839def8c18..684265fc3c2 100644 --- a/packages/clerk-js/src/core/resources/Session.ts +++ b/packages/clerk-js/src/core/resources/Session.ts @@ -1,6 +1,12 @@ import { createCheckAuthorization } from '@clerk/shared/authorization'; -import { isValidBrowserOnline } from '@clerk/shared/browser'; -import { ClerkOfflineError, ClerkWebAuthnError, is4xxError, MissingExpiredTokenError } from '@clerk/shared/error'; +import { isBrowserOnline, isValidBrowserOnline } from '@clerk/shared/browser'; +import { + ClerkOfflineError, + ClerkRuntimeError, + ClerkWebAuthnError, + is4xxError, + MissingExpiredTokenError, +} from '@clerk/shared/error'; import { convertJSONToPublicKeyRequestOptions, serializePublicKeyCredentialAssertion, @@ -438,18 +444,32 @@ export class Session extends BaseResource implements SessionResource { // Dispatch tokenUpdate only for __session tokens with the session's active organization ID, and not JWT templates const shouldDispatchTokenUpdate = !template && organizationId === this.lastActiveOrganizationId; + let result: string | null; + if (cacheResult) { // Proactive refresh is handled by timers scheduled in the cache // Prefer synchronous read to avoid microtask overhead when token is already resolved const cachedToken = cacheResult.entry.resolvedToken ?? (await cacheResult.entry.tokenResolver); - if (shouldDispatchTokenUpdate) { + // Only emit token updates when we have an actual token — emitting with an empty + // token causes AuthCookieService to remove the __session cookie (looks like sign-out). + if (shouldDispatchTokenUpdate && cachedToken.getRawString()) { eventBus.emit(events.TokenUpdate, { token: cachedToken }); } - // Return null when raw string is empty to indicate signed-out state - return cachedToken.getRawString() || null; + result = cachedToken.getRawString() || null; + } else { + if (!isBrowserOnline()) { + throw new ClerkRuntimeError('Browser is offline, skipping token fetch', { code: 'network_error' }); + } + result = await this.#fetchToken(template, organizationId, tokenId, shouldDispatchTokenUpdate, skipCache); + } + + // Throw when offline and no token so retry() in getToken() can fire. + // Without this, _getToken returns null (success) and retry() never calls shouldRetry. + if (result === null && !isValidBrowserOnline()) { + throw new ClerkRuntimeError('Network request failed while offline', { code: 'network_error' }); } - return this.#fetchToken(template, organizationId, tokenId, shouldDispatchTokenUpdate, skipCache); + return result; } #createTokenResolver( @@ -477,6 +497,12 @@ export class Session extends BaseResource implements SessionResource { return; } + // Never dispatch empty tokens — this would cause AuthCookieService to remove + // the __session cookie even though the user is still authenticated. + if (!token.getRawString()) { + return; + } + eventBus.emit(events.TokenUpdate, { token }); if (token.jwt) { @@ -502,9 +528,15 @@ export class Session extends BaseResource implements SessionResource { }); return tokenResolver.then(token => { + const rawString = token.getRawString(); + if (!rawString) { + // Token fetch returned an empty response — this happens when _baseFetch returns null + // due to a network error while offline. Throw so retry logic in getToken() can handle it, + // rather than silently returning null (which callers interpret as "signed out"). + throw new ClerkRuntimeError('Token fetch returned empty response', { code: 'network_error' }); + } this.#dispatchTokenEvents(token, shouldDispatchTokenUpdate); - // Return null when raw string is empty to indicate signed-out state - return token.getRawString() || null; + return rawString; }); } @@ -534,6 +566,12 @@ export class Session extends BaseResource implements SessionResource { // This allows concurrent calls to continue using the stale token tokenResolver .then(token => { + // Never cache or dispatch empty tokens — preserve the stale-but-valid + // token in cache instead of replacing it with an empty one. + if (!token.getRawString()) { + return; + } + // Cache the resolved token for future calls // Re-register onRefresh to handle the next refresh cycle when this token approaches expiration SessionTokenCache.set({ diff --git a/packages/clerk-js/src/core/resources/__tests__/Session.test.ts b/packages/clerk-js/src/core/resources/__tests__/Session.test.ts index 6926672c397..4ccae5510e2 100644 --- a/packages/clerk-js/src/core/resources/__tests__/Session.test.ts +++ b/packages/clerk-js/src/core/resources/__tests__/Session.test.ts @@ -261,8 +261,6 @@ describe('Session', () => { describe('with offline browser and network failure', () => { beforeEach(() => { - // Use real timers for offline tests to avoid unhandled rejection issues with retry logic - vi.useRealTimers(); Object.defineProperty(window.navigator, 'onLine', { writable: true, value: false, @@ -274,10 +272,9 @@ describe('Session', () => { writable: true, value: true, }); - vi.useFakeTimers(); }); - it('throws ClerkOfflineError when offline', async () => { + it('throws ClerkOfflineError after retries when offline', async () => { const session = new Session({ status: 'active', id: 'session_1', @@ -292,15 +289,15 @@ describe('Session', () => { mockNetworkFailedFetch(); BaseResource.clerk = { getFapiClient: () => createFapiClient(baseFapiClientOptions) } as any; - try { - await session.getToken({ skipCache: true }); - expect.fail('Expected ClerkOfflineError to be thrown'); - } catch (error) { - expect(ClerkOfflineError.is(error)).toBe(true); - } + const errorPromise = session.getToken({ skipCache: true }).catch(e => e); + + await vi.advanceTimersByTimeAsync(60_000); + + const error = await errorPromise; + expect(ClerkOfflineError.is(error)).toBe(true); }); - it('throws ClerkOfflineError after fetch fails while offline', async () => { + it('retries 3 times before throwing when offline without making network requests', async () => { const session = new Session({ status: 'active', id: 'session_1', @@ -315,11 +312,114 @@ describe('Session', () => { mockNetworkFailedFetch(); BaseResource.clerk = { getFapiClient: () => createFapiClient(baseFapiClientOptions) } as any; - await expect(session.getToken({ skipCache: true })).rejects.toThrow(ClerkOfflineError); + const getTokenSpy = vi.spyOn(session as any, '_getToken'); + + const errorPromise = session.getToken({ skipCache: true }).catch(e => e); + + await vi.advanceTimersByTimeAsync(60_000); - // Fetch should have been called at least once - expect(global.fetch).toHaveBeenCalled(); + await errorPromise; + + expect(getTokenSpy).toHaveBeenCalledTimes(4); + expect(global.fetch).toHaveBeenCalledTimes(0); }); + + it('does not emit token:update with an empty token when offline', async () => { + const session = new Session({ + status: 'active', + id: 'session_1', + object: 'session', + user: createUser({}), + last_active_organization_id: null, + actor: null, + created_at: new Date().getTime(), + updated_at: new Date().getTime(), + } as SessionJSON); + + mockNetworkFailedFetch(); + BaseResource.clerk = { getFapiClient: () => createFapiClient(baseFapiClientOptions) } as any; + + const errorPromise = session.getToken({ skipCache: true }).catch(e => e); + await vi.advanceTimersByTimeAsync(60_000); + await errorPromise; + + const emptyTokenUpdates = dispatchSpy.mock.calls.filter( + (call: unknown[]) => + call[0] === 'token:update' && !(call[1] as { token: { getRawString(): string } })?.token?.getRawString(), + ); + expect(emptyTokenUpdates).toHaveLength(0); + }); + + it('throws error instead of returning null when browser recovers mid-request', async () => { + // Simulate the race condition: + // 1. _baseFetch catches a network error while offline → returns null + // 2. Browser comes back online before _getToken checks isValidBrowserOnline() + // 3. _getToken sees result=null but browser is online → skips the throw → returns null + // The caller gets null which looks like "signed out" even though user is authenticated. + const session = new Session({ + status: 'active', + id: 'session_1', + object: 'session', + user: createUser({}), + last_active_organization_id: null, + actor: null, + created_at: new Date().getTime(), + updated_at: new Date().getTime(), + } as SessionJSON); + + // Browser was offline (set by parent describe's beforeEach) but has now recovered. + Object.defineProperty(window.navigator, 'onLine', { writable: true, value: true }); + + // Mock _fetch to return null, simulating what _baseFetch does when the offline + // branch fires. The browser was offline when the catch + // ran, but has since recovered by the time _getToken checks. + const fetchSpy = vi.spyOn(BaseResource, '_fetch' as any).mockResolvedValue(null); + + try { + const promise = session.getToken(); + // Suppress unhandled rejection from intermediate retry promises during timer advancement. + // The assertion below still checks the original rejected promise. + promise.catch(() => {}); + // Advance timers to allow all retries to complete + await vi.advanceTimersByTimeAsync(200_000); + // Should throw — not silently return null + await expect(promise).rejects.toThrow(); + } finally { + fetchSpy.mockRestore(); + } + }); + }); + + it('does not emit token:update with an empty token even when online', async () => { + const session = new Session({ + status: 'active', + id: 'session_1', + object: 'session', + user: createUser({}), + last_active_organization_id: null, + actor: null, + created_at: new Date().getTime(), + updated_at: new Date().getTime(), + } as SessionJSON); + + BaseResource.clerk = { getFapiClient: () => createFapiClient(baseFapiClientOptions) } as any; + + const fetchSpy = vi.spyOn(BaseResource, '_fetch' as any).mockResolvedValue(null); + + try { + const promise = session.getToken(); + promise.catch(() => {}); + await vi.advanceTimersByTimeAsync(200_000); + await expect(promise).rejects.toThrow(); + + const emptyTokenUpdates = dispatchSpy.mock.calls.filter( + (call: unknown[]) => + call[0] === 'token:update' && !(call[1] as { token: { getRawString(): string } })?.token?.getRawString(), + ); + expect(emptyTokenUpdates).toHaveLength(0); + } finally { + fetchSpy.mockRestore(); + } }); it(`uses the current session's lastActiveOrganizationId by default, not clerk.organization.id`, async () => { @@ -588,6 +688,48 @@ describe('Session', () => { expect(requestSpy).not.toHaveBeenCalled(); }); + it('does not emit token:update with an empty token when background refresh fires while offline', async () => { + BaseResource.clerk = clerkMock(); + const requestSpy = BaseResource.clerk.getFapiClient().request as Mock; + + const session = new Session({ + status: 'active', + id: 'session_1', + object: 'session', + user: createUser({}), + last_active_organization_id: null, + last_active_token: { object: 'token', jwt: mockJwt }, + actor: null, + created_at: new Date().getTime(), + updated_at: new Date().getTime(), + } as SessionJSON); + + await Promise.resolve(); + requestSpy.mockClear(); + dispatchSpy.mockClear(); + + // Go offline before the refresh timer fires + Object.defineProperty(window.navigator, 'onLine', { writable: true, value: false }); + mockNetworkFailedFetch(); + BaseResource.clerk = { getFapiClient: () => createFapiClient(baseFapiClientOptions) } as any; + + // Advance to trigger the refresh timer (~43s) and let the refresh complete + await vi.advanceTimersByTimeAsync(44 * 1000); + + const emptyTokenUpdates = dispatchSpy.mock.calls.filter( + (call: unknown[]) => + call[0] === 'token:update' && !(call[1] as { token: { getRawString(): string } })?.token?.getRawString(), + ); + expect(emptyTokenUpdates).toHaveLength(0); + + // Come back online and restore mock + Object.defineProperty(window.navigator, 'onLine', { writable: true, value: true }); + BaseResource.clerk = clerkMock(); + + const token = await session.getToken(); + expect(token).toEqual(mockJwt); + }); + it('does not make API call when token has plenty of time remaining', async () => { BaseResource.clerk = clerkMock(); const requestSpy = BaseResource.clerk.getFapiClient().request as Mock; diff --git a/packages/shared/src/__tests__/browser.spec.ts b/packages/shared/src/__tests__/browser.spec.ts index d370f886d4e..4cb73c6989a 100644 --- a/packages/shared/src/__tests__/browser.spec.ts +++ b/packages/shared/src/__tests__/browser.spec.ts @@ -162,7 +162,7 @@ describe('isValidBrowserOnline', () => { expect(isValidBrowserOnline()).toBe(false); }); - it('returns FALSE if connection is NOT online, navigator is online, has disabled the webdriver flag, and is not a bot', () => { + it('returns TRUE if connection reports zero values but navigator is online (headless browser)', () => { userAgentGetter.mockReturnValue( 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:109.0) Gecko/20100101 Firefox/109.0', ); @@ -170,7 +170,7 @@ describe('isValidBrowserOnline', () => { onLineGetter.mockReturnValue(true); connectionGetter.mockReturnValue({ downlink: 0, rtt: 0 }); - expect(isValidBrowserOnline()).toBe(false); + expect(isValidBrowserOnline()).toBe(true); }); it('returns FALSE if connection is online, navigator is NOT online, has disabled the webdriver flag, and is not a bot', () => { diff --git a/packages/shared/src/browser.ts b/packages/shared/src/browser.ts index 8b27e783ed4..ea61931f6d3 100644 --- a/packages/shared/src/browser.ts +++ b/packages/shared/src/browser.ts @@ -73,13 +73,12 @@ export function isBrowserOnline(): boolean { return false; } - const isNavigatorOnline = navigator?.onLine; - - // Being extra safe with the experimental `connection` property, as it is not defined in all browsers - // https://developer.mozilla.org/en-US/docs/Web/API/Navigator/connection#browser_compatibility - // @ts-ignore - const isExperimentalConnectionOnline = navigator?.connection?.rtt !== 0 && navigator?.connection?.downlink !== 0; - return isExperimentalConnectionOnline && isNavigatorOnline; + // navigator.onLine is the standard API and is reliable for detecting + // complete disconnection (airplane mode, WiFi off, etc.). + // The experimental navigator.connection API (rtt/downlink) was previously + // used as a secondary signal, but it reports zero values in headless browsers + // and CI environments even when connected, causing false offline detection. + return !!navigator.onLine; } /**