fix(validation): correct 'on' and 'property' fields in validation errors for constraint violations#1674
Conversation
…ors for constraint violations Fixes elysiajs#1660 ## Problem When query validation fails due to constraint violations (like with value ), the validation error incorrectly returns: - `on: 'property'` instead of `on: 'query'` - `property: 'root'` instead of `property: '/limit'` Type errors (like passing a non-numeric string) worked correctly. ## Root Cause When `t.Numeric` (or other Transform types) decode a value, they validate constraints inside the Decode step. If constraints fail, they throw a `ValidationError` with hardcoded `type: 'property'` from the internal type-system utilities. This error is wrapped in `TransformDecodeError`, which Elysia's `coerceTransformDecodeError` function caught and re-threw directly via `throw error.error`, preserving the incorrect type. ## Solution Modified `coerceTransformDecodeError` to: 1. Extract the `valueError` from the internal `ValidationError` 2. Fix the path using `TransformDecodeError.path` (which has the correct path) 3. Create a new `ValidationError` with the correct type context (e.g., 'query') 4. Preserve existing behavior for non-ValidationError errors (e.g., NotFoundError) ## Testing - Added 2 new test cases for constraint violation error metadata - All 1448 tests pass
WalkthroughAdjusts transform-decode error handling to preserve and correct inner ValidationError paths for value-level constraint violations and adds tests verifying accurate Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✏️ Tip: You can disable this entire section by setting 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 |
commit: |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/compose.ts (1)
452-476: Don’t pre-setc.set.status=422if you might rethrowerror.error(status can become wrong/sticky).Right now, any
TransformDecodeErrorsets 422 (Line 464) even if you then rethrowerror.error(Line 475). Because later error handling often won’t overwrite an already-4xx status, a non-validation error rethrown here (your comment mentionsNotFoundError) can incorrectly end up as a 422 response.Proposed fix (only set 422 when throwing a ValidationError)
const coerceTransformDecodeError = ( fnLiteral: string, type: string, allowUnsafeValidationDetails = false, value = `c.${type}` ) => // NOTE: We only handle TransformDecodeError and intentionally let other errors // fall through silently. This is required because TypeBox's Transform mechanism // throws internal errors during successful decode operations that are not // TransformDecodeError. Re-throwing those would break Transform-based validation. `try{${fnLiteral}}catch(error){` + `if(error.constructor.name === 'TransformDecodeError'){` + - `c.set.status=422\n` + // Fix `#1660`: When error.error is a ValidationError with wrong 'type' field, // extract valueError and fix its path using TransformDecodeError.path - `if(error.error?.valueError){` + + `if(error.error?.valueError){` + + `c.set.status=422\n` + `const ve=error.error.valueError;` + `const fe={...ve,path:error.path};` + `const errs={[Symbol.iterator]:function*(){yield fe},First:()=>fe};` + `throw new ValidationError('${type}',validator.${type},${value},${allowUnsafeValidationDetails},errs)` + `}` + // If error.error exists but is not a ValidationError (e.g., NotFoundError), // re-throw it directly. Otherwise, create new ValidationError. - `throw error.error ?? new ValidationError('${type}',validator.${type},${value},${allowUnsafeValidationDetails})}` + + `if(error.error)throw error.error\n` + + `c.set.status=422\n` + + `throw new ValidationError('${type}',validator.${type},${value},${allowUnsafeValidationDetails})}` + `}`
🤖 Fix all issues with AI agents
In `@src/compose.ts`:
- Around line 465-472: Replace the duck-typed check `if
(error.error?.valueError)` with an explicit ValidationError type check—e.g., `if
(error.error instanceof ValidationError && error.error.valueError)`—so you only
treat true ValidationError instances (referencing the `error`,
`ValidationError`, and `valueError` symbols); keep the custom `errs` object and
its iterator/First contract as-is so `ValidationError` can call `errors.First()`
and revalidate via `validator.Errors()`; ensure you still construct `fe` using
`error.error.valueError` and `path: error.path` and throw the new
`ValidationError('${type}', validator.${type}, ${value},
${allowUnsafeValidationDetails}, errs)` as before.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/compose.ts
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
5b5c817 to
9fa92b5
Compare
Replace duck-typed check `if(error.error?.valueError)` with explicit type check `if(error.error instanceof ValidationError && error.error.valueError)` to ensure only true ValidationError instances are handled.
The instanceof check doesn't work in generated code strings because ValidationError is not in scope at runtime. Using constructor.name matches the existing pattern used for TransformDecodeError check.
Summary
Fixes #1660 - Incorrect query validation error fields
Problem
When query validation fails due to constraint violations (like
maximum: 100with value200), the validation error incorrectly returns:on: "property"instead ofon: "query"property: "root"instead ofproperty: "/limit"Before (broken):
{ "type": "validation", "on": "property", "property": "root", "message": "Expected number to be less or equal to 100" }After (fixed):
{ "type": "validation", "on": "query", "property": "/limit", "message": "Expected number to be less or equal to 100" }Root Cause
When
t.Numeric(or other Transform types) decode a value, they validate constraints inside the Decode step. If constraints fail, they throw aValidationErrorwith hardcodedtype: 'property'from the internal type-system utilities (src/type-system/utils.ts:48).This error is wrapped in
TransformDecodeError, which Elysia'scoerceTransformDecodeErrorfunction caught and re-threw directly viathrow error.error, preserving the incorrect type.The key insight is that
TransformDecodeError.pathhas the correct path (e.g.,/limit), but the innerValidationError.valueError.pathis empty because it was created against the inner schema (the Number), not the full Object schema.Solution
Modified
coerceTransformDecodeErrorinsrc/compose.tsto:error.error?.valueErrorexists (indicating it's a ValidationError with constraint info)valueErrorand fix itspathusingTransformDecodeError.pathValidationErrorwith the correct type context (e.g., 'query') and fixed errorsNotFoundErrorthrown in Transform)Testing
Reproduction
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.