fix: Preserve session on transient refresh failures

[m0tzy]

Summary

authkitLoader caught any SessionRefreshError and destroyed the sealed session + redirected to sign-in. That includes transient failures (network error, request timeout, 429, 5xx) that survive the SDK's internal retries — so a brief outage discards a still-valid refresh token and forces re-authentication (the BaseTen lockout pattern). This is the React Router v7 port of the same fix landed for authkit-remix (workos/authkit-remix#88) and authkit-nextjs (workos/authkit-nextjs#461).

This classifies the wrapped refresh error and only destroys the session for a terminal failure. On a transient failure the sealed cookie is kept, so a later request refreshes successfully once the condition clears.

// session.ts — authkitLoader catch, on SessionRefreshError
const { url, headers: authHeaders } = await getAuthorizationUrl({ returnPathname, request });

if (error.isTransient) {
  // keep the cookie: redirect setting only the PKCE verifier cookie, no destroySession()
  throw redirect(url, { headers: [['Set-Cookie', authHeaders['Set-Cookie']]] });
}
// terminal (unchanged): destroy the session and redirect
throw redirect(url, {
  headers: [
    ['Set-Cookie', await destroySession(cookieSession)],
    ['Set-Cookie', authHeaders['Set-Cookie']],
  ],
});

SessionRefreshError now carries an isTransient flag derived from the wrapped cause, using the SDK's own retry classification — a transient HTTP response carries a retryable numeric status; a network failure surfaces as a TypeError (possibly re-wrapped by the SDK, so the classifier follows the cause chain):

const RETRYABLE_REFRESH_STATUS_CODES = new Set([408, 429, 500, 502, 503, 504]);

export function isTransientRefreshError(error: unknown): boolean {
  if (typeof error === 'object' && error !== null && 'status' in error) {
    const { status } = error;
    return typeof status === 'number' && RETRYABLE_REFRESH_STATUS_CODES.has(status);
  }
  return isNetworkError(error); // "fetch failed" TypeError, following Error.cause
}

onSessionRefreshError still runs for both cases and now receives the computed isTransient (parity with the thrown error), so consumers can override behavior. Terminal invalid_grant/401/unrecognized errors still destroy the cookie and redirect.

Test plan

• npm test (162 tests pass), npm run lint, prettier --check, and tsc --noEmit all green.
• New session.spec.ts coverage: parametrized transient cases (429, 503, 408, network TypeError, SDK-wrapped network error) assert destroySession is not called and the destroyed-session-cookie is not emitted (only the PKCE verifier cookie is); a terminal invalid_grant (400) case asserts the session is destroyed; and onSessionRefreshError receives isTransient: true/false for transient/terminal failures respectively.

