fix: ensure auth endpoints always return valid JSON responses #268
Conversation
- Fix /signup endpoint to return JSON on validation errors - Fix /login endpoint to return JSON on password mismatch - Add consistent error response format - Resolves AOSSIE-Org#244
- Fix /signup endpoint to return JSON on validation errors - Fix /login endpoint to return JSON on password mismatch - Add consistent error response format - Resolves AOSSIE-Org#244
📝 WalkthroughWalkthroughThis PR addresses authentication robustness by introducing email normalization in the backend, reordering password verification before status checks, implementing safe JSON parsing to prevent frontend crashes on invalid API responses, and adding new auth context methods (verifyEmail, forgotPassword, confirmForgotPassword, googleLogin). Minor UI and routing adjustments accompany the core changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Frontend<br/>(Browser)
participant Server as Backend<br/>(API)
participant Storage as LocalStorage<br/>(Cache)
rect rgba(100, 150, 200, 0.5)
Note over Client,Storage: Traditional Flow (Unsafe)
Client->>Server: POST /auth/login
Server-->>Client: Response (may be empty or non-JSON)
Client->>Client: ❌ response.json() crashes<br/>on invalid/empty body
end
rect rgba(100, 200, 100, 0.5)
Note over Client,Storage: New Flow (Safe)
Client->>Server: POST /auth/login
Server-->>Client: Response
Client->>Client: ✅ safeJson() safely parses<br/>or returns fallback
alt Success Path
Client->>Client: normalizeUser() maps to User type
Client->>Storage: Store token & user
Client->>Client: Update authContext state
else Error Path
Client->>Client: Extract error message safely
Client->>Client: handleError(unknown) throws
Client->>Storage: Clear cached data on failure
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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: 4
🤖 Fix all issues with AI agents
In `@frontend/src/context/authContext.tsx`:
- Around line 204-208: The code calls safeJson(res) and then uses
data.accessToken without verifying data is non-null; update the authentication
flow in the function containing safeJson, setToken, and localStorage.setItem to
check that data is not null and that data.accessToken exists after awaiting
safeJson(res); if data is null or lacks accessToken, throw a descriptive Error
(e.g., "Invalid JSON response" or "Missing accessToken") instead of proceeding,
and only call setToken(data.accessToken) and localStorage.setItem('token',
data.accessToken) when the checks pass.
- Around line 274-278: In googleLogin, safeJson(res) can return null so using
data.accessToken will throw; add a null check after const data = await
safeJson(res) and before setToken/localStorage.setItem to throw or handle an
error when data is null (similar to verifyEmail), e.g., if (!data) throw new
Error('Invalid response from server') or include data?.message in the error;
ensure setToken and localStorage.setItem('token', data.accessToken) only run
when data is non-null and contains accessToken.
- Around line 139-151: The response body is being consumed twice: safeJson(res)
already reads the body into data, then the code calls res.text() again and
incorrectly shadows data; fix by removing the second body read and use the
parsed value returned by safeJson instead—inside the block that checks res.ok,
inspect the existing data variable (from safeJson) for message or error fields
and build the fallback message ('Invalid email or password') from that, and
remove the commented-out line that redeclares/shadows data so you don't re-read
or shadow the response payload.
- Around line 87-92: The handleError function signature was changed to return
never and re-throws errors, which breaks callers like SignUpForm.handleSubmit
that expect handleError to set error state via setError and allow normal flow;
revert handleError to return void and stop re-throwing so it only normalizes the
message and calls setError, and ensure any places that need to propagate an
actual Error (inside try/catch blocks) explicitly throw an Error object instead
of a string; update the function referenced as handleError in authContext.tsx
and adjust callers (e.g., SignUpForm.handleSubmit) to only rely on setError for
validation messages while preserving existing try/catch throw behavior where
appropriate.
🧹 Nitpick comments (3)
frontend/src/context/authContext.tsx (2)
52-73: Default rating value inconsistent with backend.The backend initializes users with
rating: 1200.0(line 83 in auth.go), butnormalizeUserdefaults to1500. Consider aligning these values.Suggested fix
const normalizeUser = (data: any, fallbackEmail?: string): User => ({ id: data?.id || data?._id, email: data?.email || fallbackEmail || '', displayName: data?.displayName || 'User', bio: data?.bio || '', - rating: data?.rating || 1500, + rating: data?.rating ?? 1200, rd: data?.rd || 350, volatility: data?.volatility || 0.06,
119-122: Missing setUser dependency in useCallback.
verifyTokenreferencessetUser(via the closure) but it's not listed in the dependency array. WhilesetUserfromuseSetAtomis stable, adding it explicitly is the idiomatic pattern and avoids lint warnings.Suggested fix
- }, []); + }, [setUser]);frontend/src/Pages/Authentication/forms.tsx (1)
62-66: Inconsistent button width between LoginForm and SignUpForm.The
width: '100%'property was removed from the Google Sign-In button inLoginForm, butSignUpForm(lines 174-179) still includes it. This may cause visual inconsistency between the login and signup pages.Was this intentional, or should both forms use the same styling?
| const handleError = (err: unknown): never => { | ||
| const message = | ||
| error instanceof Error ? error.message : 'An unexpected error occurred'; | ||
| err instanceof Error ? err.message : 'Something went wrong'; | ||
| setError(message); | ||
| throw error; | ||
| throw err; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find all usages of handleError
rg -n "handleError" --type=ts --type=tsx -A2 -B2Repository: AOSSIE-Org/DebateAI
Length of output: 90
🏁 Script executed:
# Also examine the authContext.tsx file to see the full context
fd "authContext" --type=tsxRepository: AOSSIE-Org/DebateAI
Length of output: 233
🏁 Script executed:
# Let's also check the forms.tsx file mentioned in the review
fd "forms" --type=tsxRepository: AOSSIE-Org/DebateAI
Length of output: 233
🏁 Script executed:
# Find the files mentioned in the review
fd -e tsx -e ts | grep -E "(authContext|forms)"Repository: AOSSIE-Org/DebateAI
Length of output: 143
🏁 Script executed:
# Search for handleError usages with correct syntax
rg "handleError" -A2 -B2Repository: AOSSIE-Org/DebateAI
Length of output: 3448
🏁 Script executed:
# Read authContext.tsx to see the full context around handleError
cat -n frontend/src/context/authContext.tsx | sed -n '80,100p'Repository: AOSSIE-Org/DebateAI
Length of output: 772
🏁 Script executed:
# Read forms.tsx to see the handleError usages
cat -n frontend/src/Pages/Authentication/forms.tsx | head -200Repository: AOSSIE-Org/DebateAI
Length of output: 7143
🏁 Script executed:
# Count total handleError usages to ensure we found them all
rg "handleError" --count-matchesRepository: AOSSIE-Org/DebateAI
Length of output: 147
🏁 Script executed:
# Check the second handleError usage in forms.tsx
sed -n '140,250p' frontend/src/Pages/Authentication/forms.tsx | grep -n "handleError" -A3 -B3Repository: AOSSIE-Org/DebateAI
Length of output: 219
handleError signature change breaks validation error handling in SignUpForm.
The return type changed from void to never, and the function now re-throws errors. In SignUpForm.handleSubmit (line 144 in forms.tsx), the validation error 'Passwords do not match' is passed to handleError, which will now throw instead of setting the error state and allowing normal control flow. Since handleSubmit is async, this creates an unhandled promise rejection. Additionally, throwing a string violates error handling best practices — only Error objects should be thrown.
The calls within authContext.tsx itself (inside try-catch blocks) are safe, but the public API change breaks existing form validation patterns.
🤖 Prompt for AI Agents
In `@frontend/src/context/authContext.tsx` around lines 87 - 92, The handleError
function signature was changed to return never and re-throws errors, which
breaks callers like SignUpForm.handleSubmit that expect handleError to set error
state via setError and allow normal flow; revert handleError to return void and
stop re-throwing so it only normalizes the message and calls setError, and
ensure any places that need to propagate an actual Error (inside try/catch
blocks) explicitly throw an Error object instead of a string; update the
function referenced as handleError in authContext.tsx and adjust callers (e.g.,
SignUpForm.handleSubmit) to only rely on setError for validation messages while
preserving existing try/catch throw behavior where appropriate.
| const data = await safeJson(res); | ||
| // if (!res.ok) throw new Error(data?.message || 'Login failed'); | ||
|
|
||
| if (!res.ok) { | ||
| let message = 'Invalid email or password'; | ||
|
|
||
| try { | ||
| const data = JSON.parse(await res.text()); | ||
| message = data.message || data.error || message; | ||
| } catch {} | ||
|
|
||
| throw new Error(message); | ||
| } |
There was a problem hiding this comment.
Bug: Response body consumed twice causes failure.
On line 139, safeJson(res) reads and consumes the response body. On line 146, res.text() attempts to read it again, but the body stream has already been consumed. This will either throw an error or return an empty string.
Additionally, line 140 has a commented-out line that shadows the data variable declared on line 139.
Proposed fix
- const data = await safeJson(res);
- // if (!res.ok) throw new Error(data?.message || 'Login failed');
-
if (!res.ok) {
- let message = 'Invalid email or password';
-
- try {
- const data = JSON.parse(await res.text());
- message = data.message || data.error || message;
- } catch {}
-
- throw new Error(message);
+ const data = await safeJson(res);
+ const message = data?.message || data?.error || 'Invalid email or password';
+ throw new Error(message);
}
-
+
+ const data = await safeJson(res);
+ if (!data) throw new Error('Invalid response from server');
+
setToken(data.accessToken);📝 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.
| const data = await safeJson(res); | |
| // if (!res.ok) throw new Error(data?.message || 'Login failed'); | |
| if (!res.ok) { | |
| let message = 'Invalid email or password'; | |
| try { | |
| const data = JSON.parse(await res.text()); | |
| message = data.message || data.error || message; | |
| } catch {} | |
| throw new Error(message); | |
| } | |
| if (!res.ok) { | |
| const data = await safeJson(res); | |
| const message = data?.message || data?.error || 'Invalid email or password'; | |
| throw new Error(message); | |
| } | |
| const data = await safeJson(res); | |
| if (!data) throw new Error('Invalid response from server'); | |
| setToken(data.accessToken); |
🤖 Prompt for AI Agents
In `@frontend/src/context/authContext.tsx` around lines 139 - 151, The response
body is being consumed twice: safeJson(res) already reads the body into data,
then the code calls res.text() again and incorrectly shadows data; fix by
removing the second body read and use the parsed value returned by safeJson
instead—inside the block that checks res.ok, inspect the existing data variable
(from safeJson) for message or error fields and build the fallback message
('Invalid email or password') from that, and remove the commented-out line that
redeclares/shadows data so you don't re-read or shadow the response payload.
| const data = await safeJson(res); | ||
| if (!res.ok) throw new Error(data?.error || 'Verification failed'); | ||
|
|
||
| const data = await response.json(); | ||
|
|
||
| // User is now verified and logged in | ||
| if (data.accessToken) { | ||
| setToken(data.accessToken); | ||
| localStorage.setItem('token', data.accessToken); | ||
|
|
||
| // Set user details | ||
| 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 || 1200, | ||
| rd: data.user?.rd || 350, | ||
| volatility: data.user?.volatility || 0.06, | ||
| 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: '', | ||
| nickname: data.user?.nickname || 'User', | ||
| isVerified: true, | ||
| verificationCode: undefined, | ||
| 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)); | ||
| navigate('/'); | ||
| } | ||
| } catch (error) { | ||
| handleError(error); | ||
| setToken(data.accessToken); | ||
| localStorage.setItem('token', data.accessToken); |
There was a problem hiding this comment.
Missing null check after safeJson.
safeJson can return null if the response isn't JSON. If data is null, accessing data.accessToken on line 207 will throw a TypeError.
Proposed fix
const data = await safeJson(res);
if (!res.ok) throw new Error(data?.error || 'Verification failed');
+ if (!data) throw new Error('Invalid response from server');
setToken(data.accessToken);📝 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.
| const data = await safeJson(res); | |
| if (!res.ok) throw new Error(data?.error || 'Verification failed'); | |
| const data = await response.json(); | |
| // User is now verified and logged in | |
| if (data.accessToken) { | |
| setToken(data.accessToken); | |
| localStorage.setItem('token', data.accessToken); | |
| // Set user details | |
| 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 || 1200, | |
| rd: data.user?.rd || 350, | |
| volatility: data.user?.volatility || 0.06, | |
| 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: '', | |
| nickname: data.user?.nickname || 'User', | |
| isVerified: true, | |
| verificationCode: undefined, | |
| 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)); | |
| navigate('/'); | |
| } | |
| } catch (error) { | |
| handleError(error); | |
| setToken(data.accessToken); | |
| localStorage.setItem('token', data.accessToken); | |
| const data = await safeJson(res); | |
| if (!res.ok) throw new Error(data?.error || 'Verification failed'); | |
| if (!data) throw new Error('Invalid response from server'); | |
| setToken(data.accessToken); | |
| localStorage.setItem('token', data.accessToken); |
🤖 Prompt for AI Agents
In `@frontend/src/context/authContext.tsx` around lines 204 - 208, The code calls
safeJson(res) and then uses data.accessToken without verifying data is non-null;
update the authentication flow in the function containing safeJson, setToken,
and localStorage.setItem to check that data is not null and that
data.accessToken exists after awaiting safeJson(res); if data is null or lacks
accessToken, throw a descriptive Error (e.g., "Invalid JSON response" or
"Missing accessToken") instead of proceeding, and only call
setToken(data.accessToken) and localStorage.setItem('token', data.accessToken)
when the checks pass.
| const data = await safeJson(res); | ||
| if (!res.ok) throw new Error(data?.message || 'Google login failed'); | ||
|
|
||
| setToken(data.accessToken); | ||
| localStorage.setItem('token', data.accessToken); |
There was a problem hiding this comment.
Missing null check after safeJson in googleLogin.
Same issue as verifyEmail — if safeJson returns null, accessing data.accessToken will throw.
Proposed fix
const data = await safeJson(res);
if (!res.ok) throw new Error(data?.message || 'Google login failed');
+ if (!data) throw new Error('Invalid response from server');
setToken(data.accessToken);📝 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.
| const data = await safeJson(res); | |
| if (!res.ok) throw new Error(data?.message || 'Google login failed'); | |
| setToken(data.accessToken); | |
| localStorage.setItem('token', data.accessToken); | |
| const data = await safeJson(res); | |
| if (!res.ok) throw new Error(data?.message || 'Google login failed'); | |
| if (!data) throw new Error('Invalid response from server'); | |
| setToken(data.accessToken); | |
| localStorage.setItem('token', data.accessToken); |
🤖 Prompt for AI Agents
In `@frontend/src/context/authContext.tsx` around lines 274 - 278, In googleLogin,
safeJson(res) can return null so using data.accessToken will throw; add a null
check after const data = await safeJson(res) and before
setToken/localStorage.setItem to throw or handle an error when data is null
(similar to verifyEmail), e.g., if (!data) throw new Error('Invalid response
from server') or include data?.message in the error; ensure setToken and
localStorage.setItem('token', data.accessToken) only run when data is non-null
and contains accessToken.
1 participant
Description
Fixes #244 - Auth endpoints sometimes return empty response body
Changes Made:
/signupendpoint to always return JSON on error/loginendpoint to always return JSON on errorRoot Cause:
Certain error paths (validation failure, user exists, password mismatch) were exiting without calling
c.JSON(), resulting in empty responses.Testing:
{error: "user already exists"}{error: "invalid credentials"}Impact:
Summary by CodeRabbit
New Features
Bug Fixes
Style
✏️ Tip: You can customize this high-level summary in your review settings.