-
-
Notifications
You must be signed in to change notification settings - Fork 4
[FEATURE] Custom Error Handler #6
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
Conversation
WalkthroughThe pull request introduces enhancements to error handling in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Server
participant ErrorHandler
User->>Server: Make a request
Server->>Server: Process request
alt Error occurs
Server->>ErrorHandler: Call handleErrors(errorType, issues)
ErrorHandler-->>Server: Return custom Response
else No Error
Server-->>User: Return successful Response
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (6)
src/types/error.ts (1)
1-7: LGTM! Consider adding JSDoc comments.The implementation of
customErrorTypeslooks good. The use ofas constensures type safety for the string literals. To improve documentation, consider adding JSDoc comments explaining the purpose and usage of this constant.Here's a suggested addition:
/** * Defines a set of custom error types related to parsing operations. * These types can be used for specific error handling in request processing. */ export const customErrorTypes = [ // ... (existing code) ];src/core/search-params.ts (1)
22-22: Approved: Enhanced error reporting with a minor suggestion.The change improves error handling by including the
ZodErrorissues as the cause of the thrown error. This enhancement aligns with the PR objective and provides more context for debugging.For improved type safety, consider using a type assertion or type guard before accessing the
issuesproperty:throw new Error("PARSE_SEARCH_PARAMS", { cause: error instanceof ZodError ? error.issues : error });This ensures that
causeis always set, even if the caught error is not aZodError.src/core/body.ts (1)
43-46: Improved handling of empty JSON input.The new conditional check for "Unexpected end of JSON input" error enhances the function's robustness by handling cases where the request body is empty. This approach provides more informative error reporting for this specific scenario.
Consider adding a brief comment explaining why this specific case is handled differently, for example:
+ // Handle empty request body case if (error instanceof Error && error.message === "Unexpected end of JSON input") { const result = schema.safeParse({}); throw new Error("PARSE_REQUEST_BODY", { cause: result.error?.issues }); }README.md (2)
57-57: Consider expanding the description forhandleErrors.The addition of the
handleErrorsproperty is well-documented. However, to provide more clarity for users, consider expanding the description slightly.Suggested improvement:
- | handleErrors | (errorType: string, issues?: [ZodIssues](https://zod.dev/ERROR_HANDLING?id=zodissue)[]) => Response | `(Optional)` Custom error handler can be provided to replace the default behavior. [See below](#example) | + | handleErrors | (errorType: string, issues?: [ZodIssues](https://zod.dev/ERROR_HANDLING?id=zodissue)[]) => Response | `(Optional)` Custom error handler function to replace the default error handling behavior. It receives the error type and optional Zod validation issues. [See example below](#example) for usage. |
102-116: Great example, consider adding adefaultcase.The example effectively demonstrates the usage of the new
handleErrorsproperty. It covers all the possible error types and provides a clear implementation.To make the example even more robust, consider adding a
defaultcase to the switch statement:handleErrors: (errorType, issues) => { console.log(issues); switch (errorType) { case "PARSE_FORM_DATA": case "PARSE_REQUEST_BODY": case "PARSE_SEARCH_PARAMS": return new Response(null, { status: 400 }); case "PARSE_PATH_PARAMS": return new Response(null, { status: 404 }); case "UNNECESSARY_PATH_PARAMS": case "UNKNOWN_ERROR": return new Response(null, { status: 500 }); + default: + return new Response(null, { status: 500 }); } },This ensures that all possible cases are handled, even if new error types are introduced in the future.
src/core/definer.test.ts (1)
265-267: Leverage_errorTypeand_causeinhandleErrorsThe parameters
_errorTypeand_causein yourhandleErrorsfunction are currently unused. To enhance error handling, consider utilizing these parameters to provide more specific responses or to log detailed error information.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
- README.md (2 hunks)
- src/core/body.ts (1 hunks)
- src/core/definer.test.ts (1 hunks)
- src/core/definer.ts (4 hunks)
- src/core/search-params.ts (1 hunks)
- src/types/error.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (8)
src/core/body.ts (3)
37-37: Improved error handling for form data parsing.The addition of the
causeproperty to the thrown error enhances error reporting by including detailed ZodError issues. This change allows for more effective debugging and error handling in the calling code.
51-51: Consistent error handling for JSON parsing.The addition of the
causeproperty to the thrown error in the JSON parsing section maintains consistency with the form data parsing error handling. This change enhances error reporting by including detailed ZodError issues, facilitating better debugging and error handling in the calling code.
37-51: Overall improvement in error handling and reporting.The changes in this file consistently enhance error handling and reporting across different parsing scenarios (form data and JSON). By including detailed error information in the
causeproperty of thrown errors, these modifications provide valuable context for debugging and custom error handling. This aligns well with the PR objective of implementing a custom error handler feature.README.md (1)
Line range hint
1-116: Overall, excellent documentation of the new feature.The changes to the README.md file effectively introduce and explain the new
handleErrorsfeature. The documentation is clear, concise, and includes a helpful example of usage. The minor suggestions provided will further enhance the clarity and robustness of the documentation.src/core/definer.ts (3)
1-1: Import of 'customErrorTypes' is correctly addedThe import statement for
customErrorTypesfrom"~/types/error"is appropriate and correctly implemented.
10-10: Import of 'Zod' types is accurateThe types
ZodIssue,ZodType, andZodTypeDefare correctly imported from"zod".
48-48: Addition of 'handleErrors' enhances error handling flexibilityThe optional
handleErrorsproperty in theRouteOptionstype allows for custom error handling, improving the flexibility and robustness of thedefineRoutefunction.src/core/definer.test.ts (1)
296-300: 🛠️ Refactor suggestionEnsure consistency of
errorTypevaluesIn your
handleErrorsfunction, you check iferrorType === "UNKNOWN_ERROR". Verify that"UNKNOWN_ERROR"is the correct and consistent value used by your error handling system for unexpected errors. Consider defining error types as constants or enums to prevent typos and improve maintainability.Run the following script to check all occurrences of
errorTypevalues:✅ Verification successful
Consistency of
errorTypeValues VerifiedAll instances of
errorTypecomparisons use the consistent value"UNKNOWN_ERROR". No discrepancies found in the error handling system.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all unique `errorType` values used in the codebase. # Search for errorType comparisons and extract the literals. rg --type ts --pcre2 --no-filename --no-line-number "errorType\s*===\s*[\"'](\w+)[\"']" -or '$1' | sort | uniqLength of output: 130
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
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.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- src/core/definer.test.ts (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
src/core/definer.test.ts (2)
248-276: LGTM!The test case is well-written and effectively tests the custom error handler for an expected error scenario.
278-309: LGTM!The test case correctly verifies the custom error handler for an unexpected error scenario.
Summary by CodeRabbit
New Features
handleErrorsproperty for custom error handling in route definitions.Bug Fixes
Tests
defineRoutefunction to validate custom error handling for expected and unexpected errors.Documentation
handleErrorsproperty and example usage.