fix: improve JSON parsing and error handling#695
Conversation
|
@anshiikaa001 is attempting to deploy a commit to the Karan Mani Tripathi 's projects Team on Vercel. A member of the Team first needs to authorize it. |
Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
github-actions
Bot
added
gssoc'26
WalkthroughThree files replace unsafe ChangesFetch Error Handling Hardening
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 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)
⚔️ Resolve merge conflicts
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 |
github-actions
Bot
added
merge-conflict
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hello there! 🎉 Thank you so much for your first pull request to DoubtDesk!
We really appreciate your contribution. A maintainer will review your code soon. If you are participating in GSSoC, ensure your PR is linked to an open issue. Please make sure you have followed all rules in our Contributing Guidelines. Happy coding!
There was a problem hiding this comment.
⭐ Star Required
Hi @anshiikaa001! Thank you for your contribution to DoubtDesk.
Before we can complete the review and merge this pull request, please star the DoubtDesk repository.
Once you have starred the repository, please drop a comment here saying "done" and we will proceed with reviewing your PR. Thank you!
codeant-ai
Bot
added
the
size:L
|
@coderabbitai review |
github-actions
Bot
added
size/l
quality : needs-work
and removed
size:L
![codeant-ai[bot]](https://avatars.githubusercontent.com/in/646884?s=60&v=4){kind=small}
| if (!res.ok) { | ||
| console.error(`Failed to fetch replies: ${res.status}`); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Suggestion: On non-OK fetch responses you return early without clearing previous replies, so the modal can continue showing stale data from an earlier doubt and allow actions on outdated UI state. Reset the replies state on this failure path before returning. [logic error]
Severity Level: Major ⚠️
- ⚠️ DoubtRepliesModal can show stale replies after fetch failures.
- ⚠️ Users see outdated discussion instead of reflected load error.Steps of Reproduction ✅
1. From any classroom view that renders `DoubtCard`, open a replies modal by clicking the
replies entrypoint on a doubt; this instantiates `DoubtRepliesModal` via
`components/DoubtCard.tsx:12-18` with props `{ doubt, isOpen, onClose, onReplyChange,
isTeacher }`.
2. On first open, `useEffect` in `components/DoubtRepliesModal.tsx:59-66` calls
`fetchReplies()`, which issues `fetch(url)` and, when `res.ok`, sets `replies` from `await
res.json()` (`components/DoubtRepliesModal.tsx:82-104`), populating the UI list in the
messages area (`components/DoubtRepliesModal.tsx:295-370`).
3. Close the modal (toggling `isOpen` to false) and later reopen it for the same doubt so
that `replies` still holds the previously fetched list; `isOpen` toggling back to true
triggers `fetchReplies()` again via the same effect at
`components/DoubtRepliesModal.tsx:59-66`.
4. If this second fetch now returns a non-OK response (for example, a 403 classroom access
error or 500 internal error from `app/api/replies/route.ts:36-84`), the code at
`components/DoubtRepliesModal.tsx:88-91` logs `Failed to fetch replies` and returns early
without touching `replies`, so the UI continues to display and allow interactions on the
stale `replies` list from the earlier successful load instead of reflecting that the
latest load failed.
(Use Cmd/Ctrl + Click for best experience)
Prompt for AI Agent 🤖
This is a comment left during a code review.
**Path:** components/DoubtRepliesModal.tsx
**Line:** 88:91
**Comment:**
*Logic Error: On non-OK fetch responses you return early without clearing previous replies, so the modal can continue showing stale data from an earlier doubt and allow actions on outdated UI state. Reset the replies state on this failure path before returning.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (5)
components/AskAIView.tsx (2)
99-115: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract hardcoded error messages to constants.
The error messages
"Could not retrieve the solution."(line 101) and"Invalid server response."(line 111) are hardcoded strings. Per path instructions, user-facing strings should be extracted to constants for maintainability and potential internationalization.Additionally, the same error message appears at line 124, making them candidates for a shared constant.
♻️ Suggested refactor
+const ERROR_MESSAGES = { + SOLUTION_FETCH_FAILED: "Could not retrieve the solution.", + INVALID_SERVER_RESPONSE: "Invalid server response.", + NO_SOLUTION_FOUND: "No solution found for this query.", + CONNECTION_ERROR: "Connection error while fetching solution.", +} as const; + export default function AskAIView({ classroomId = null, onSuccess, initialDoubt }: {Then use these constants in the error handling:
if (!res.ok) { - setErrorMsg("Could not retrieve the solution."); + setErrorMsg(ERROR_MESSAGES.SOLUTION_FETCH_FAILED); return; }🤖 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 `@components/AskAIView.tsx` around lines 99 - 115, Extract the hardcoded error messages used in setErrorMsg calls to named constants at the top of the AskAIView component. Create constants for "Could not retrieve the solution." and "Invalid server response." that are referenced in the error handling blocks (where res.ok is checked and where res.json() is parsed), and replace the hardcoded strings with these constants throughout the component to improve maintainability and support future internationalization.Source: Path instructions
227-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueVerify dual error display intent.
Lines 228-229 both display error messages to the user -
setErrorMsgupdates component state (likely rendered in the UI) andtoast.errorshows a toast notification. This results in two different error messages for the same failure:
- Component:
err.message || "Something went wrong. Please try again."- Toast:
err.message || "Failed to process AI request."Consider whether both displays are necessary, and if so, ensure the messages are consistent.
🤖 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 `@components/AskAIView.tsx` around lines 227 - 229, The catch block in the error handling section contains two separate error display mechanisms - setErrorMsg and toast.error - that show different fallback messages to the user for the same error. Decide whether both error displays are necessary for your user experience, and if both are required, ensure they display the same consistent error message. If only one is needed, remove the redundant call between setErrorMsg and toast.error to avoid confusing users with duplicate or conflicting error notifications.components/DoubtRepliesModal.tsx (3)
829-833: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd aria-label attributes to markdown toolbar buttons.
The markdown formatting buttons (Bold, Italic, Code, List) display only icons without text labels or aria-label attributes. Screen reader users won't be able to identify the purpose of these buttons. Per path instructions for .tsx files, interactive elements need proper accessibility labels.
♻️ Suggested fix
-<button onClick={() => insertMarkdown(solutionTextareaRef, "bold", setSolutionContent)} className="p-2 hover:bg-slate-200 dark:hover:bg-white/10 rounded-xl text-slate-600 dark:text-slate-400"><Bold className="w-4 h-4" /></button> +<button onClick={() => insertMarkdown(solutionTextareaRef, "bold", setSolutionContent)} className="p-2 hover:bg-slate-200 dark:hover:bg-white/10 rounded-xl text-slate-600 dark:text-slate-400" aria-label="Bold text"><Bold className="w-4 h-4" /></button> -<button onClick={() => insertMarkdown(solutionTextareaRef, "italic", setSolutionContent)} className="p-2 hover:bg-slate-200 dark:hover:bg-white/10 rounded-xl text-slate-600 dark:text-slate-400"><Italic className="w-4 h-4" /></button> +<button onClick={() => insertMarkdown(solutionTextareaRef, "italic", setSolutionContent)} className="p-2 hover:bg-slate-200 dark:hover:bg-white/10 rounded-xl text-slate-600 dark:text-slate-400" aria-label="Italic text"><Italic className="w-4 h-4" /></button> -<button onClick={() => insertMarkdown(solutionTextareaRef, "code", setSolutionContent)} className="p-2 hover:bg-slate-200 dark:hover:bg-white/10 rounded-xl text-slate-600 dark:text-slate-400"><Code className="w-4 h-4" /></button> +<button onClick={() => insertMarkdown(solutionTextareaRef, "code", setSolutionContent)} className="p-2 hover:bg-slate-200 dark:hover:bg-white/10 rounded-xl text-slate-600 dark:text-slate-400" aria-label="Code block"><Code className="w-4 h-4" /></button> -<button onClick={() => insertMarkdown(solutionTextareaRef, "list", setSolutionContent)} className="p-2 hover:bg-slate-200 dark:hover:bg-white/10 rounded-xl text-slate-600 dark:text-slate-400"><List className="w-4 h-4" /></button> +<button onClick={() => insertMarkdown(solutionTextareaRef, "list", setSolutionContent)} className="p-2 hover:bg-slate-200 dark:hover:bg-white/10 rounded-xl text-slate-600 dark:text-slate-400" aria-label="Bullet list"><List className="w-4 h-4" /></button>🤖 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 `@components/DoubtRepliesModal.tsx` around lines 829 - 833, The markdown formatting buttons in the toolbar (Bold, Italic, Code, and List buttons that call insertMarkdown with "bold", "italic", "code", and "list" respectively) lack aria-label attributes, making them inaccessible to screen reader users. Add appropriate aria-label attributes to each button with descriptive labels that match their functionality ("Bold", "Italic", "Code", and "List" respectively) to ensure screen reader users can identify the purpose of each button.Source: Path instructions
82-99: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winConsider surfacing fetch failures to users.
When
fetchRepliesfails (either HTTP error at line 88-91 or JSON parse error at line 93-97), the function logs to console but doesn't show any user-facing error message. The modal will display "No interactions yet" (line 727) regardless of whether replies failed to load or simply don't exist, potentially confusing users when a network issue occurs.Consider adding a toast notification or error state to distinguish between an empty discussion and a failed load attempt.
🤖 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 `@components/DoubtRepliesModal.tsx` around lines 82 - 99, The fetchReplies function logs errors to console when HTTP requests fail or JSON parsing fails, but provides no user-facing feedback. Add an error state variable (similar to the replies state) to track fetch failures, then update this error state in both the HTTP error check (after res.ok check) and the JSON parse catch block. Finally, display a toast notification or error message using this error state so users can distinguish between a network failure and an empty discussion, rather than defaulting to "No interactions yet" in both cases.
547-549: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd aria-label attributes to edit toolbar buttons.
The markdown formatting buttons in the edit mode (Bold, Italic, Code) also lack aria-label attributes. Apply the same accessibility fix as suggested for the solution form toolbar.
🤖 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 `@components/DoubtRepliesModal.tsx` around lines 547 - 549, The three markdown formatting buttons in the edit toolbar (Bold, Italic, and Code) that use the insertMarkdown function are missing aria-label attributes for accessibility. Add aria-label attributes to each of these three buttons with descriptive labels that identify their function, such as "Bold text", "Italic text", and "Code text" respectively. This will ensure screen readers can properly announce the purpose of each button to users with accessibility needs.Source: Path instructions
🤖 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 `@components/AskAIView.tsx`:
- Around line 99-115: Extract the hardcoded error messages used in setErrorMsg
calls to named constants at the top of the AskAIView component. Create constants
for "Could not retrieve the solution." and "Invalid server response." that are
referenced in the error handling blocks (where res.ok is checked and where
res.json() is parsed), and replace the hardcoded strings with these constants
throughout the component to improve maintainability and support future
internationalization.
- Around line 227-229: The catch block in the error handling section contains
two separate error display mechanisms - setErrorMsg and toast.error - that show
different fallback messages to the user for the same error. Decide whether both
error displays are necessary for your user experience, and if both are required,
ensure they display the same consistent error message. If only one is needed,
remove the redundant call between setErrorMsg and toast.error to avoid confusing
users with duplicate or conflicting error notifications.
In `@components/DoubtRepliesModal.tsx`:
- Around line 829-833: The markdown formatting buttons in the toolbar (Bold,
Italic, Code, and List buttons that call insertMarkdown with "bold", "italic",
"code", and "list" respectively) lack aria-label attributes, making them
inaccessible to screen reader users. Add appropriate aria-label attributes to
each button with descriptive labels that match their functionality ("Bold",
"Italic", "Code", and "List" respectively) to ensure screen reader users can
identify the purpose of each button.
- Around line 82-99: The fetchReplies function logs errors to console when HTTP
requests fail or JSON parsing fails, but provides no user-facing feedback. Add
an error state variable (similar to the replies state) to track fetch failures,
then update this error state in both the HTTP error check (after res.ok check)
and the JSON parse catch block. Finally, display a toast notification or error
message using this error state so users can distinguish between a network
failure and an empty discussion, rather than defaulting to "No interactions yet"
in both cases.
- Around line 547-549: The three markdown formatting buttons in the edit toolbar
(Bold, Italic, and Code) that use the insertMarkdown function are missing
aria-label attributes for accessibility. Add aria-label attributes to each of
these three buttons with descriptive labels that identify their function, such
as "Bold text", "Italic text", and "Code text" respectively. This will ensure
screen readers can properly announce the purpose of each button to users with
accessibility needs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5e1450e1-8697-4c44-af7c-4e57f1627779
📒 Files selected for processing (3)
app/provider.tsxcomponents/AskAIView.tsxcomponents/DoubtRepliesModal.tsx
1 participant
User description
This PR addresses issue #398 by improving API response handling and preventing failures caused by malformed JSON responses.
Changes Made
Added proper
try-catchblocks aroundres.json()calls.Validated
res.okbefore processing response data.Replaced unsafe JSON parsing patterns that could silently fail.
Added error logging using
console.error()for easier debugging.Added fallback error handling with user-friendly toast messages.
Improved stability in reply-related workflows including:
Fetching replies
Creating replies
Updating replies
Updating solutions
Voting on replies
Marking solutions
Deleting replies
Impact
Prevents component crashes aused by malformed API responses.
Improves debugging by surfacing parsing failures.
Reduces reply modal instability.
Provides safer and more predictable API handling across affected components.
Closes #398
CodeAnt-AI Description
Handle bad server responses without breaking replies and AI actions
What Changed
Impact
✅ Fewer reply screen crashes✅ Clearer AI and video error messages✅ Safer reply actions after server errors💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Summary by CodeRabbit
Bug Fixes