Conversation
There was a problem hiding this comment.
Pull request overview
This PR revises the app’s periodic auth refresh behavior by moving the refresh-interval hook out of src/layouts and into the auth feature hook module, then updating layouts to consume the new hook location.
Changes:
- Deleted
src/layouts/useAuthRefreshInterval.ts(previous refresh interval implementation). - Added
useAuthRefreshIntervaltosrc/features/auth/hooks/useAuth.tsusingauthRefreshToken. - Updated
UserLayoutandAdminLayoutimports to reference the hook from@/features/auth/hooks/useAuth.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| src/layouts/useAuthRefreshInterval.ts | Removed the previous refresh-interval implementation (token presence check, in-flight de-dupe, query invalidation/error handling). |
| src/layouts/UserLayout.tsx | Updated to import useAuthRefreshInterval from the auth feature hook module. |
| src/layouts/AdminLayout.tsx | Updated to import useAuthRefreshInterval from the auth feature hook module (hook call moved below useState). |
| src/features/auth/hooks/useAuth.ts | Introduced a new useAuthRefreshInterval implementation based on SDK authRefreshToken. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| useEffect(() => { | ||
| console.log("Setting up auth refresh interval"); | ||
| const interval = setInterval(() => { | ||
| console.log("Refreshing auth token"); | ||
| authRefreshToken(""); | ||
| }, DEFAULT_AUTH_REFRESH_INTERVAL); | ||
|
|
||
| return () => clearInterval(interval); | ||
| }, []); |
There was a problem hiding this comment.
There’s no success/error handling around the refresh call to keep React Query auth state consistent. Previously, refresh failures cleared ["user","me"] and refresh success invalidated it; without similar handling, useAuth() may stay authenticated with stale cached user data even when the refresh token expires and subsequent requests start returning 401.
| useEffect(() => { | |
| console.log("Setting up auth refresh interval"); | |
| const interval = setInterval(() => { | |
| console.log("Refreshing auth token"); | |
| authRefreshToken(""); | |
| }, DEFAULT_AUTH_REFRESH_INTERVAL); | |
| return () => clearInterval(interval); | |
| }, []); | |
| const qc = useQueryClient(); | |
| useEffect(() => { | |
| console.log("Setting up auth refresh interval"); | |
| const interval = setInterval(() => { | |
| console.log("Refreshing auth token"); | |
| authRefreshToken("") | |
| .then(() => { | |
| // Refresh succeeded: ensure user data is up to date. | |
| qc.invalidateQueries({ queryKey: ["user", "me"] }); | |
| }) | |
| .catch(() => { | |
| // Refresh failed (e.g., expired/invalid token): clear cached user. | |
| qc.removeQueries({ queryKey: ["user", "me"] }); | |
| }); | |
| }, DEFAULT_AUTH_REFRESH_INTERVAL); | |
| return () => clearInterval(interval); | |
| }, [qc]); |
| const interval = setInterval(() => { | ||
| console.log("Refreshing auth token"); | ||
| authRefreshToken(""); | ||
| }, DEFAULT_AUTH_REFRESH_INTERVAL); |
There was a problem hiding this comment.
authRefreshToken("") is invoked with an empty refresh token and without the request options used elsewhere (e.g. credentials: "include"). This bypasses the existing authService.refreshAccessToken() logic (stored token lookup, rotation handling, 404 expiry handling) and will likely make refresh fail or behave incorrectly. Use authService.refreshAccessToken() here (or pass the stored refresh token + the same default request options) and handle the returned promise.
src/features/auth/hooks/useAuth.ts
Outdated
| console.log("Setting up auth refresh interval"); | ||
| const interval = setInterval(() => { | ||
| console.log("Refreshing auth token"); |
There was a problem hiding this comment.
These console.log statements will run for every mount and every refresh tick, creating noisy logs (and potentially leaking auth-related timing info in production). Please remove them or route through the app’s logging facility behind an environment/debug flag.
| console.log("Setting up auth refresh interval"); | |
| const interval = setInterval(() => { | |
| console.log("Refreshing auth token"); | |
| const interval = setInterval(() => { |
| const interval = setInterval(() => { | ||
| console.log("Refreshing auth token"); | ||
| authRefreshToken(""); | ||
| }, DEFAULT_AUTH_REFRESH_INTERVAL); |
There was a problem hiding this comment.
The interval is set up unconditionally and the callback doesn’t guard against overlapping refreshes or unauthenticated sessions. The previous implementation checked getStoredRefreshToken() and de-duped in-flight refreshes; without that, this can spam refresh requests (including for logged-out users) if the request is slow or multiple layouts mount. Consider reintroducing the stored-token check and an in-flight guard (e.g., via useRef) before calling refresh.
Type of changes
Purpose