fix(api): name the offending fields on a validation error - #474
Conversation
The backend normalises every 422 to the bare message "Validation error" and puts the per-field detail in `data` (main.py's RequestValidationError handler). apiCore read only `message`, so that detail was fetched, serialised, sent, and discarded — every form in the app reported two words and named nothing. describeError() folds the field paths into the message: "Validation error — tmdb_id: Field required". Drops FastAPI's request-section prefix (body/query/…), keeps nested paths readable (gdrive_uploads.0.folder_id), caps the list at three and says how many were hidden. Non-validation errors are returned unchanged, so no other message shifts.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe API core now formats validation errors with field paths, removes request prefixes, and limits displayed fields. HTTP error handling uses this formatter before fallback messages. Tests cover validation, nested paths, truncation, malformed data, and empty payloads. ChangesValidation Error Handling
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
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 `@frontend/src/utils/api/core.js`:
- Around line 105-111: Update the detail mapping in validation to ignore entries
whose msg is not a non-empty string, so validation([{}]) uses the base-message
fallback instead of emitting “is invalid.” Replace the current || fallback with
an explicit message check while preserving valid message handling and add a
regression test for validation([{}]).
🪄 Autofix
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec25a878-be32-44ec-b6b3-e98ff9cf8be5
📒 Files selected for processing (2)
frontend/src/utils/api/core.jsfrontend/src/utils/api/core.test.js
`d?.msg || 'is invalid'` turned an empty detail record into "Validation error — is invalid": a field-less phrase that is strictly worse than the base message it replaced. An empty-string msg took the same path. Explicit type check instead of `||`, per the repo rule about falsy-but-valid values. A malformed entry is dropped, so the message degrades to the base one; valid entries alongside it are still named.
|
@coderabbitai review |
✅ Action performedReview finished.
|
Summary
main.py'sRequestValidationErrorhandler normalises every 422 to the bare message"Validation error", with the per-field detail indata.apiCorereads onlymessage, and callers toast that — so the detail is fetched, serialised, sent over the wire and thrown away.The result: submit any form with a missing or malformed field, anywhere in CHUB, and you get two words that name nothing.
Found while looking at why the CL2K asset makers reject a title with no TMDB id — but the fix is app-wide, not specific to that.
Type of change
What it does
describeError()folds the field paths into the message:body/query/path/header)Testing
npm run lint,npx prettier --check(whole tree),npm run build→ exit 0npx vitest run→ 45 passed (7 new)The four behavioural tests were verified red against the previous bare-message behaviour. Also covered: a non-validation payload passing through unchanged,
datathat isn't a list falling back to the base message, and an empty/absent payload returning''so the caller still gets the HTTP status line.Checklist
DAPSreferences introduceddescribeErrorfix(api):commit on mainSummary by CodeRabbit