feat(shared): add custom error handling for schema validation #434
Conversation
|
|
Warning Rate limit exceeded@luxass has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 22 minutes and 52 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (4)
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR introduces a specialized Changes
Sequence DiagramsequenceDiagram
participant Client
participant Fetch Handler
participant Schema Validator
participant Error Handler
participant Response
Client->>Fetch Handler: fetch with schema
Fetch Handler->>Response: HTTP request
Response->>Fetch Handler: response received
alt Status < 400 (Success)
Fetch Handler->>Schema Validator: validate response against schema
alt Validation passes
Fetch Handler->>Client: return typed response
else Validation fails
rect rgb(255, 200, 200)
Schema Validator->>Error Handler: throw ValidationError
Error Handler->>Error Handler: wrap in FetchSchemaValidationError
Error Handler-->>Error Handler: attach cause & issues
end
Error Handler->>Client: throw FetchSchemaValidationError
end
else Status >= 400 (Error)
rect rgb(200, 200, 255)
Fetch Handler->>Error Handler: skip schema validation
Error Handler->>Client: throw FetchError
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
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 |
🌏 Preview Deployments
Built from commit: 🤖 This comment will be updated automatically when you push new commits to this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
This PR adds custom error handling for schema validation failures by introducing a new FetchSchemaValidationError class that extends FetchError. The changes enable more specific error handling when response data fails Zod schema validation.
Key Changes
- Introduced
FetchSchemaValidationErrorclass with anissuesfield to capture Zod validation errors - Updated schema validation logic to use the new error type and preserve it through the error handling flow
- Added tests covering schema validation failures for both successful and error responses
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/shared/src/fetch/error.ts | Adds new FetchSchemaValidationError class extending FetchError with validation issues support |
| packages/shared/src/fetch/fetch.ts | Updates error handling to preserve FetchError subclasses and uses FetchSchemaValidationError for schema validation failures |
| packages/shared/src/fetch/types.ts | Updates SafeFetchResponse type to include FetchSchemaValidationError in error union type |
| packages/shared/test/fetch/fetch-schema.test.ts | Adds comprehensive test coverage for the new error handling behavior including edge cases with error responses |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Greptile SummaryThis PR introduces dedicated error handling for schema validation failures in the custom fetch utility. A new Key Changes:
Issue Found:
Confidence Score: 4/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant customFetch
participant fetch API
participant Schema
participant ErrorHandler
Client->>customFetch: customFetch(url, {schema})
customFetch->>fetch API: fetch(request, options)
alt Fetch Success
fetch API-->>customFetch: Response
alt Status < 400 (Success)
customFetch->>customFetch: Parse response data
customFetch->>Schema: schema.safeParseAsync(data)
alt Validation Success
Schema-->>customFetch: {success: true, data}
customFetch-->>Client: FetchResponse with validated data
else Validation Failed
Schema-->>customFetch: {success: false, error}
customFetch->>ErrorHandler: FetchSchemaValidationError
ErrorHandler-->>Client: Throw FetchSchemaValidationError
end
else Status >= 400 (Error)
Note over customFetch,Schema: Skip schema validation
customFetch->>ErrorHandler: FetchError
ErrorHandler-->>Client: Throw FetchError
end
else Fetch Failed
fetch API-->>customFetch: Error
customFetch->>ErrorHandler: handleError()
ErrorHandler-->>Client: Throw FetchError
end
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @packages/shared/test/fetch/fetch-schema.test.ts:
- Around line 164-173: The test name "should emit FetchSchemaValidationError
when error response fails schema" contradicts its assertions which check for
FetchError and not FetchSchemaValidationError; update the test description (the
it(...) string) to accurately reflect actual behavior—for example "should return
FetchError and not FetchSchemaValidationError for error-invalid response"—so the
test name matches the assertions involving FetchError and
FetchSchemaValidationError in the test block.
🧹 Nitpick comments (1)
packages/shared/src/fetch/error.ts (1)
63-75: Consider makingcauseoptional in the constructor options.The parent class
FetchErrorhasopts?: { cause: unknown }wherecauseis effectively required whenoptsis provided. However, the newFetchSchemaValidationErrorconstructor signatureopts?: { cause: unknown; issues?: unknown }requirescauseto be present ifoptsis passed, which prevents passing onlyissueswithout acause.For consistency and flexibility, consider making
causeoptional:🔎 Proposed fix
- constructor(message: string, opts?: { cause: unknown; issues?: unknown }) { + constructor(message: string, opts?: { cause?: unknown; issues?: unknown }) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/shared/src/fetch/error.tspackages/shared/src/fetch/fetch.tspackages/shared/src/fetch/types.tspackages/shared/test/fetch/fetch-schema.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{test,spec}.?(c|m)[jt]s?(x)
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{test,spec}.?(c|m)[jt]s?(x): Import test utilities from #test-utils/* path aliases instead of @ucdjs/test-utils in test files to avoid build step requirements and prevent cyclic dependencies
Use testdir() from vitest-testdirs to create temporary test directories in filesystem tests
Use mockStoreApi() utility from #test-utils/mock-store for MSW-based mocking of the UCD API in test files
Files:
packages/shared/test/fetch/fetch-schema.test.ts
🧠 Learnings (1)
📚 Learning: 2025-12-27T11:08:54.630Z
Learnt from: CR
Repo: ucdjs/ucd PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-27T11:08:54.630Z
Learning: Applies to apps/api/src/**/*.ts : Use Zod schemas for defining API route schemas and request/response types in the API application
Applied to files:
packages/shared/src/fetch/types.ts
🧬 Code graph analysis (2)
packages/shared/src/fetch/types.ts (1)
packages/shared/src/fetch/error.ts (2)
FetchError(8-62)FetchSchemaValidationError(64-75)
packages/shared/src/fetch/fetch.ts (1)
packages/shared/src/fetch/error.ts (2)
FetchError(8-62)FetchSchemaValidationError(64-75)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Upload results
- GitHub Check: lint
- GitHub Check: typecheck
- GitHub Check: Greptile Review
- GitHub Check: test (windows-latest)
- GitHub Check: test (macos-latest)
🔇 Additional comments (7)
packages/shared/src/fetch/types.ts (1)
2-2: LGTM!The type updates correctly expand
SafeFetchResponse.errorto include the newFetchSchemaValidationError<T>variant, maintaining proper type parameter propagation. This aligns well with the runtime changes infetch.ts.Also applies to: 29-33
packages/shared/src/fetch/fetch.ts (3)
85-100: LGTM! Good preservation of error subclass identity.The updated
handleErrorcorrectly preservesFetchErrorsubclasses (likeFetchSchemaValidationError) while ensuring common fields are attached. This maintains the error type through the retry/error handling flow.
222-226: Verify this design choice aligns with expected usage patterns.Schema validation is now skipped for error responses (status >= 400). This means users cannot validate error response payloads against a schema—they'll receive a generic
FetchErrorinstead ofFetchSchemaValidationError.If this is intentional (e.g., schemas are meant only for success responses), consider documenting this behavior in the
schemaoption's JSDoc at lines 45-53.
229-233: LGTM!The
FetchSchemaValidationErroris constructed with appropriate context: a descriptive message, the original Zod error as the cause, and the validation issues for programmatic inspection.packages/shared/test/fetch/fetch-schema.test.ts (3)
1-6: LGTM!The test imports correctly bring in the new
FetchSchemaValidationErroralongsideFetchErrorfor assertion purposes. The import path to production code is appropriate here (the coding guidelines about#test-utils/*apply to test utilities, not production exports).
136-162: Good coverage of error response scenarios.These tests effectively validate that schema validation is skipped for error responses (status >= 400), ensuring users receive
FetchErrorrather thanFetchSchemaValidationErrorfor 4xx/5xx responses.
12-12: No action needed.z.email()is the correct syntax for Zod 4.2.1 (the version used by this project). This is a top-level string format factory introduced in Zod 4.x and is the standard way to validate email format.
Introduce `FetchSchemaValidationError` to handle schema validation errors specifically. Update `createCustomFetch` to throw this error when response validation fails, preserving context and issues for better debugging. Update type definitions to accommodate the new error type.
- Introduced `FetchSchemaValidationError` for better error categorization. - Added tests for various error response scenarios to ensure robust validation. - Updated existing tests to reflect changes in error handling logic.
luxass
force-pushed
the
feat/custom-error-for-fetch-schema
branch
from
efda26e to
2ac5d5d
Compare
🔗 Linked issue
📚 Description
This PR adds a custom error for when schema fails validation.
This will help close the mega PR of #420
Summary by CodeRabbit
New Features
Improvements
✏️ Tip: You can customize this high-level summary in your review settings.