Skip to content

fix: improve error handling #1100

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
saurabhhhcodes wants to merge 1 commit into from
+3 −3
Closed

fix: improve error handling #1100

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/app/(routes)/dashboard/analytics/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default function AnalyticsDashboard() {
try {
const end = new Date();
const start = new Date();
start.setDate(end.getDate() - parseInt(dateRange));
start.setDate(end.getDate() - parseInt(dateRange, 10));

@codeant-ai codeant-ai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The client now sends the selected date range, but the analytics API switches to demo data whenever fewer than three doubts exist and always generates only seven trend points. Selecting 30 or 90 days therefore displays and exports preview trends covering seven days while labeling them as the selected range, producing misleading analytics. The demo trend generation must use the requested range or the UI must clearly indicate that the preview is fixed to seven days. [api mismatch]

Severity Level: Major ⚠️ 
- ❌ Teacher analytics trends misrepresent selected date ranges.
- ⚠️ CSV exports label seven-day previews as longer periods.
- ⚠️ New classrooms commonly enter simulated preview mode.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/app/(routes)/dashboard/analytics/page.tsx
**Line:** 36:36
**Comment:**
	*Api Mismatch: The client now sends the selected date range, but the analytics API switches to demo data whenever fewer than three doubts exist and always generates only seven trend points. Selecting 30 or 90 days therefore displays and exports preview trends covering seven days while labeling them as the selected range, producing misleading analytics. The demo trend generation must use the requested range or the UI must clearly indicate that the preview is fixed to seven days.

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
👍 | 👎


let url = `/api/teacher/analytics?startDate=${start.toISOString()}&endDate=${end.toISOString()}`;
if (selectedClassroom !== "all") {
Expand Down
4 changes: 2 additions & 2 deletions src/app/api/doubts/action/[id]/route.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export async function PATCH(req: Request, { params }: { params: Promise<{ id: st
const email = user?.primaryEmailAddress?.emailAddress;

const { id } = await params;
const doubtId = parseInt(id);
const doubtId = parseInt(id, 10);

@codeant-ai codeant-ai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: Using parseInt accepts partially numeric route parameters such as 123abc and 123.4, silently converting them to doubt ID 123. Because this handler performs mutations after the lookup, a malformed URL can update or like the wrong doubt instead of returning a client error. Validate that the entire parameter is a canonical positive integer before parsing. [api mismatch]

Severity Level: Critical 🚨 
- ❌ Crafted URLs can mutate an unintended doubt record.
- ⚠️ PATCH actions accept malformed identifiers.
- ⚠️ Client errors become incorrect mutations or lookups.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/app/api/doubts/action/[id]/route.ts
**Line:** 25:25
**Comment:**
	*Api Mismatch: Using `parseInt` accepts partially numeric route parameters such as `123abc` and `123.4`, silently converting them to doubt ID `123`. Because this handler performs mutations after the lookup, a malformed URL can update or like the wrong doubt instead of returning a client error. Validate that the entire parameter is a canonical positive integer before parsing.

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
👍 | 👎


if (isNaN(doubtId)) {
return NextResponse.json({ error: "Invalid doubt ID" }, { status: 400 });
Expand Down Expand Up @@ -320,7 +320,7 @@ export async function DELETE(req: Request, { params }: { params: Promise<{ id: s
}

const { id } = await params;
const doubtId = parseInt(id);
const doubtId = parseInt(id, 10);

@codeant-ai codeant-ai Bot Jul 30, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggestion: The DELETE handler parses the route ID but never checks for NaN. A request such as /api/doubts/action/abc passes NaN into the integer database predicate, which causes the database query to fail and the catch block to return 500 instead of the expected 400 invalid-ID response. Add the same explicit validity check used by PATCH before querying. [error handling]

Severity Level: Major ⚠️ 
- ❌ Malformed delete requests return internal server errors.
- ⚠️ Invalid client input is misclassified as server failure.
- ⚠️ Database errors add avoidable error logging.

Fix in Cursor Fix in VSCode Claude

(Use Cmd/Ctrl + Click for best experience)

Prompt for AI Agent 🤖 
This is a comment left during a code review.

**Path:** src/app/api/doubts/action/[id]/route.ts
**Line:** 323:323
**Comment:**
	*Error Handling: The DELETE handler parses the route ID but never checks for `NaN`. A request such as `/api/doubts/action/abc` passes `NaN` into the integer database predicate, which causes the database query to fail and the catch block to return 500 instead of the expected 400 invalid-ID response. Add the same explicit validity check used by PATCH before querying.

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
👍 | 👎


const [doubt] = await db.select().from(doubtsTable).where(and(eq(doubtsTable.id, doubtId), isNull(doubtsTable.deletedAt))).limit(1);
if (!doubt) return NextResponse.json({ error: "Doubt not found" }, { status: 404 });
Expand Down
Loading