fix(frontend): safely handle empty or invalid auth JSON responses#274
Open
SoorejS wants to merge 3 commits intoAOSSIE-Org:mainfrom
Open
fix(frontend): safely handle empty or invalid auth JSON responses#274SoorejS wants to merge 3 commits intoAOSSIE-Org:mainfrom
SoorejS wants to merge 3 commits intoAOSSIE-Org:mainfrom
Conversation
📝 WalkthroughWalkthroughThe auth context and authentication forms were updated to safely parse API responses (via Changes
Sequence Diagram(s)sequenceDiagram
participant UI as Client UI
participant Auth as AuthProvider
participant API as Backend API
participant Store as LocalState/Storage
UI->>Auth: call login(credentials)
Auth->>API: fetch /login
API-->>Auth: response (maybe empty/non-JSON)
Auth->>Auth: safeJson(response) -> payload/null
alt response.ok and payload valid
Auth->>Store: set tokens, set user
Auth-->>UI: { success: true }
else error or invalid payload
Auth->>Auth: setApiError(payload, fallback)
Auth-->>UI: { success: false }
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
![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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/context/authContext.tsx (1)
144-163: Clear stale error state at the start of signup.Unlike login, signup doesn’t reset
error, so a previous failure can persist after a successful attempt. Consider mirroring this in the other auth flows too.🔧 Proposed fix
const signup = async (email: string, password: string) => { setLoading(true); + setError(null); try {
🤖 Fix all issues with AI agents
In `@frontend/src/context/authContext.tsx`:
- Around line 110-138: The login function currently casts safeJson's result to
AuthPayload and checks payload.accessToken without guarding against null,
causing a runtime TypeError when safeJson returns null; update the runtime
checks in login to treat the parsed data safely (e.g., use data as AuthPayload |
null and check payload?.accessToken or data?.accessToken before throwing) and
ensure you verify payload (or data) is non-null before proceeding to setToken,
localStorage, setUser, and navigate; apply the identical null-guard fix to
googleLogin as well to prevent the same failure when safeJson returns null.
Comment on lines
110
to
+138
| const login = async (email: string, password: string) => { | ||
| setLoading(true); | ||
| setError(null); | ||
|
|
||
| try { | ||
| const response = await fetch(`${baseURL}/login`, { | ||
| const res = await fetch(`${baseURL}/login`, { | ||
| method: 'POST', | ||
| headers: { 'Content-Type': 'application/json' }, | ||
| body: JSON.stringify({ email, password }), | ||
| }); | ||
|
|
||
| const data = await response.json(); | ||
| if (!response.ok) throw new Error(data.message || 'Login failed'); | ||
|
|
||
| setToken(data.accessToken); | ||
| localStorage.setItem('token', data.accessToken); | ||
| // Set user details in userAtom based on the new User type | ||
| const normalizedUser: User = { | ||
| id: data.user?.id || data.user?._id || undefined, | ||
| email: data.user?.email || email, | ||
| displayName: data.user?.displayName || 'User', | ||
| bio: data.user?.bio || '', | ||
| rating: data.user?.rating || 1500, | ||
| rd: data.user?.rd || 350, // Default Glicko-2 RD value | ||
| volatility: data.user?.volatility || 0.06, // Default Glicko-2 volatility | ||
| lastRatingUpdate: | ||
| data.user?.lastRatingUpdate || new Date().toISOString(), | ||
| avatarUrl: | ||
| data.user?.avatarUrl || 'https://avatar.iran.liara.run/public/10', | ||
| twitter: data.user?.twitter || undefined, | ||
| instagram: data.user?.instagram || undefined, | ||
| linkedin: data.user?.linkedin || undefined, | ||
| password: '', // Password should not be stored in client-side state | ||
| nickname: data.user?.nickname || 'User', | ||
| isVerified: data.user?.isVerified || false, | ||
| verificationCode: data.user?.verificationCode || undefined, | ||
| resetPasswordCode: data.user?.resetPasswordCode || undefined, | ||
| createdAt: data.user?.createdAt || new Date().toISOString(), | ||
| updatedAt: data.user?.updatedAt || new Date().toISOString(), | ||
| }; | ||
| setUser(normalizedUser); | ||
| localStorage.setItem(USER_CACHE_KEY, JSON.stringify(normalizedUser)); | ||
| const data = await safeJson(res); | ||
| if (!res.ok) setApiError(data, 'Login failed'); | ||
|
|
||
| const payload = data as AuthPayload; | ||
| if (!payload.accessToken || !payload.user?.email) { | ||
| throw new Error('Invalid auth payload'); | ||
| } | ||
|
|
||
| setToken(payload.accessToken); | ||
| localStorage.setItem('token', payload.accessToken); | ||
| setUser(payload.user); | ||
| localStorage.setItem(USER_CACHE_KEY, JSON.stringify(payload.user)); | ||
| navigate('/'); | ||
| } catch (error) { | ||
| handleError(error); | ||
|
|
||
| return { success: true }; | ||
| } catch (err) { | ||
| setError(err instanceof Error ? err.message : 'Login failed'); | ||
| return { success: false }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the file and check its size first
wc -l frontend/src/context/authContext.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 103
🏁 Script executed:
# Search for safeJson function definition
rg "safeJson" frontend/src/ -B 2 -A 5Repository: AOSSIE-Org/DebateAI
Length of output: 4891
🏁 Script executed:
# Search for AuthPayload type definition
rg "AuthPayload" frontend/src/ -B 2 -A 5Repository: AOSSIE-Org/DebateAI
Length of output: 1607
🏁 Script executed:
# Read lines 110-140 to see the login function
sed -n '110,140p' frontend/src/context/authContext.tsxRepository: AOSSIE-Org/DebateAI
Length of output: 1040
🏁 Script executed:
# Check if the same pattern exists in googleLogin
rg "const payload = data as AuthPayload" frontend/src/context/authContext.tsx -B 5 -A 8Repository: AOSSIE-Org/DebateAI
Length of output: 998
Use optional chaining to guard against null payload from safeJson.
When safeJson fails to parse JSON, it returns null. The TypeScript cast to AuthPayload doesn't protect against this at runtime. If data is null, accessing payload.accessToken throws TypeError: Cannot read properties of null instead of the intended error message. The second condition already uses optional chaining, but the first does not.
🔧 Proposed fix
const payload = data as AuthPayload;
- if (!payload.accessToken || !payload.user?.email) {
+ if (!payload?.accessToken || !payload?.user?.email) {
throw new Error('Invalid auth payload');
}Apply the same fix to the googleLogin function, which has the identical vulnerability.
🤖 Prompt for AI Agents
In `@frontend/src/context/authContext.tsx` around lines 110 - 138, The login
function currently casts safeJson's result to AuthPayload and checks
payload.accessToken without guarding against null, causing a runtime TypeError
when safeJson returns null; update the runtime checks in login to treat the
parsed data safely (e.g., use data as AuthPayload | null and check
payload?.accessToken or data?.accessToken before throwing) and ensure you verify
payload (or data) is non-null before proceeding to setToken, localStorage,
setUser, and navigate; apply the identical null-guard fix to googleLogin as well
to prevent the same failure when safeJson returns null.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/Pages/Authentication/forms.tsx`:
- Around line 395-409: The current flow calls confirmForgotPassword(email, code,
newPassword) and then unconditionally calls login(email, newPassword) and
handlePasswordReset(), but the new auth contract returns { success: boolean } so
you must check those results; update the flow to await confirmForgotPassword and
verify its returned object's success is true before calling login, and similarly
await login(...) and only call handlePasswordReset() when login returns success;
if either returns { success: false } setLocalError with an appropriate message
and return to avoid false-positive success.
Comment on lines
395
to
409
| setLocalError(null); | ||
|
|
||
| if (newPassword !== confirmNewPassword) { | ||
| authContext.handleError('Passwords do not match'); | ||
| setLocalError('Passwords do not match'); | ||
| return; | ||
| } | ||
|
|
||
| await confirmForgotPassword(email, code, newPassword); | ||
| await login(email, newPassword); | ||
| handlePasswordReset(); | ||
| try { | ||
| await confirmForgotPassword(email, code, newPassword); | ||
| await login(email, newPassword); | ||
| handlePasswordReset(); | ||
| } catch (err) { | ||
| console.error('Password reset error:', err); | ||
| setLocalError('Failed to reset password. Please try again.'); | ||
| } |
There was a problem hiding this comment.
Handle { success } results to avoid false-positive reset success.
With the new auth contract returning { success: boolean }, this flow will continue even if reset/login fails but doesn’t throw. It should gate login and handlePasswordReset() on success flags.
✅ Suggested fix
- try {
- await confirmForgotPassword(email, code, newPassword);
- await login(email, newPassword);
- handlePasswordReset();
- } catch (err) {
+ try {
+ const { success: resetSuccess } = await confirmForgotPassword(email, code, newPassword);
+ if (!resetSuccess) {
+ setLocalError('Failed to reset password. Please try again.');
+ return;
+ }
+ const { success: loginSuccess } = await login(email, newPassword);
+ if (!loginSuccess) {
+ setLocalError('Password reset succeeded, but login failed. Please sign in again.');
+ return;
+ }
+ handlePasswordReset();
+ } catch (err) {
console.error('Password reset error:', err);
setLocalError('Failed to reset password. Please try again.');
}📝 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
| setLocalError(null); | |
| if (newPassword !== confirmNewPassword) { | |
| authContext.handleError('Passwords do not match'); | |
| setLocalError('Passwords do not match'); | |
| return; | |
| } | |
| await confirmForgotPassword(email, code, newPassword); | |
| await login(email, newPassword); | |
| handlePasswordReset(); | |
| try { | |
| await confirmForgotPassword(email, code, newPassword); | |
| await login(email, newPassword); | |
| handlePasswordReset(); | |
| } catch (err) { | |
| console.error('Password reset error:', err); | |
| setLocalError('Failed to reset password. Please try again.'); | |
| } | |
| setLocalError(null); | |
| if (newPassword !== confirmNewPassword) { | |
| setLocalError('Passwords do not match'); | |
| return; | |
| } | |
| try { | |
| const { success: resetSuccess } = await confirmForgotPassword(email, code, newPassword); | |
| if (!resetSuccess) { | |
| setLocalError('Failed to reset password. Please try again.'); | |
| return; | |
| } | |
| const { success: loginSuccess } = await login(email, newPassword); | |
| if (!loginSuccess) { | |
| setLocalError('Password reset succeeded, but login failed. Please sign in again.'); | |
| return; | |
| } | |
| handlePasswordReset(); | |
| } catch (err) { | |
| console.error('Password reset error:', err); | |
| setLocalError('Failed to reset password. Please try again.'); | |
| } |
🤖 Prompt for AI Agents
In `@frontend/src/Pages/Authentication/forms.tsx` around lines 395 - 409, The
current flow calls confirmForgotPassword(email, code, newPassword) and then
unconditionally calls login(email, newPassword) and handlePasswordReset(), but
the new auth contract returns { success: boolean } so you must check those
results; update the flow to await confirmForgotPassword and verify its returned
object's success is true before calling login, and similarly await login(...)
and only call handlePasswordReset() when login returns success; if either
returns { success: false } setLocalError with an appropriate message and return
to avoid false-positive success.
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.
Fixes #244
This PR fixes a frontend crash caused by auth endpoints occasionally
returning empty, non-JSON, or malformed responses, which previously
caused the application to crash during authentication flows.
Why this change was necessary:
The frontend auth layer assumed that all auth endpoints always return
valid JSON and accessed response bodies directly via response.json()
and unchecked property reads (e.g. data.message, data.accessToken).
In real-world error paths (validation failure, auth failure, early
returns), the backend may return:
This caused runtime crashes such as:
"Unexpected end of JSON input"
What changed (with code-level reasoning):
Introduced safe JSON parsing using a guarded helper (safeJson)
→ Prevents crashes when response.json() fails due to empty or
invalid response bodies.
Removed unsafe double-reading of response bodies
→ Ensures each response body is consumed exactly once, avoiding
stream consumption errors caused by calling json() and text()
on the same response.
Replaced unsafe property access on unknown data
→ All error fields (message / error) are now accessed only after
locally casting the response payload, preventing runtime and
TypeScript errors.
Added explicit null and shape checks before accessing auth fields
→ Prevents undefined access when accessToken or user objects are
missing from error responses.
Standardized auth method return values
→ All public auth methods now return a predictable
{ success: boolean }result instead of relying on thrownexceptions for control flow.
Preserved existing UI behavior and API contracts
→ No changes to backend APIs, UI logic, routing, or response formats.
Only defensive handling was added.
Scope:
Result:
Summary by CodeRabbit
Refactor
Bug Fixes
New Features
✏️ Tip: You can customize this high-level summary in your review settings.