fix signuploginerrorhandling #34 issue part-1#44
Conversation
📝 WalkthroughWalkthroughAuthContext removes try/catch blocks from ChangesAuth Error Handling Refactor
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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.
🧹 Nitpick comments (1)
frontend/src/pages/Login.jsx (1)
40-53: ⚡ Quick winExtract shared auth error parsing/mapping to a helper.
err.responseparsing and status-to-message branching now exists here and infrontend/src/pages/Signup.jsx(Line 53-64). Centralizing this logic will prevent message/status drift as backend error contracts evolve.♻️ Suggested direction
+// frontend/src/lib/authErrorMessage.js +export function getAuthErrorMessage(err, mode = "login") { + const statusCode = err.response?.status; + const errorMsg = err.response?.data?.message || err.response?.data?.error; + + if (mode === "login") { + if (statusCode === 401) return errorMsg || "Invalid email or password. Please try again."; + if (statusCode === 404) return "Account not found. Please sign up first."; + if (statusCode === 500) return "Server error. Please try again later."; + return errorMsg || "Login failed. Please check your credentials."; + } + + if (mode === "signup") { + if (statusCode === 400) return errorMsg || "Invalid input. Please check your details."; + if (statusCode === 409) return "User already exists. Please login instead."; + if (statusCode === 500) return "Server error. Please try again later."; + return errorMsg || "Registration failed. Please try again."; + } +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/pages/Login.jsx` around lines 40 - 53, Extract the status+body parsing into a shared helper (e.g., parseAuthErrorResponse) that accepts the caught error object and returns a finalized user-facing message; implement it to read err.response?.status and err.response?.data?.message || err.response?.data?.error and map 401/404/500 to the same default strings shown here and fall back to the raw errorMsg or a generic login failure text. Export this helper from a new util (e.g., frontend/src/utils/authErrors.js) and replace the inline logic in Login.jsx (the block using statusCode/errorMsg and setError) and the same block in Signup.jsx to simply call setError(parseAuthErrorResponse(err)). Ensure imports are added and tests/behavior remain identical to the current mapping.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/src/pages/Login.jsx`:
- Around line 40-53: Extract the status+body parsing into a shared helper (e.g.,
parseAuthErrorResponse) that accepts the caught error object and returns a
finalized user-facing message; implement it to read err.response?.status and
err.response?.data?.message || err.response?.data?.error and map 401/404/500 to
the same default strings shown here and fall back to the raw errorMsg or a
generic login failure text. Export this helper from a new util (e.g.,
frontend/src/utils/authErrors.js) and replace the inline logic in Login.jsx (the
block using statusCode/errorMsg and setError) and the same block in Signup.jsx
to simply call setError(parseAuthErrorResponse(err)). Ensure imports are added
and tests/behavior remain identical to the current mapping.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2984ee35-f944-4dc4-bb9f-3413382513c9
⛔ Files ignored due to path filters (1)
frontend/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
frontend/src/context/AuthContext.jsxfrontend/src/pages/Login.jsxfrontend/src/pages/Signup.jsx
|
Hey @arpitasi1gh, great work on this! Getting these error messages to properly display on the frontend is a massive UX improvement. The logic looks solid, but before we merge, let's take a look at the nitpick comment left by the coderabbitai bot. Since the error-parsing logic is nearly identical in both Login.jsx and Signup.jsx, it's a great best practice to extract that into a shared helper function (like getAuthErrorMessage). This keeps our codebase DRY (Don't Repeat Yourself) and makes future changes much easier! Could you implement that quick refactor? Once that's done, I'll get this merged right away. |
2 participants
Related Issue
Addresses part of #34
Problem
Users were not seeing actual error messages from the backend during signup/login failures. The AuthContext was catching all errors internally and only returning true/false, causing pages to never receive the exceptions they were trying to catch.
Solution
AuthContext.register()andAuthContext.login()to throw errors instead of catching them internallySignup.jsxandLogin.jsxerror handling to properly catch and display backend error messagesmessageanderrorfields from API responsesResult
Users now see helpful, specific error messages:
Summary by CodeRabbit
Release Notes