Skip to content

fix: handle standard schema validators in ValidationError.all #1483

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix: handle standard schema validators in ValidationError.all #1483

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
28 changes: 27 additions & 1 deletion src/error.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,33 @@ export class ValidationError extends Error {
Object.setPrototypeOf(this, ValidationError.prototype)
}

get all() {
get all(): MapValueError[] {
// Handle standard schema validators (Zod, Valibot, etc.)
if (
// @ts-ignore
this.validator?.provider === 'standard' ||
'~standard' in this.validator ||
// @ts-ignore
('schema' in this.validator && this.validator.schema && '~standard' in this.validator.schema)
) {
const standard = // @ts-ignore
('~standard' in this.validator
? this.validator
: // @ts-ignore
this.validator.schema)['~standard']

const issues = standard.validate(this.value).issues
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

Re-validation on every access degrades performance.

Line 449 calls standard.validate(this.value) every time the all getter is accessed. This re-validates the entire value on each call, which is expensive. The validation already occurred in the constructor (line 292), and the issues could be cached.

Consider caching the validation issues in the constructor and reusing them in the getter to avoid redundant validation:

 	valueError?: ValueError
 	expected?: unknown
 	customError?: string
+	private cachedIssues?: any[]

Then in the constructor's standard validator branch (after line 292):

 			const _errors = errors ?? standard.validate(value).issues
+			this.cachedIssues = _errors

And in the getter:

-			const issues = standard.validate(this.value).issues
+			const issues = this.cachedIssues ?? standard.validate(this.value).issues
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const issues = standard.validate(this.value).issues
const issues = this.cachedIssues ?? standard.validate(this.value).issues
🤖 Prompt for AI Agents 
In src/error.ts around line 449 (and the constructor around line 292), the
getter currently calls standard.validate(this.value) on every access which
re-runs expensive validation; instead compute and cache the validation result
once in the constructor (when the standard validator branch runs) into a private
field (e.g. this._validationIssues) and have the all getter return that cached
value; if the value can ever be mutated, add logic to invalidate/update the
cache on mutation, otherwise simply reuse the cached issues to avoid repeated
validation.


// Map standard schema issues to the expected format
return issues?.map((issue: any) => ({
summary: issue.message,
path: issue.path?.join('.') || 'root',
message: issue.message,
value: this.value
Comment on lines +452 to +456
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Value assignment is misleading.

Line 456 assigns this.value (the entire input object) to each error's value property. However, TypeBox's ValueError includes the field-specific value, not the entire input. This inconsistency could mislead consumers who expect the actual field value.

For standard validators, either:

  1. Extract the field-specific value by traversing this.value using issue.path, or
  2. Omit the value property if field-specific extraction is not feasible

Consider extracting field-specific values:

 			// Map standard schema issues to the expected format
 			return issues?.map((issue: any) => ({
 				summary: issue.message,
 				path: issue.path?.join('.') || 'root',
 				message: issue.message,
-				value: this.value
+				value: issue.path?.reduce((obj: any, key: string) => obj?.[key], this.value) ?? this.value
 			})) || []

This traverses the path to extract the specific field's value.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return issues?.map((issue: any) => ({
summary: issue.message,
path: issue.path?.join('.') || 'root',
message: issue.message,
value: this.value
// Map standard schema issues to the expected format
return issues?.map((issue: any) => ({
summary: issue.message,
path: issue.path?.join('.') || 'root',
message: issue.message,
value: issue.path?.reduce((obj: any, key: string) => obj?.[key], this.value) ?? this.value
})) || []
🤖 Prompt for AI Agents 
In src/error.ts around lines 452 to 456, the current code sets each error's
value to this.value (the entire input) which is misleading; change it to derive
the field-specific value by traversing this.value using issue.path (e.g., if
issue.path is empty use this.value/root, otherwise walk the path segments safely
and return undefined if any segment missing) and assign that extracted value to
the error.value property; if implementing traversal is not feasible, remove the
value property instead.

})) || []
}

// Handle TypeBox validators
return 'Errors' in this.validator
? [...this.validator.Errors(this.value)].map(mapValueError)
: // @ts-ignore
Expand Down
78 changes: 78 additions & 0 deletions test/lifecycle/error.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from '../../src'
import { describe, expect, it } from 'bun:test'
import { post, req } from '../utils'
import * as z from 'zod'

describe('error', () => {
it('use custom 404', async () => {
Expand Down Expand Up @@ -450,4 +451,81 @@ describe('error', () => {
expect(value.type).toBe('validation')
expect(value.message).not.toStartWith('{')
})

it('ValidationError.all works with Zod validators', async () => {
const app = new Elysia()
.onError(({ error, code }) => {
if (error instanceof ValidationError) {
const errors = error.all

return {
message: 'Validation failed',
errors: errors
}
}
})
.post('/login', ({ body }) => body, {
body: z.object({
username: z.string(),
password: z.string()
})
})

const res = await app.handle(post('/login', {}))
const data = (await res.json()) as any

expect(data).toHaveProperty('message', 'Validation failed')
expect(data).toHaveProperty('errors')
expect(data.errors).toBeArray()
expect(data.errors.length).toBeGreaterThan(0)
expect(res.status).toBe(422)
})

it('ValidationError.all provides error details with Zod validators', async () => {
const app = new Elysia()
.onError(({ error, code }) => {
expect(code).toBe('VALIDATION')
if (error instanceof ValidationError) {
const errors = error.all

return {
message: 'Validation failed',
errors: errors.map((e: any) => ({
path: e.path,
message: e.message,
summary: e.summary
}))
}
}
})
.post('/user', ({ body }) => body, {
body: z.object({
name: z.string().min(3),
email: z.string(),
age: z.number().min(18)
})
})

const res = await app.handle(
post('/user', {
name: 'ab',
email: 'invalid',
age: 10
})
)
const data = (await res.json()) as any

expect(data).toHaveProperty('message', 'Validation failed')
expect(data).toHaveProperty('errors')
expect(data.errors).toBeArray()
expect(data.errors.length).toBeGreaterThan(0)

for (const error of data.errors) {
expect(error).toHaveProperty('path')
expect(error).toHaveProperty('message')
expect(error).toHaveProperty('summary')
}

expect(res.status).toBe(422)
})
})