From 7c3dceca2d730404605452ed898181f38329b349 Mon Sep 17 00:00:00 2001 From: "madison.packer" Date: Thu, 23 Jul 2026 20:03:27 +0000 Subject: [PATCH 1/2] fix: Preserve session on transient refresh failures authkitLoader destroyed the sealed session and redirected to sign-in on any SessionRefreshError, including transient failures (network error, request timeout, 429, or 5xx) that survived the SDK's internal retries. During a brief outage this discarded a still-valid refresh token and forced re-authentication. Classify the wrapped refresh error and only destroy the session for a terminal failure. On a transient failure keep the sealed cookie (the redirect still sets the PKCE verifier cookie) so a later request refreshes successfully once the condition clears. SessionRefreshError now exposes an isTransient flag, also surfaced on onSessionRefreshError. Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- src/interfaces.ts | 7 +++++ src/session.spec.ts | 76 +++++++++++++++++++++++++++++++++++++++++++++ src/session.ts | 65 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 148 insertions(+) diff --git a/src/interfaces.ts b/src/interfaces.ts index 58a7d6a..607aaaf 100644 --- a/src/interfaces.ts +++ b/src/interfaces.ts @@ -35,6 +35,13 @@ export interface RefreshErrorOptions { error: unknown; request: Request; sessionData: SessionData; + /** + * Whether the refresh failed for a transient reason (network error, timeout, + * 429, or 5xx) rather than a terminal one (the refresh token is dead). When + * `true`, the sealed session is preserved (not destroyed) so a later request + * can refresh successfully once the condition clears. + */ + isTransient: boolean; } export interface RefreshSuccessOptions { diff --git a/src/session.spec.ts b/src/session.spec.ts index f4a6473..d47cf92 100644 --- a/src/session.spec.ts +++ b/src/session.spec.ts @@ -835,6 +835,58 @@ describe('session', () => { } }); + it.each([ + ['a rate limit (429)', Object.assign(new Error('Too many requests'), { status: 429 })], + ['a server error (503)', Object.assign(new Error('Service unavailable'), { status: 503 })], + ['a request timeout (408)', Object.assign(new Error('Request timeout'), { status: 408 })], + ['a network error', new TypeError('fetch failed')], + // The SDK re-wraps a raw network TypeError in a plain Error with the + // TypeError as its cause; the classifier must follow the cause chain. + [ + 'an SDK-wrapped network error', + new Error('Unexpected error: TypeError: fetch failed', { cause: new TypeError('fetch failed') }), + ], + ])('should preserve the session cookie when refresh fails transiently: %s', async (_label, transientError) => { + authenticateWithRefreshToken.mockRejectedValue(transientError); + getAuthorizationUrlMock.mockResolvedValue({ + url: 'https://auth.workos.com/oauth/authorize?state=abc123', + headers: { 'Set-Cookie': 'wos-auth-verifier-abc=sealed; Path=/; HttpOnly; SameSite=Lax; Max-Age=600' }, + }); + + try { + await authkitLoader(createLoaderArgs(createMockRequest())); + fail('Expected redirect response to be thrown'); + } catch (response: unknown) { + assertIsResponse(response); + expect(response.status).toBe(302); + // The sealed session must not be destroyed on a transient failure. + expect(destroySession).not.toHaveBeenCalled(); + const setCookies = response.headers.getSetCookie(); + expect(setCookies).not.toContain('destroyed-session-cookie'); + expect(setCookies).toContain('wos-auth-verifier-abc=sealed; Path=/; HttpOnly; SameSite=Lax; Max-Age=600'); + } + }); + + it('should destroy the session for a terminal refresh failure (invalid_grant)', async () => { + authenticateWithRefreshToken.mockRejectedValue( + Object.assign(new Error('invalid_grant'), { status: 400, error: 'invalid_grant' }), + ); + getAuthorizationUrlMock.mockResolvedValue({ + url: 'https://auth.workos.com/oauth/authorize?state=abc123', + headers: { 'Set-Cookie': 'wos-auth-verifier-abc=sealed; Path=/; HttpOnly; SameSite=Lax; Max-Age=600' }, + }); + + try { + await authkitLoader(createLoaderArgs(createMockRequest())); + fail('Expected redirect response to be thrown'); + } catch (response: unknown) { + assertIsResponse(response); + expect(response.status).toBe(302); + expect(destroySession).toHaveBeenCalled(); + expect(response.headers.getSetCookie()).toContain('destroyed-session-cookie'); + } + }); + it('calls onSessionRefreshSuccess when provided', async () => { const onSessionRefreshSuccess = jest.fn(); await authkitLoader(createLoaderArgs(createMockRequest()), { @@ -855,6 +907,30 @@ describe('session', () => { expect(onSessionRefreshError).toHaveBeenCalled(); }); + it('passes isTransient: true to onSessionRefreshError for a transient failure', async () => { + authenticateWithRefreshToken.mockRejectedValue( + Object.assign(new Error('Service unavailable'), { status: 503 }), + ); + const onSessionRefreshError = jest.fn().mockReturnValue(redirect('/error')); + + await authkitLoader(createLoaderArgs(createMockRequest()), { + onSessionRefreshError, + }); + + expect(onSessionRefreshError).toHaveBeenCalledWith(expect.objectContaining({ isTransient: true })); + }); + + it('passes isTransient: false to onSessionRefreshError for a terminal failure', async () => { + authenticateWithRefreshToken.mockRejectedValue(Object.assign(new Error('invalid_grant'), { status: 400 })); + const onSessionRefreshError = jest.fn().mockReturnValue(redirect('/error')); + + await authkitLoader(createLoaderArgs(createMockRequest()), { + onSessionRefreshError, + }); + + expect(onSessionRefreshError).toHaveBeenCalledWith(expect.objectContaining({ isTransient: false })); + }); + it('allows redirect from onSessionRefreshError callback', async () => { authenticateWithRefreshToken.mockRejectedValue(new Error('Refresh token invalid')); diff --git a/src/session.ts b/src/session.ts index 819cb2d..f8b9cd2 100644 --- a/src/session.ts +++ b/src/session.ts @@ -28,12 +28,66 @@ export type TypedResponse = Response & { }; export class SessionRefreshError extends Error { + /** + * Whether the refresh failed for a transient reason (network error, timeout, + * 429, or 5xx) rather than a terminal one (the refresh token is dead). When + * `true`, the existing session is still valid and should be preserved and + * retried rather than destroyed. + */ + readonly isTransient: boolean; + constructor(cause: unknown) { super('Session refresh error', { cause }); this.name = 'SessionRefreshError'; + this.isTransient = isTransientRefreshError(cause); } } +// The WorkOS SDK's HTTP client already retries these with backoff + jitter +// internally. If one of these still surfaces, the failure is transient rather +// than a dead refresh token: request timeouts (normalized to 408), rate limits +// (429), and 5xx. +const RETRYABLE_REFRESH_STATUS_CODES = new Set([408, 429, 500, 502, 503, 504]); + +// A network-level fetch failure surfaces as a TypeError ("fetch failed" / +// "Failed to fetch"). Match its message so an unrelated programming TypeError +// (e.g. from a helper after a successful exchange) is not misclassified. +const NETWORK_ERROR_MESSAGE = /fetch failed|failed to fetch|network|load failed|terminated/i; + +// A raw network TypeError is not an HttpClientError, so the WorkOS SDK re-wraps +// it in a plain Error whose `cause` is the original TypeError. Follow the cause +// chain to recognize it. +function isNetworkError(error: unknown): boolean { + if (error instanceof TypeError) { + return NETWORK_ERROR_MESSAGE.test(error.message); + } + + if (error instanceof Error && error.cause != null && error.cause !== error) { + return isNetworkError(error.cause); + } + + return false; +} + +/** + * Classifies a refresh failure as transient (retryable) rather than terminal. + * Transient failures carry a retryable numeric `status` (408/429/5xx, mirroring + * the SDK's own retry set) or are network failures (a "fetch failed" `TypeError`, + * possibly wrapped by the SDK with the original `TypeError` as its `cause`). + * Anything else (a terminal `invalid_grant` at 400, a 401, or an unrecognized + * error) is treated as terminal. + */ +export function isTransientRefreshError(error: unknown): boolean { + if (typeof error === 'object' && error !== null && 'status' in error) { + const { status } = error; + if (typeof status === 'number' && RETRYABLE_REFRESH_STATUS_CODES.has(status)) { + return true; + } + } + + return isNetworkError(error); +} + /** * This function is used to refresh the session by using the refresh token. * It will authenticate the user with the refresh token and return a new session object. @@ -448,6 +502,7 @@ export async function authkitLoader( error: error.cause, request, sessionData: cookieSession, + isTransient: error.isTransient, }); if (result instanceof Response) { @@ -463,6 +518,16 @@ export async function authkitLoader( const returnPathname = getReturnPathname(request.url); const { url, headers: authHeaders } = await getAuthorizationUrl({ returnPathname, request }); + + // Only destroy the session for a terminal failure. A transient failure + // (network error, timeout, 429, or 5xx that survived the SDK's internal + // retries) leaves the refresh token valid, so keep the sealed cookie and + // let a later request refresh successfully rather than forcing the user + // to re-authenticate. + if (error.isTransient) { + throw redirect(url, { headers: [['Set-Cookie', authHeaders['Set-Cookie']]] }); + } + throw redirect(url, { headers: [ ['Set-Cookie', await destroySession(cookieSession)], From 316bb92e91a27df66bad8ec1782d489974f24a37 Mon Sep 17 00:00:00 2001 From: "madison.packer" Date: Thu, 23 Jul 2026 20:10:21 +0000 Subject: [PATCH 2/2] Treat known non-retryable status as terminal in refresh classifier Co-Authored-By: Devin AI <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- src/session.spec.ts | 25 +++++++++++++++++++++++++ src/session.ts | 8 ++++++-- 2 files changed, 31 insertions(+), 2 deletions(-) diff --git a/src/session.spec.ts b/src/session.spec.ts index d47cf92..af1fae3 100644 --- a/src/session.spec.ts +++ b/src/session.spec.ts @@ -867,6 +867,31 @@ describe('session', () => { } }); + it('should destroy the session for a terminal status even if its cause chain looks network-like', async () => { + // A terminal HTTP status must win over the network-cause fallback: a + // 400 that happens to wrap a "fetch failed" TypeError is still terminal. + authenticateWithRefreshToken.mockRejectedValue( + Object.assign(new Error('invalid_grant', { cause: new TypeError('fetch failed') }), { + status: 400, + error: 'invalid_grant', + }), + ); + getAuthorizationUrlMock.mockResolvedValue({ + url: 'https://auth.workos.com/oauth/authorize?state=abc123', + headers: { 'Set-Cookie': 'wos-auth-verifier-abc=sealed; Path=/; HttpOnly; SameSite=Lax; Max-Age=600' }, + }); + + try { + await authkitLoader(createLoaderArgs(createMockRequest())); + fail('Expected redirect response to be thrown'); + } catch (response: unknown) { + assertIsResponse(response); + expect(response.status).toBe(302); + expect(destroySession).toHaveBeenCalled(); + expect(response.headers.getSetCookie()).toContain('destroyed-session-cookie'); + } + }); + it('should destroy the session for a terminal refresh failure (invalid_grant)', async () => { authenticateWithRefreshToken.mockRejectedValue( Object.assign(new Error('invalid_grant'), { status: 400, error: 'invalid_grant' }), diff --git a/src/session.ts b/src/session.ts index f8b9cd2..cbab1d1 100644 --- a/src/session.ts +++ b/src/session.ts @@ -78,10 +78,14 @@ function isNetworkError(error: unknown): boolean { * error) is treated as terminal. */ export function isTransientRefreshError(error: unknown): boolean { + // A known HTTP status is authoritative: a retryable code is transient, and + // any other status (e.g. a terminal 400 `invalid_grant`) is terminal. Return + // eagerly so a terminal response is never reclassified as transient by the + // network-cause fallback below. if (typeof error === 'object' && error !== null && 'status' in error) { const { status } = error; - if (typeof status === 'number' && RETRYABLE_REFRESH_STATUS_CODES.has(status)) { - return true; + if (typeof status === 'number') { + return RETRYABLE_REFRESH_STATUS_CODES.has(status); } }