fix(auth): prevent infinite session loop with non-reactive provider - #481
Merged
Conversation
Root cause: crossDomainClient plugin notifies $sessionSignal on EVERY response with set-better-auth-cookie header, causing useSession to refetch endlessly. Fix: - Add deduplicating storage that only writes when value actually changes - Replace ConvexBetterAuthProvider with custom StableAuthProvider - Use one-time session fetch instead of reactive useSession hook - Implement custom useStableConvexAuth for Convex token management
|
🚅 Deployed to the openchat-pr-481 environment in OpenChat
|
Contributor
🚀 Preview Deployment Ready
Convex Preview Backend
🤖 Deployed automatically by GitHub Actions |
Contributor
Greptile SummaryThis PR fixes a critical infinite loop bug causing ~4 req/sec to Key changes:
Issues found:
The approach is sound and addresses the root cause, but the two bugs in Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant User
participant Component
participant StableAuthProvider
participant AuthHook
participant AuthClient
participant Storage
participant ConvexProvider
participant Backend
Note over User,Backend: Initial Mount
User->>Component: Load App
Component->>StableAuthProvider: Mount
activate StableAuthProvider
StableAuthProvider->>AuthClient: getSession once
AuthClient->>Backend: Fetch session
Backend-->>AuthClient: Session data
AuthClient->>Storage: setItem check
Note over Storage: Value unchanged skip write
AuthClient-->>StableAuthProvider: Session result
StableAuthProvider->>StableAuthProvider: Update state
deactivate StableAuthProvider
Component->>AuthHook: Mount
activate AuthHook
AuthHook->>AuthClient: getSession once
AuthClient->>Backend: Fetch session
Backend-->>AuthClient: Session response
AuthClient->>Storage: setItem check
Note over Storage: Same value no write
AuthClient-->>AuthHook: Session data
AuthHook->>AuthClient: Get access details
AuthClient->>Backend: Fetch details
Backend-->>AuthClient: Details response
AuthClient-->>AuthHook: Details result
AuthHook->>AuthHook: Update state
deactivate AuthHook
Note over User,Backend: Convex Query
Component->>ConvexProvider: useQuery
ConvexProvider->>AuthHook: fetchAccessToken
AuthHook-->>ConvexProvider: Return cached value
ConvexProvider->>Backend: Convex query
Backend-->>ConvexProvider: Query result
ConvexProvider-->>Component: Data
Note over Component,Storage: Solution prevents infinite loop
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| const [token, setToken] = useState<string | null>(null); | ||
| const [isLoading, setIsLoading] = useState(true); | ||
| const [isAuthenticated, setIsAuthenticated] = useState(false); | ||
| const fetchedRef = { current: false }; |
Contributor
There was a problem hiding this comment.
logic: fetchedRef should be useRef(false) instead of a plain object
Suggested change
| const fetchedRef = { current: false }; | |
| const fetchedRef = useRef(false); |
Comment on lines
+105
to
+109
| crossDomainClient({ | ||
| storage: deduplicatingStorage, | ||
| // Disable local session cache - we manage caching ourselves | ||
| disableCache: true, | ||
| }), |
Contributor
There was a problem hiding this comment.
style: disableCache option may not exist on crossDomainClient - verify this in Better Auth docs to avoid runtime errors. Does the crossDomainClient plugin actually support a disableCache option?
Comment on lines
+57
to
+67
| const fetchAccessToken = useCallback(async () => { | ||
| if (token) return token; | ||
| try { | ||
| const result = await authClient.convex.token(); | ||
| const newToken = result.data?.token || null; | ||
| setToken(newToken); | ||
| return newToken; | ||
| } catch { | ||
| return null; | ||
| } | ||
| }, [token]); |
Contributor
There was a problem hiding this comment.
logic: early return with cached token doesn't refresh when token expires, causing potential auth failures
remove early return to always fetch fresh token
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CRITICAL FIX: Stops the Better Auth session spam (~4 req/sec to
/api/auth/get-session) that was causing millions of Convex function calls and making the app unusable.Root Cause
The
crossDomainClientplugin from@convex-dev/better-authcallsstore.notify("$sessionSignal")on EVERY response withset-better-auth-cookieheader. This triggersuseSessionto refetch, creating an infinite loop:set-better-auth-cookieheader$sessionSignaluseSessionreacts and refetchesFix
$sessionSignalChanges
apps/web/src/lib/auth-client.tsx(renamed from .ts): Added deduplicating storage, StableAuthProvider, non-reactive useAuthapps/web/src/providers/index.tsx: Replaced ConvexBetterAuthProvider with StableAuthProvider + ConvexProviderWithAuthTesting