Closed
Conversation
the loading gate in the mobile-ui layout blocked on !hasToken, which was set once on mount via hasValidJwtToken() and never updated. when the jwt expired after 30 days, the backend returned 401 but the cookie persisted (getJWTCookie refreshes its maxAge on every call), leaving the app in a dead state: hasToken=false permanently, spinner forever. - remove hasToken/hasValidJwtToken from the loading gate. the gate now blocks only on transient conditions (isFetchingUser, isCheckingAccount, needsRedirect) and !user (resolves once redirect navigates away) - upgrade redirect to router.replace to avoid stacking history entries - add 3s hard navigation fallback for pwa contexts where soft navigation can silently fail
…expired-jwt 🐛 fix: infinite spinner on /home when jwt expires
[TASK-18464] fix: show different error message for PIX
when backend returns 401 on /get-user, clear the jwt cookie and send Clear-Site-Data to nuke the service worker cache. this lets the client recover by redirecting to /setup for re-authentication instead of spinning forever with a stale cookie.
…r-cookie 🐛 fix: clear expired jwt cookie and sw cache on 401
redux persists user data to localStorage, so on app boot the query was skipping the backend check entirely (enabled: !authUser.userId). this meant expired JWTs were never detected — the app rendered with stale cached data while all API calls failed silently. - remove redux gate from enabled, always fetch on mount - use placeholderData instead of initialData (show cached data instantly but don't skip the fetch) - clear redux on fetch failure so layout redirect triggers
…-on-auth-failure 🐛 fix: clear stale redux user on auth failure
fix: lp faq copy
feat: show appx. amt in local currency
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
WalkthroughMultiple independent feature updates and refactorings across the mobile UI, components, and API layer. Changes include authentication redirect mechanism updates, currency conversion display enhancements, payment error handling, API cookie management improvements, Redux data model expansion, and UI component simplifications. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~28 minutes 🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Tip Issue Planner is now in beta. Read the docs and try it out! Share your feedback on Discord. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/components/ExchangeRate/index.tsx (1)
41-54: Normalize the non‑Euro rate to a numeric value before reuse.Right now
rateis assigned fromnonEruoExchangeRatedirectly, while the Euro path parses to a number. Aligning both avoids mixed types and makes the local-currency math more predictable.♻️ Suggested normalization
- rate = nonEruoExchangeRate + const parsedRate = Number(nonEruoExchangeRate) + rate = Number.isFinite(parsedRate) ? parsedRate : null🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/components/ExchangeRate/index.tsx` around lines 41 - 54, The branch that handles nonEuroCurrency assigns rate directly from nonEruoExchangeRate, causing mixed types with the USD path which uses parseFloat; update the nonEuroCurrency branch to normalize nonEruoExchangeRate to a numeric value (e.g., via parseFloat(nonEruoExchangeRate.toString()) or Number(...)) before assigning to rate, and keep displayValue, isLoadingRate and moreInfoText behavior the same so downstream math expecting a number uses a consistent numeric rate.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app/api/peanut/user/get-user-from-cookie/route.ts`:
- Around line 24-37: The Set-Cookie used to clear the jwt-token in the 401
branch omits the Secure flag, so deletion may fail in production; update the
headers assignment inside the response.status === 401 block (the headers object
and the Set-Cookie string) to include the Secure attribute only when
process.env.NODE_ENV === 'production' (i.e., append "; Secure" in production) so
it matches how the original cookie was set and ensures proper deletion; modify
the logic around headers['Set-Cookie'] in this route handler to conditionally
include Secure while keeping Path=/; Max-Age=0; SameSite=Lax.
In `@src/components/ExchangeRate/index.tsx`:
- Around line 57-63: The parsing of amountToConvert using parseFloat can
mis-handle comma-formatted values (e.g., "1,234.56"); update the logic around
localCurrencyAmount/amountToConvert to first sanitize the string by removing
thousands separators (commas) and trim whitespace, then parse to a number (e.g.,
Number(...) or parseFloat on the cleaned string) and validate with
Number.isFinite(amount) and amount > 0 before calculating (amount * rate).
Ensure the guard still checks rate && rate > 0 and preserve the toFixed(2)
assignment to localCurrencyAmount.
In `@src/hooks/query/user.ts`:
- Around line 31-36: The code currently clears Redux user state for any non-OK
fetch by calling dispatch(userActions.setUser(null)) when userResponse.status
!== 200; change this so only authentication errors trigger logout: detect 401
and 403 from userResponse.status and call dispatch(userActions.setUser(null))
only in that branch, while for other statuses (5xx, 429, etc.) just log a
warning and return null without clearing state; also ensure any fetch/network
exceptions caught by the enclosing function (e.g., in the same async function
that awaits the fetch) do not call userActions.setUser(null) on transient errors
— handle them by logging and returning null instead.
---
Nitpick comments:
In `@src/components/ExchangeRate/index.tsx`:
- Around line 41-54: The branch that handles nonEuroCurrency assigns rate
directly from nonEruoExchangeRate, causing mixed types with the USD path which
uses parseFloat; update the nonEuroCurrency branch to normalize
nonEruoExchangeRate to a numeric value (e.g., via
parseFloat(nonEruoExchangeRate.toString()) or Number(...)) before assigning to
rate, and keep displayValue, isLoadingRate and moreInfoText behavior the same so
downstream math expecting a number uses a consistent numeric rate.
Comment on lines
+24
to
+37
| const headers: Record<string, string> = { | ||
| 'Content-Type': 'application/json', | ||
| } | ||
|
|
||
| // on auth failure, clear the jwt cookie and sw cache so the client | ||
| // can recover even if running old cached code | ||
| if (response.status === 401) { | ||
| headers['Set-Cookie'] = 'jwt-token=; Path=/; Max-Age=0; SameSite=Lax' | ||
| headers['Clear-Site-Data'] = '"cache"' | ||
| } | ||
|
|
||
| return new NextResponse('Error in get-from-cookie', { | ||
| status: response.status, | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| headers, |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
rg -n "jwt-token" --type ts --type tsx --type js --type jsx -B 2 -A 2Repository: peanutprotocol/peanut-ui
Length of output: 95
🏁 Script executed:
rg -n "Set-Cookie" --type ts --type tsx --type js --type jsx -B 2 -A 2Repository: peanutprotocol/peanut-ui
Length of output: 95
🏁 Script executed:
fd -e "ts" -e "tsx" -e "js" -e "jsx" | xargs grep -l "cookie" -i | head -20Repository: peanutprotocol/peanut-ui
Length of output: 849
🏁 Script executed:
rg "jwt-token" -n -B 2 -A 2Repository: peanutprotocol/peanut-ui
Length of output: 20501
🏁 Script executed:
cat -n src/app/api/peanut/user/get-jwt-token/route.tsRepository: peanutprotocol/peanut-ui
Length of output: 2448
🏁 Script executed:
cat -n src/app/api/peanut/user/logout-user/route.tsRepository: peanutprotocol/peanut-ui
Length of output: 1092
🏁 Script executed:
cat -n src/app/api/peanut/user/get-user-from-cookie/route.ts | head -40Repository: peanutprotocol/peanut-ui
Length of output: 1761
Add Secure flag to cookie-clear Set-Cookie header in production.
The original jwt-token cookie is set with secure: process.env.NODE_ENV === 'production', but the clear operation at line 31 omits the Secure flag. Include Secure in the Set-Cookie header to maintain consistency:
headers['Set-Cookie'] = 'jwt-token=; Path=/; Max-Age=0; SameSite=Lax; Secure'
This ensures proper deletion in production environments and aligns with the original cookie configuration.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app/api/peanut/user/get-user-from-cookie/route.ts` around lines 24 - 37,
The Set-Cookie used to clear the jwt-token in the 401 branch omits the Secure
flag, so deletion may fail in production; update the headers assignment inside
the response.status === 401 block (the headers object and the Set-Cookie string)
to include the Secure attribute only when process.env.NODE_ENV === 'production'
(i.e., append "; Secure" in production) so it matches how the original cookie
was set and ensures proper deletion; modify the logic around
headers['Set-Cookie'] in this route handler to conditionally include Secure
while keeping Path=/; Max-Age=0; SameSite=Lax.
Comment on lines
+57
to
+63
| // calculate local currency amount if provided | ||
| let localCurrencyAmount: string | null = null | ||
| if (amountToConvert && rate && rate > 0) { | ||
| const amount = parseFloat(amountToConvert) | ||
| if (!isNaN(amount) && amount > 0) { | ||
| localCurrencyAmount = (amount * rate).toFixed(2) | ||
| } |
Contributor
There was a problem hiding this comment.
Guard against comma‑formatted inputs when parsing amountToConvert.
parseFloat("1,234.56") becomes 1, which will understate the local amount. Consider stripping commas and using Number.isFinite.
🛠️ Suggested guard
- const amount = parseFloat(amountToConvert)
- if (!isNaN(amount) && amount > 0) {
+ const normalized = amountToConvert.replace(/,/g, '')
+ const amount = Number(normalized)
+ if (Number.isFinite(amount) && amount > 0) {
localCurrencyAmount = (amount * rate).toFixed(2)
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // calculate local currency amount if provided | |
| let localCurrencyAmount: string | null = null | |
| if (amountToConvert && rate && rate > 0) { | |
| const amount = parseFloat(amountToConvert) | |
| if (!isNaN(amount) && amount > 0) { | |
| localCurrencyAmount = (amount * rate).toFixed(2) | |
| } | |
| // calculate local currency amount if provided | |
| let localCurrencyAmount: string | null = null | |
| if (amountToConvert && rate && rate > 0) { | |
| const normalized = amountToConvert.replace(/,/g, '') | |
| const amount = Number(normalized) | |
| if (Number.isFinite(amount) && amount > 0) { | |
| localCurrencyAmount = (amount * rate).toFixed(2) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/components/ExchangeRate/index.tsx` around lines 57 - 63, The parsing of
amountToConvert using parseFloat can mis-handle comma-formatted values (e.g.,
"1,234.56"); update the logic around localCurrencyAmount/amountToConvert to
first sanitize the string by removing thousands separators (commas) and trim
whitespace, then parse to a number (e.g., Number(...) or parseFloat on the
cleaned string) and validate with Number.isFinite(amount) and amount > 0 before
calculating (amount * rate). Ensure the guard still checks rate && rate > 0 and
preserve the toFixed(2) assignment to localCurrencyAmount.
Comment on lines
31
to
36
| } else { | ||
| // RECOVERY FIX: Log error status for debugging | ||
| if (userResponse.status === 400 || userResponse.status === 500) { | ||
| console.error('Failed to fetch user with error status:', userResponse.status) | ||
| // This indicates a backend issue - user might be in broken state | ||
| // The KernelClientProvider recovery logic will handle cleanup | ||
| } else { | ||
| console.warn('Failed to fetch user. Probably not logged in.') | ||
| } | ||
| console.warn('Failed to fetch user, status:', userResponse.status) | ||
| // clear stale redux data so the app doesn't keep serving cached user | ||
| dispatch(userActions.setUser(null)) | ||
| return null | ||
| } |
Contributor
There was a problem hiding this comment.
Clearing user data on any fetch failure may cause unintended logouts.
The current implementation clears Redux user data for all non-OK responses, including temporary network issues (500, timeouts, connection errors). This could log users out during transient server issues or brief connectivity problems.
Consider differentiating between authentication failures (401/403) and server/network errors:
🛡️ Proposed fix to preserve user data on transient errors
} else {
- console.warn('Failed to fetch user, status:', userResponse.status)
- // clear stale redux data so the app doesn't keep serving cached user
- dispatch(userActions.setUser(null))
+ console.warn('Failed to fetch user, status:', userResponse.status)
+ // only clear user data on auth failures, not transient server errors
+ if (userResponse.status === 401 || userResponse.status === 403) {
+ dispatch(userActions.setUser(null))
+ }
return null
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| } else { | |
| // RECOVERY FIX: Log error status for debugging | |
| if (userResponse.status === 400 || userResponse.status === 500) { | |
| console.error('Failed to fetch user with error status:', userResponse.status) | |
| // This indicates a backend issue - user might be in broken state | |
| // The KernelClientProvider recovery logic will handle cleanup | |
| } else { | |
| console.warn('Failed to fetch user. Probably not logged in.') | |
| } | |
| console.warn('Failed to fetch user, status:', userResponse.status) | |
| // clear stale redux data so the app doesn't keep serving cached user | |
| dispatch(userActions.setUser(null)) | |
| return null | |
| } | |
| } else { | |
| console.warn('Failed to fetch user, status:', userResponse.status) | |
| // only clear user data on auth failures, not transient server errors | |
| if (userResponse.status === 401 || userResponse.status === 403) { | |
| dispatch(userActions.setUser(null)) | |
| } | |
| return null | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/query/user.ts` around lines 31 - 36, The code currently clears
Redux user state for any non-OK fetch by calling
dispatch(userActions.setUser(null)) when userResponse.status !== 200; change
this so only authentication errors trigger logout: detect 401 and 403 from
userResponse.status and call dispatch(userActions.setUser(null)) only in that
branch, while for other statuses (5xx, 429, etc.) just log a warning and return
null without clearing state; also ensure any fetch/network exceptions caught by
the enclosing function (e.g., in the same async function that awaits the fetch)
do not call userActions.setUser(null) on transient errors — handle them by
logging and returning null instead.
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
3 participants
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.
No description provided.