-
Notifications
You must be signed in to change notification settings - Fork 626
Issue[#689] Solved #703
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.
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
base: main
Are you sure you want to change the base?
Issue[#689] Solved #703
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Summary
Improved error handling across the codebase by replacing Zod's parse() with safeParse() to provide more detailed validation error messages and better debugging capabilities.
- Syntax error in
evals/args.ts
line 92 with unexpected '=' character needs fixing - Duplicate image warning checks in
examples/external_clients/customOpenAI.ts
at lines 47 and 72 should be consolidated - Double stagehand instance closure in
evals/tasks/extract_press_releases.ts
needs to be resolved - Missing destructuring of
items
fromparsed.data
inevals/tasks/extract_press_releases.ts
- Error messages in catch blocks should include the detailed validation errors from
safeParse()
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
5 file(s) reviewed, 6 comment(s)
Edit PR Review Bot Settings | Greptile
const parsed = schema.safeParse(rawResult); | ||
if(!parsed.success){ | ||
throw new Error(parsed.error.format()); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Error.format() returns a complex object that may not stringify well in the catch block's error logging
const result = { page_text: formattedText }; | ||
return pageTextSchema.parse(result); | ||
} | ||
const res = pageTextSchema.safeParse(result); | ||
|
||
if (!res.success) { | ||
throw new Error(JSON.stringify(res.error.format())); | ||
} | ||
|
||
return res.data; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
syntax: Indentation issue - closing brace of extractPageText() is misaligned
const result = { page_text: formattedText }; | |
return pageTextSchema.parse(result); | |
} | |
const res = pageTextSchema.safeParse(result); | |
if (!res.success) { | |
throw new Error(JSON.stringify(res.error.format())); | |
} | |
return res.data; } | |
const result = { page_text: formattedText }; | |
const res = pageTextSchema.safeParse(result); | |
if (!res.success) { | |
throw new Error(JSON.stringify(res.error.format())); | |
} | |
return res.data; | |
} |
function validateZodSchema(schema: z.ZodTypeAny, data: unknown) { | ||
try { | ||
schema.parse(data); | ||
return true; | ||
} catch { | ||
return false; | ||
} | ||
const result = schema.safeParse(data); | ||
return result.success; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: validateZodSchema could return the error details from safeParse instead of just a boolean for better error reporting
export function validateZodSchema(schema: z.ZodTypeAny, data: unknown) { | ||
try { | ||
schema.parse(data); | ||
return true; | ||
} catch { | ||
return false; | ||
} | ||
const result = schema.safeParse(data); | ||
return result.success; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider returning result.error for better error reporting, since safeParse provides detailed validation errors
export function validateZodSchema(schema: z.ZodTypeAny, data: unknown) { | |
try { | |
schema.parse(data); | |
return true; | |
} catch { | |
return false; | |
} | |
const result = schema.safeParse(data); | |
return result.success; | |
export function validateZodSchema(schema: z.ZodTypeAny, data: unknown): { success: boolean; error?: z.ZodError } { | |
const result = schema.safeParse(data); | |
return { success: result.success, error: result.success ? undefined : result.error }; |
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
Co-authored-by: greptile-apps[bot] <165735046+greptile-apps[bot]@users.noreply.github.com>
why
Current error handling for Zod parsing failures logs only "Invalid response schema" without context, which makes debugging difficult — especially in automated test scenarios like stagehand. Developers have to dig deeper into the code or add temporary logs to identify the root issue.
what changed
Replaced schema.parse(data) with schema.safeParse(data) to gracefully handle validation errors.
When parsing fails, the error is formatted using result.error.format() and thrown as a MyCustomError.
This provides a structured, detailed breakdown of validation issues for each field.