fix(react): drop cached JWT when sessionId rotates#329
Conversation
|
@ramonclaudio is attempting to deploy a commit to the Convex Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe hook useUseAuthFromBetterAuth mirrors cachedToken into cachedTokenRef via a setCachedToken helper so fetchAccessToken reads the latest token. It clears the cached JWT when the Better Auth session disappears and adds lastSessionIdRef plus an effect that, on sessionId rotation, clears cachedToken and nulls pendingTokenRef.current to drop in-flight fetches tied to the old session. fetchAccessToken now prefers cachedTokenRef, reuses in-flight promises, and only updates the cache when the resolved promise is still the active one. Sequence Diagram(s)sequenceDiagram
participant Client
participant useUseAuthFromBetterAuth
participant fetchAccessToken
participant TokenEndpoint
Client->>useUseAuthFromBetterAuth: needs authenticated request
useUseAuthFromBetterAuth->>fetchAccessToken: fetchAccessToken()
fetchAccessToken->>useUseAuthFromBetterAuth: read cachedTokenRef.current
alt cached token valid
fetchAccessToken->>Client: return cached token
else no valid token
fetchAccessToken->>useUseAuthFromBetterAuth: set pendingTokenRef
fetchAccessToken->>TokenEndpoint: request new JWT
TokenEndpoint-->>fetchAccessToken: JWT response
fetchAccessToken->>useUseAuthFromBetterAuth: setCachedToken(JWT)
useUseAuthFromBetterAuth->>Client: return JWT
end
Possibly related issues
Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsGit: Failed to clone repository. Please run the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ramonclaudio
force-pushed
the
fix/react-cached-token-on-session-rotation
branch
from
5cad276 to
50dea0d
Compare
The bridge's fetchAccessToken called setCachedToken inside its `/convex/token` .then chain. The setState scheduled a re-render that landed inside Convex's fetchTokenAndGuardAgainstRace await window (authentication_manager.ts), bumped configVersion, and made the in-flight setConfig bail with isFromOutdatedConfig:true. The WebSocket stayed paused and useConvexAuth().isAuthenticated never settled to true on Hermes V1 native async (Expo SDK 56 canary 2026-05-05+ since expo/expo#45345 dropped @babel/plugin-transform-async-to-generator from the Hermes V1 preset). Drop the async keyword and wrap the body in new Promise(...). The wrapping Promise resolves via thenable adoption, which inserts the same microtask hop the regenerator runtime's _asyncToGenerator provides. The hop pushes the React re-render past the consumer's await continuation, so the next setAuth fires after setConfig completes instead of inside its await window. Refs: - get-convex#168 (Expo, open since Nov 2025) - get-convex#303 (Next.js, open since Mar 2026) - get-convex#329 (closed; addressed session-rotation leg of the same race class) - get-convex#346 (closed; iOS background resume)
… on Hermes V1 The /convex/token response triggers a session rotation (via Better Auth's Set-Cookie processing) plus a setCachedToken call inside the bridge's .then. The next render rebuilds fetchAccessToken's useCallback (keyed on [sessionId]) and fires ConvexAuthStateFirstEffect's client.setAuth a second time. On Hermes V1 native async (Expo SDK 56 canary 2026-05-05+ since expo/expo#45345 dropped @babel/plugin-transform-async-to-generator), that second setAuth lands inside the first setConfig's await window in authentication_manager.ts. fetchTokenAndGuardAgainstRace bumps configVersion on entry and the original await sees the stale value, returning isFromOutdatedConfig: true. setConfig bails without resumeSocket() and the chain repeats. Drop the async keyword and wrap the body in new Promise(executor) directly. The constructor's resolve(thenable) schedules a NewPromiseResolveThenableJob microtask, the same hop regenerator's _asyncToGenerator provides. With the hop in place the second setAuth lands after the first setConfig finishes rather than during its await window. Refs: - get-convex#168 (Expo, open since Nov 2025) - get-convex#303 (Next.js, open since Mar 2026) - get-convex#329 (closed; addressed session-rotation leg of the same race class) - get-convex#346 (closed; iOS background resume)
ramonclaudio
changed the title
ramonclaudio
force-pushed
the
fix/react-cached-token-on-session-rotation
branch
from
1b21b5a to
2ed5be1
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/react/index.tsx`:
- Around line 140-154: The effect drops the pendingTokenRef pointer but doesn't
prevent an already-started fetchAccessToken promise from later calling
setCachedToken and restoring a stale JWT; change the
fetchAccessToken/pendingTokenRef flow so each token request carries a unique
request id or the promise object and, before calling setCachedToken(token),
verify that pendingTokenRef.current still matches that id/promise (or that a
monotonic tokenRequestVersion equals the version captured at start). Also, when
the session rotates in the useEffect (where lastSessionIdRef and pendingTokenRef
are handled), increment/clear that version or set pendingTokenRef to a sentinel
so any in-flight .then handlers will detect the mismatch and abort calling
setCachedToken; update places that assign/clear pendingTokenRef and the
.then/.catch handlers in fetchAccessToken to perform this guard.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository: get-convex/coderabbit/.coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: abee74a9-72d1-4c6d-9c8e-a05779a27311
📒 Files selected for processing (1)
src/react/index.tsx
ramonclaudio
force-pushed
the
fix/react-cached-token-on-session-rotation
branch
5 times, most recently
from
5fdf934 to
9469884
Compare
|
Hey @erquhart, sorry for the multiple pings here. Just did a bunch of testing so I could provide useful repros. The unit tests show this fix works at the React layer (6/9 fail without it, all pass with it). But the Expo and TanStack real-app repros show the user-visible Curious for your read on the upstream side (better-auth#9345). I opened this PR in Better Auth a few weeks ago after closing this one initially. Is preserving the caller's session on change-password the right call, or is the rotation actually the right behavior and users should just ignore the flicker? If neither direction feels right, happy to close this and #9345. Don't want to pile on review work. |
ramonclaudio
force-pushed
the
fix/react-cached-token-on-session-rotation
branch
from
9469884 to
37bdd4b
Compare
commit: |
1 participant
cachedTokeninConvexBetterAuthProviderdoesn't get cleared when the session id changes (only on logout). So right after a session rotation (password change), the next call tofetchAccessToken({ forceRefreshToken: false })hands back the JWT for the now-deleted session.This PR backs
cachedTokenwith a ref so the fetcher reads the current value instead of the one captured at the last render. The rotation check runs inline in the fetcher and clears the ref whensessionIdchanges. A promise-identity guard in.then/.catch/.finallykeeps a late-resolving staletoken()fetch from overwriting a fresh value.Unit test repro: 6/9 fail on
v0.12.2, all pass with this fix.Worth flagging: this doesn't close the visible
Invalid session IDflicker onchangePassword({ revokeOtherSessions: true }). Tested end-to-end on repros with real credentials (Expo test repro, TanStack test repro) but the patched build behaves the same as vanilla on that flow. Looks like an upstream Convex issue, same shape as convex-js#82 but I am honestly not 100% sure. better-auth#9345 is the upstream complement on the Better Auth side that would preserve the caller's session on change-password instead of rotating it.