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 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
if (error.isTransient) {
  // keep the cookie: redirect without destroySession()
  throw redirect(await getAuthorizationUrl({ returnPathname }));
}
// terminal (unchanged): destroy the session and redirect
throw redirect(await getAuthorizationUrl({ returnPathname }), {
  headers: { 'Set-Cookie': await destroySession(cookieSession) },
});

SessionRefreshError now carries an isTransient flag derived from the wrapped cause, using the SDK's own retry classification — a network failure surfaces as a TypeError; transient HTTP responses carry a retryable numeric status:

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

export function isTransientRefreshError(error: unknown): boolean {
  if (error instanceof TypeError) return true; // network / DNS / connection reset
  if (typeof error === 'object' && error !== null && 'status' in error) {
    const { status } = error;
    return typeof status === 'number' && RETRYABLE_REFRESH_STATUS_CODES.has(status);
  }
  return false; // terminal: invalid_grant (400), 401, or unrecognized
}

onSessionRefreshError still runs for both cases, so consumers can override behavior. Terminal invalid_grant/401/unrecognized errors still destroy the cookie and redirect.

Test plan

• npm test (95 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) assert destroySession is not called and no Set-Cookie is emitted; a terminal invalid_grant (400) case asserts the session is destroyed.

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; matching fix for Next.js in workos/authkit-nextjs#461).

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 classification depends on WorkOS SDK error shape

isTransientRefreshError (src/session.ts:58-69) relies on two assumptions about what authenticateWithRefreshToken throws after the SDK exhausts its internal retries: (1) network-level failures surface as a raw TypeError, and (2) transient HTTP responses expose a numeric status property (not statusCode) with values in {408,429,500,502,503,504}. If the WorkOS SDK wraps network errors in its own exception class (rather than letting a TypeError propagate) or exposes the code under a different property name, transient failures would be misclassified as terminal and the session would still be destroyed. This could not be verified from the repo since @workos-inc/node was not installed in node_modules. Worth confirming against the SDK's actual error contract.

---

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

[devin-ai-integration]

Good flag — I verified against workos-node's actual error contract and it exposed a real gap. Two findings:

1. Transient HTTP responses do carry a numeric status: the SDK maps them to OauthException/GenericServerException/RateLimitExceededException, all of which set a numeric status (not statusCode), and request timeouts are normalized to 408 by the fetch client. So the status-based branch is correct.
2. Network failures do not reach the caller as a TypeError. The fetch client throws a raw TypeError, but WorkOS.handleHttpError re-wraps anything that isn't an HttpClientError in new Error('Unexpected error: ...', { cause: originalTypeError }). So the bare instanceof TypeError check was effectively dead for real SDK usage.

Fixed in the latest commit: the classifier now follows the cause chain and matches a TypeError only when its message looks like a network failure. Added a test for the SDK-wrapped shape.

[greptile-apps]

Greptile Summary

This PR fixes a session durability bug where any SessionRefreshError — including transient network/rate-limit failures — caused the sealed session cookie to be destroyed, forcing unnecessary re-authentication. It introduces a isTransient classification on SessionRefreshError so that only terminal failures (invalid token, 401, unrecognized errors) destroy the cookie.

• Adds isTransientRefreshError / isNetworkError helpers that mirror the WorkOS SDK's own retry classification: retryable HTTP status codes (408, 429, 5xx) and network-level TypeErrors (with cause-chain following to handle the SDK's re-wrapping) are transient; everything else is terminal.
• Forks the authkitLoader error path: transient failures redirect to the auth URL without touching the cookie; terminal failures destroy the session as before. isTransient is also threaded into the onSessionRefreshError callback so consumers can differentiate without reimplementing the classification.
• New parametrized tests cover all transient statuses (429, 503, 408), a raw network TypeError, an SDK-wrapped TypeError, and a terminal invalid_grant (400), asserting the presence or absence of destroySession and Set-Cookie in each case.

Confidence Score: 5/5

Safe to merge. The change is well-scoped, thoroughly tested, and the session-preservation path is additive — it touches no existing terminal-error logic.

The transient/terminal split is correctly placed after the onSessionRefreshError callback, tests exercise every code path (transient statuses, SDK-wrapped TypeError, terminal invalid_grant), and the isNetworkError cause-chain follower is correctly guarded against infinite recursion. No pre-existing behavior is removed or weakened.

No files require special attention.

Important Files Changed

Filename	Overview
src/session.ts	Core change: adds isTransient to SessionRefreshError, exports isTransientRefreshError/isNetworkError helpers, and forks the authkitLoader error path to skip destroySession on transient failures. Logic is sound.
src/interfaces.ts	Adds isTransient: boolean to RefreshErrorOptions, surfacing the classification to onSessionRefreshError consumers. Clean, additive change.
src/session.spec.ts	Adds parametrized tests for transient statuses (429, 503, 408, TypeError, SDK-wrapped TypeError) asserting no destroySession; adds terminal (400/invalid_grant) and isTransient callback pass-through tests. Coverage is thorough.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[authkitLoader request] --> B[updateSession]
    B --> C{Access token valid?}
    C -- Yes --> D[Return session data]
    C -- No --> E[authenticateWithRefreshToken]
    E -- Success --> F[Encrypt & commit session]
    F --> D
    E -- Failure --> G[throw SessionRefreshError\n isTransient = isTransientRefreshError cause]
    G --> H{onSessionRefreshError\ndefined?}
    H -- Yes --> I[Call onSessionRefreshError\nerror, request, sessionData, isTransient]
    I -- Returns Response --> J[Return response]
    I -- Returns void --> K{error.isTransient?}
    H -- No --> K
    K -- true: 408/429/5xx/network --> L[redirect to auth URL\nkeep session cookie intact]
    K -- false: 400/401/invalid_grant --> M[destroySession\nredirect to auth URL\nSet-Cookie: destroyed]

Loading

_{Reviews (4): Last reviewed commit: "Add onSessionRefreshError transient cove..." | Re-trigger Greptile}

[devin-ai-integration]

Re: the "Comments Outside Diff" note that onSessionRefreshError can't observe isTransient — addressed in the latest commit. RefreshErrorOptions now includes isTransient: boolean, and authkitLoader threads error.isTransient into the callback so consumers can branch on transient vs. terminal without re-classifying.

[devin-ai-integration]

Re: the coverage note that onSessionRefreshError tests only exercised terminal errors — added two tests that assert the callback receives isTransient: true for a transient failure (503) and isTransient: false for a terminal one (400). All 40 tests pass.

[nicknisi]

This looks good but should it be on workos/authkit-react-router instead/too?