-
Notifications
You must be signed in to change notification settings - Fork 430
feat(clerk-js): Improve getToken offline handling
#7598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
feat(clerk-js): Improve getToken offline handling
#7598
Conversation
- Trigger background refresh when needsRefresh is true, without requiring refreshIfStale option. Guarantees cache revalidation without relying solely on the poller. - Add #refreshTokenInBackground() that doesn't cache pending promise, allowing concurrent getToken() calls to return stale token while refresh is in progress. - Track in-flight background refreshes to prevent duplicate requests. - Rename DEFAULT_LEEWAY to BACKGROUND_REFRESH_THRESHOLD_IN_SECONDS with clear documentation about 15s minimum and rate limiting warning. - Add comprehensive JSDoc for leewayInSeconds option explaining minimum value, default, and rate limiting considerations.
Add upgrade guide entries for: - getToken `leewayInSeconds` minimum of 15 seconds - stale-while-revalidate pattern for session tokens
- Test refreshIfStale: true forces synchronous refresh when token is stale - Test refreshIfStale: true with fresh token returns cached value - Test leewayInSeconds triggers earlier background refresh - Test minimum 15s leeway enforcement - Test no background refresh when token has sufficient TTL
- Rename option from leewayInSeconds to backgroundRefreshThreshold for clarity - Lower minimum threshold from 15s to 5s (poller interval) - Remove explicit refreshIfStale: false in AuthCookieService (default behavior) - Update tests and upgrade documentation
🦋 Changeset detectedLatest commit: 9de4869 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| if (ClerkOfflineError.is(e)) { | ||
| throw e; | ||
| } | ||
|
|
||
| if (isNetworkError(e) || !isValidBrowserOnline()) { | ||
| throw new ClerkOfflineError('Network request failed', { | ||
| cause: e instanceof Error ? e : undefined, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this something we want to happen for all resources, or just the session resource?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've change this since the last refactor, I've only kept the ClerkOfflineError exception only on session resource
|
@bratsos I am concerned that this PR is going to conflict heavily with the stale-while-revalidate change: #7317 I think we will want to base this PR off feat/stale-while-revalidate-token and build on top of it. Some of the offline heuristics work well with SWR since we can return a cached token whenever possible and error only when we truly cannot fetch a new token |
# Conflicts: # packages/clerk-js/bundlewatch.config.json
Replace the stale-while-revalidate approach with a timer-based proactive refresh mechanism. Instead of relying on getToken() calls to trigger background refresh, we now schedule a timer when tokens are cached that fires before the leeway period begins. Key changes: - Add onRefresh callback to TokenCacheEntry for proactive refresh - Schedule refresh timer at (TTL - leeway - 2s) = 43s for 60s tokens - Remove needsRefresh flag and refreshThreshold parameter from cache - Remove backgroundRefreshThreshold and refreshIfStale from GetTokenOptions - Update Session to pass onRefresh callback when caching tokens Benefits: - More consistent refresh regardless of getToken() call frequency - Zero risk: if timer doesn't fire, poller catches it as fallback - Fits naturally with existing cleanup timer pattern
BREAKING CHANGE: `getToken()` now throws `ClerkOfflineError` instead of returning `null` when the client is offline. This makes it explicit that the failure was due to network conditions, not authentication state.
b6350c6 to
640e362
Compare
getToken offline handlinggetToken offline handling
| '@clerk/tanstack-react-start': major | ||
| '@clerk/react-router': major | ||
| '@clerk/clerk-js': major | ||
| '@clerk/upgrade': major | ||
| '@clerk/nextjs': major | ||
| '@clerk/shared': major | ||
| '@clerk/react': major | ||
| '@clerk/nuxt': major | ||
| '@clerk/vue': major |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this should be a major change in all sdks 🤔
Description
offline-demo.mp4
Checklist
pnpm testruns as expected.pnpm buildruns as expected.Type of change