Part of the AuthKit refresh-token DX work (server returns 429 for transient refresh lock timeouts in workos/workos#66577; typed transient/terminal session.refresh() in workos/workos-node#1663).

Link to Devin session: https://app.devin.ai/sessions/fc39103abf694f90b1c92dd714a81461
Requested by: @m0tzy

[devin-ai-integration]

Original prompt from madison.packer

SYSTEM:
=== BEGIN THREAD HISTORY (in #team-authkit) ===
<most_recent_message>
Madison Packer (U0ACAL99VSL): @Devin can you investigate the refresh token feedback here and plan for how we might improve the DX <https://work-os.slack.com/archives/C0APCBRV47Q/p1784811878194419|https://work-os.slack.com/archives/C0APCBRV47Q/p1784811878194419>
</most_recent_message>
=== END THREAD HISTORY ===

Thread URL: https://work-os.slack.com/archives/C0173N0DDSQ/p1784817744204199?thread_ts=1784817744.204199&amp;cid=C0173N0DDSQ

The latest message is the one right above that tagged you. The <most_recent_message> is the message that you should use to guide your goals + task for this session, and you should use the rest of the slack thread as context.

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment, CI, and merge conflict monitoring

[devin-ai-integration]

Devin Review found 1 potential issue.

[devin-ai-integration]

🔍 Transient failures still redirect to the sign-in URL

On a transient refresh failure the loader still throws a redirect to the WorkOS authorization URL (src/session.ts:527-528); it only avoids destroying the sealed cookie. Since the access token is expired (that is why refresh was attempted), the current request cannot be served with valid auth, so the user is still bounced to re-authenticate. The stated benefit ('a later request refreshes successfully once the condition clears') therefore only applies when the redirect is not followed (e.g. background/data requests) — for a normal document navigation the user will complete the WorkOS flow and get a fresh session anyway. This matches the PR description's code snippet and the new tests assert a 302 with the cookie preserved, so it appears intentional, but reviewers should confirm this is the desired UX parity with the authkit-remix/authkit-nextjs ports.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

Intentional, and it matches the authkit-remix/authkit-nextjs ports. You're right that for a normal document navigation the user still completes the WorkOS redirect (the current request's access token is expired, so it can't be served regardless). The value of preserving the cookie is two-fold: (1) concurrent/background data requests (loaders, fetchers, revalidations) that fire during the same transient blip keep a valid sealed session and can succeed instead of all tearing it down, and (2) we never throw away a still-valid refresh token on a transient 5xx/429/network error — which is the BaseTen outage-lockout scenario where destroying the session while login is also degraded locks users out. Terminal invalid_grant still destroys as before.

[m0tzy]

Don't users with an active cookie get bounced off the sign in URL?

[devin-ai-integration]

Good catch — yes, and it exposes an inconsistency that predates this PR. This catch block redirects to the sign-in URL unconditionally, but the no-session branch above (if (!session), line 422) only redirects when ensureSignedIn: true and otherwise returns UnauthorizedData so the route just renders logged-out.

So today, on an ensureSignedIn: false (optional-auth) route:

• a user with no session → renders logged-out, no bounce;
• a user with a valid, recoverable session that hits a transient refresh blip → gets bounced to WorkOS.

That's backwards, and it's exactly your point. (The access token is expired by the time we refresh — updateSession only refreshes a token that already failed verifyAccessToken — so we genuinely can't serve authenticated content for this request; but for a non-ensureSignedIn loader the right answer is to render logged-out for this one request, not to redirect.)

Proposed fix — mirror the no-session semantics in the catch:

if (error instanceof SessionRefreshError) {
  const cookieSession = await getSession(request.headers.get('Cookie'));
  // ...onSessionRefreshError callback unchanged...

  // Terminal failure destroys the cookie; transient preserves it.
  const setCookie = error.isTransient
    ? undefined
    : await destroySession(cookieSession);

  if (ensureSignedIn) {
    const { url, headers: authHeaders } = await getAuthorizationUrl({ returnPathname, request });
    const headers: [string, string][] = [];
    if (setCookie) headers.push(['Set-Cookie', setCookie]);
    headers.push(['Set-Cookie', authHeaders['Set-Cookie']]);
    throw redirect(url, { headers });
  }

  // Not ensureSignedIn: render logged-out instead of bouncing. On a transient
  // failure the sealed cookie is preserved so a later request refreshes.
  const auth: UnauthorizedData = { user: null, impersonator: null, /* ...nulls... */ };
  return await handleAuthLoader(loader, loaderArgs, auth); // + Set-Cookie header when terminal
}

Net effect: ensureSignedIn: true still redirects (that's the contract), but optional-auth routes stop bouncing users with a recoverable session on a transient blip — the sealed cookie is kept and the next navigation/data request refreshes. This also brings react-router closer to the authkit-nextjs port, which returns { user: null } + authorizationUrl rather than force-redirecting.

Want me to push this? It's a small change plus tests (transient + ensureSignedIn: false → no redirect, cookie preserved; terminal + ensureSignedIn: false → no redirect, cookie destroyed). I held off since it's a UX call on your repo.

[greptile-apps]

Greptile Summary

This PR fixes a session destruction bug where any SessionRefreshError — including transient network errors, 429, and 5xx — caused the sealed session cookie to be wiped and the user forced back through sign-in. The fix classifies each failure as transient or terminal and only destroys the cookie for terminal cases (dead refresh token).

• SessionRefreshError gets an isTransient flag computed by isTransientRefreshError, which returns true for status codes 408/429/500/502/503/504 or for TypeErrors whose message chain matches a fetch-failure pattern.
• authkitLoader splits the redirect path: transient failures redirect to the auth URL while keeping the sealed cookie, terminal failures destroy it first; onSessionRefreshError now receives isTransient so consumers can override behavior with full context.
• Test coverage adds parametrized transient cases (429, 503, 408, raw and SDK-wrapped network TypeError) asserting destroySession is not called, plus terminal-case tests and onSessionRefreshError isTransient propagation tests.

Confidence Score: 5/5

Safe to merge. The change correctly narrows session destruction to terminal failures only, and the key edge case (terminal status with a TypeError in the cause chain) is both handled by the early return and covered by a dedicated test.

The isTransientRefreshError classification is correct: the numeric-status guard returns eagerly so a 400 invalid_grant that happens to wrap a fetch failed TypeError is still treated as terminal. The onSessionRefreshError callback receives isTransient for full consumer control. Tests parametrize five transient inputs, one terminal-with-network-cause input, and two straight terminal inputs, then separately verify the flag propagates to the callback. No session data is logged or leaked in error paths.

No files require special attention. The only export not updated is refreshSession, which wraps the raw error in a plain Error rather than a SessionRefreshError, so callers of that function still need to call isTransientRefreshError themselves — but that function is now exported and this is a separate public API path outside the scope of this change.

Important Files Changed

Filename	Overview
src/session.ts	Core change: SessionRefreshError gains isTransient, new isTransientRefreshError/isNetworkError classifiers added, authkitLoader splits destroy vs preserve on transient/terminal errors. Logic is correct — numeric status earns an early return so a terminal 400 with a TypeError cause is never reclassified as transient.
src/interfaces.ts	RefreshErrorOptions gains isTransient: boolean to surface the classification in the onSessionRefreshError callback. Additive, non-breaking change.
src/session.spec.ts	83 new lines of tests covering parametrized transient cases (429, 503, 408, raw TypeError, SDK-wrapped TypeError), terminal-with-network-cause edge case, plain invalid_grant terminal case, and isTransient propagation to onSessionRefreshError. Coverage is comprehensive.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[authkitLoader] --> B[updateSession]
    B --> C{Token valid?}
    C -- Yes --> D[Return session data]
    C -- No --> E[authenticateWithRefreshToken]
    E -- Success --> F[Commit refreshed session]
    E -- Error --> G[throw SessionRefreshError\ncause = SDK error]

    G --> H{isTransientRefreshError\ncause?}
    H -- has numeric status --> I{status in\n408/429/5xx set?}
    I -- Yes --> J[isTransient = true]
    I -- No --> K[isTransient = false]
    H -- no status --> L{isNetworkError?\nTypeError cause chain}
    L -- matches fetch/network msg --> J
    L -- no match --> K

    J --> M{onSessionRefreshError\ncallback?}
    K --> M
    M -- returns Response --> N[Return callback Response]
    M -- void / no callback --> O{error.isTransient?}
    O -- true --> P[redirect to auth URL\nkeep sealed cookie]
    O -- false --> Q[destroySession\nthen redirect to auth URL]

Loading

_{Reviews (2): Last reviewed commit: "Treat known non-retryable status as term..." | Re-trigger Greptile}