fix: handle Yup ValidationError in getServerErrorFromUnknown #25205
Conversation
Add support for Yup ValidationError thrown by the ics library when generating calendar files with invalid attendee emails. This prevents 500 errors and returns proper 400 status codes with validation messages. The ics library uses Yup for validation, and when attendee emails are invalid (e.g., street addresses instead of emails), it throws a ValidationError that was not being caught by getServerErrorFromUnknown. Changes: - Add isYupValidationError type guard to detect Yup ValidationErrors - Add parseYupValidationErrors to extract error messages from inner array - Handle Yup ValidationError in getServerErrorFromUnknown, returning 400 status Co-Authored-By: [email protected] <[email protected]>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 2 files
Prompt for AI agents (all 1 issues)
Understand the root cause of the following 1 issues and fix them.
<file name="packages/lib/server/getServerErrorFromUnknown.ts">
<violation number="1" location="packages/lib/server/getServerErrorFromUnknown.ts:39">
Rule violated: **Enforce Singular Naming for Single-Item Functions**
Rename `parseYupErrors` to a singular form (e.g., `parseYupErrorMessage`) because it returns one string rather than multiple values, in order to comply with the singular-naming rule for single-item functions.</violation>
</file>
Reply to cubic to teach it or ask questions. Re-run a review with @cubic-dev-ai review this PR
| function isYupError(cause: unknown): cause is { | ||
| name: string; | ||
| message: string; | ||
| inner: Array<{ path?: string; message: string; type?: string }>; | ||
| errors: string[]; | ||
| } { | ||
| return ( | ||
| hasName(cause) && | ||
| cause.name === "ValidationError" && | ||
| typeof cause === "object" && | ||
| "inner" in cause && | ||
| Array.isArray((cause as { inner: unknown }).inner) | ||
| ); |
There was a problem hiding this comment.
Aren't we leaking too much implementation detail in here. TRPCError and HttpError are fine as those are like pretty generic and used everywhere.
Ideally the service/place using ics should handle this error and return appropriate HttpError or some known error may be?
E2E results are ready! |
hariombalhara
left a comment
hariombalhara
left a comment
There was a problem hiding this comment.
Requesting changes, feel free to dismiss if doesn't make sense
What does this PR do?
Fixes 500 errors on
api/book/eventendpoint when theicslibrary throws Yup ValidationErrors during calendar file generation. This occurs when attendees have invalid email addresses (e.g., street addresses instead of emails).Problem: The
icslibrary (v2.37.0) uses Yup for validation. When generating ICS calendar files with invalid attendee emails, it throws aValidationErrorthat wasn't recognized bygetServerErrorFromUnknown, causing it to fall through to the generic handler and return 500 status instead of 400.Solution: Add Yup ValidationError detection and handling to return proper 400 status codes with validation error messages.
Mandatory Tasks (DO NOT REMOVE)
Link to Devin run: https://app.devin.ai/sessions/80180c74544745e78b2d5fee68a10fca
Requested by: [email protected] (@hbjORbj)