Skip to content

fix: prevent MongoDB query crashes by enforcing strict pagination val… - #42

Merged
Parvaggarwal01 merged 1 commit into
Parvaggarwal01:mainfrom
vansh2604-star:#41-issue-resolve
Jun 29, 2026
Merged

fix: prevent MongoDB query crashes by enforcing strict pagination val…#42
Parvaggarwal01 merged 1 commit into
Parvaggarwal01:mainfrom
vansh2604-star:#41-issue-resolve

Conversation

@vansh2604-star

@vansh2604-star vansh2604-star commented Jun 29, 2026

Copy link
Copy Markdown

…idation

What Changed

-What Changed Replaced the .transform((val) => parseInt(val)) logic in src/validations/barter.validation.js with Zod's strict native coercion: z.coerce.number().int().positive().

Why

-To prevent a 500 Internal Server Error crash when invalid (non-numeric) query parameters (like ?page=abc) are passed to the barter list endpoints. The previous logic allowed NaN to propagate into the MongoDB service layer, causing a database skip calculation error. The new logic properly catches it at the validation layer.

How To Test

-Run the backend server locally using npm run dev.
Send a GET request to /api/barters?page=abc&limit=xyz.
Verify that instead of crashing the server, it returns a 400 Bad Request with a "Expected number, received nan" validation error.

Screenshots

Add screenshots or screen recordings for UI changes.

Related Issue

Closes #

Checklist

  • I have read CONTRIBUTING.md.
  • I kept this pull request focused on one issue.
  • I ran the relevant checks locally.
  • I added or updated tests where appropriate.
  • I added screenshots or screen recordings for UI changes.
  • I documented any known limitations or follow-up work.

Notes

Used AI assistance to help identify the optimal Zod validation syntax and ensure the local testing environment was correctly configured.

Summary by CodeRabbit

  • Bug Fixes
    • Improved pagination handling for barter listings, so page and limit values are validated more reliably.
    • Added safer default pagination values, helping requests without these parameters behave consistently.
    • Numeric pagination inputs are now accepted more flexibly, reducing errors from malformed query values.

@vercel

vercel Bot commented Jun 29, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Parv Aggarwal's projects Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai

coderabbitai Bot commented Jun 29, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

In getBartersQuerySchema, the page and limit query fields are changed from z.string().optional().transform(parseInt(...)) to z.coerce.number().int().positive().optional().default(1/.default(10)), and a closing brace is repositioned accordingly.

Changes

Pagination Validation Refactor

Layer / File(s) Summary
page/limit coercion and defaults
barterly-backend/src/validations/barter.validation.js
page and limit fields switch from z.string().transform(parseInt) to z.coerce.number().int().positive() with .default(1) and .default(10); schema closing brace adjusted.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Possibly related issues

  • Barterly#41: Describes the exact same fix — replacing parseInt-based transforms with z.coerce.number().int().positive() for pagination validation in barter.validation.js.

Poem

🐇 No more strings parsed by hand,
Coerce the numbers, isn't that grand!
Page one by default, limit set to ten,
The barters will paginate cleanly again.
Hop hop, the schema's neat! 🌿

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title matches the main change: stricter pagination validation for barter queries to avoid query issues.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@barterly-backend/src/validations/barter.validation.js`:
- Around line 74-75: Persist the result of schema parsing before the controller
runs, since validate.middleware.js currently ignores schema.parseAsync(...)’s
returned data and getMyBarters still sees raw req.query values instead of the
parsed defaults from barter.validation.js. Update the validate.middleware.js
flow so the parsed query object is assigned back onto req or passed through to
the next handler, and make sure getMyBarters reads the validated query data
rather than req.query directly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 228eaaf5-f28f-42f0-9670-d463a4c0e03e

📥 Commits

Reviewing files that changed from the base of the PR and between d70fc43 and 6dfe205.

📒 Files selected for processing (1)
  • barterly-backend/src/validations/barter.validation.js

Comment on lines +74 to +75
page: z.coerce.number().int().positive().optional().default(1),
limit: z.coerce.number().int().positive().optional().default(10),

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== validate middleware =="
sed -n '1,80p' barterly-backend/src/middlewares/validate.middleware.js

echo
echo "== barter controller consumer =="
sed -n '1,90p' barterly-backend/src/controllers/barter.controller.js

echo
echo "== other direct req.query.page/limit consumers =="
rg -n -C2 '\breq\.query\.(page|limit)\b' barterly-backend/src

Repository: Parvaggarwal01/Barterly

Length of output: 6749


Persist the parsed query before calling the controller. validate.middleware.js drops schema.parseAsync(...)’s return value, so getMyBarters still reads raw req.query.page / req.query.limit strings and never gets these schema defaults. Assign the parsed query back to req or consume the parsed object directly.

🤖 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 `@barterly-backend/src/validations/barter.validation.js` around lines 74 - 75,
Persist the result of schema parsing before the controller runs, since
validate.middleware.js currently ignores schema.parseAsync(...)’s returned data
and getMyBarters still sees raw req.query values instead of the parsed defaults
from barter.validation.js. Update the validate.middleware.js flow so the parsed
query object is assigned back onto req or passed through to the next handler,
and make sure getMyBarters reads the validated query data rather than req.query
directly.

@Parvaggarwal01
Parvaggarwal01 merged commit d2c111a into Parvaggarwal01:main Jun 29, 2026
4 of 5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants