Skip to content

Conversation

@ymc9
Copy link
Member

@ymc9 ymc9 commented Mar 24, 2024

Fixes #683

Summary by CodeRabbit

  • New Features
    • Added handling for HTTP status code 422 to indicate unprocessable requests due to validation errors in both RESTful and RPC APIs.
  • Bug Fixes
    • Improved error handling logic to accurately differentiate between validationError and forbidden errors.
  • Tests
    • Updated HTTP status code assertions from 400 to 422 in REST and RPC test cases to reflect the new validation error handling.
    • Added a new test case for user creation to check for email validation errors.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Mar 24, 2024

Walkthrough

Walkthrough

The core of this update revolves around refining the response strategy for validation errors across the application. Specifically, the changes transition the handling of validation errors from a mix of HTTP status codes to consistently using HTTP 422 (Unprocessable Entity). This adjustment is applied across both RESTful and RPC interfaces, enhancing the clarity and standardization of error responses. Additionally, adjustments in test cases and user model validation rules align with this new error handling strategy.

Changes

Files Summary
packages/plugins/openapi/src/.../rest-generator.ts
packages/plugins/openapi/src/.../rpc-generator.ts
Added support for HTTP 422 for validation errors in RESTful and RPC generators.
packages/server/src/api/rest/index.ts
packages/server/src/api/rpc/index.ts
Updated error handling logic to use HTTP 422 for validation errors; differentiated between validation and forbidden errors.
packages/server/tests/api/rest.test.ts
packages/server/tests/api/rpc.test.ts
Updated HTTP status code assertions from 400 to 422 in tests related to validation errors; added new test for email validation.
packages/server/tests/utils.ts Enhanced email validation in User model and expanded user authorization capabilities.

Assessment against linked issues

Objective Addressed Explanation
Use HTTP 422 instead of 403 for field validation errors (#683)
Ensure clear differentiation between validation and forbidden errors
Update test cases to reflect new HTTP status code expectations
Enhance email validation in user model

The changes directly address the primary objective outlined in issue #683, by shifting the error response strategy for validation errors to use HTTP 422. This provides a clearer and more appropriate response to clients when data fails to meet validation criteria. The update in error handling logic, alongside modifications in test cases and model validation rules, ensures a comprehensive and standardized approach to dealing with validation errors across the application.

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?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Note: Auto-reply has been disabled for this repository by the repository owner. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit tests for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit tests.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

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 as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

CodeRabbit Discord Community

Join our Discord Community to get help, request features, and share feedback.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 213fdf3 and 9354900.
Files ignored due to path filters (8)
  • packages/plugins/openapi/tests/baseline/rest-3.0.0.baseline.yaml is excluded by: !**/*.yaml
  • packages/plugins/openapi/tests/baseline/rest-3.1.0.baseline.yaml is excluded by: !**/*.yaml
  • packages/plugins/openapi/tests/baseline/rest-type-coverage-3.0.0.baseline.yaml is excluded by: !**/*.yaml
  • packages/plugins/openapi/tests/baseline/rest-type-coverage-3.1.0.baseline.yaml is excluded by: !**/*.yaml
  • packages/plugins/openapi/tests/baseline/rpc-3.0.0.baseline.yaml is excluded by: !**/*.yaml
  • packages/plugins/openapi/tests/baseline/rpc-3.1.0.baseline.yaml is excluded by: !**/*.yaml
  • packages/plugins/openapi/tests/baseline/rpc-type-coverage-3.0.0.baseline.yaml is excluded by: !**/*.yaml
  • packages/plugins/openapi/tests/baseline/rpc-type-coverage-3.1.0.baseline.yaml is excluded by: !**/*.yaml
Files selected for processing (7)
  • packages/plugins/openapi/src/rest-generator.ts (3 hunks)
  • packages/plugins/openapi/src/rpc-generator.ts (1 hunks)
  • packages/server/src/api/rest/index.ts (3 hunks)
  • packages/server/src/api/rpc/index.ts (2 hunks)
  • packages/server/tests/api/rest.test.ts (3 hunks)
  • packages/server/tests/api/rpc.test.ts (2 hunks)
  • packages/server/tests/utils.ts (1 hunks)
Additional comments: 16
packages/server/tests/utils.ts (2)
  • 8-8: The addition of the @email directive to the email field is a positive change for enhancing email validation. This ensures that the email addresses stored in the database meet a certain format, improving data integrity.
  • 12-12: Updating the authorization rules to include 'create' operations alongside 'read' is a significant change. It broadens the scope of actions that can be performed without explicit authorization checks. Ensure that this aligns with the intended access control policies for the User model, as it could potentially allow any authenticated user to create new records.
packages/server/src/api/rpc/index.ts (2)
  • 133-133: Changing the HTTP status code for data validation violations to 422 is appropriate and aligns with the PR objectives. This status code more accurately reflects the nature of the error encountered, indicating that the request was well-formed but could not be processed due to semantic errors.
  • 158-165: The conditional logic for determining the HTTP status code based on the error metadata is a robust approach to error handling. It allows for a more granular response based on the nature of the error, which can improve the client's ability to handle errors appropriately. Ensure that all potential Prisma error codes are accounted for in the ERROR_STATUS_MAPPING to avoid unintended fallbacks to the default 400 status code.
packages/server/tests/api/rpc.test.ts (3)
  • 179-179: Updating the expected status code from 400 to 422 in the test case for validation errors is correct and aligns with the changes made in the error handling logic. This ensures that the tests accurately reflect the new behavior.
  • 190-190: Similarly, updating the expected status code in this test case is appropriate. It's important that all relevant test cases are updated to expect the new 422 status code for validation errors to maintain test accuracy and reliability.
  • 194-203: Adding a new test case for user creation validation, specifically checking for email validation errors, is a valuable addition. It ensures that the email validation logic is correctly implemented and that appropriate error responses are returned for invalid email addresses.
packages/plugins/openapi/src/rpc-generator.ts (1)
  • 519-526: Adding support for HTTP status code 422 in the OpenAPI specification generation is a thoughtful addition that enhances the API documentation. It provides clients with more detailed information about possible error responses, specifically for unprocessable requests due to validation errors. This aligns with the overall goal of improving API error handling and communication.
packages/plugins/openapi/src/rest-generator.ts (3)
  • 220-220: The addition of the '422' response for validation errors in the makeResourceCreate method aligns well with the PR objectives and follows the existing pattern for handling responses. Good job on ensuring consistency.
  • 296-296: The inclusion of the '422' response for validation errors in the makeResourceUpdate method is consistent and aligns with the PR's objectives. It's good to see consistency in handling responses across different methods.
  • 961-970: The addition of the validationError method to generate the '422' response object is a good practice. It ensures consistency and reduces duplication across the class. Well done on following the established pattern for response methods.
packages/server/src/api/rest/index.ts (2)
  • 170-173: The addition of the validationError object with a status of 422 and a title indicating validation errors is a good practice. It aligns with the PR objectives to more accurately represent errors related to unprocessable entities due to validation issues.
  • 1584-1594: The modification in the handlePrismaError method to differentiate between validationError and forbidden errors based on the CrudFailureReason.DATA_VALIDATION_VIOLATION is well-implemented. This change ensures that the correct HTTP status code is returned for validation errors, aligning with the PR objectives.
packages/server/tests/api/rest.test.ts (3)
  • 1336-1336: The update from HTTP status code 400 to 422 for validation errors is correctly implemented here, aligning with the PR objectives to more accurately reflect the nature of the error encountered.
  • 1673-1673: The update from HTTP status code 400 to 422 for validation errors during an update operation is correctly implemented here, aligning with the PR objectives to more accurately reflect the nature of the error encountered.
  • 1943-1943: The update from HTTP status code 400 to 422 for validation errors due to an invalid email format is correctly implemented here. Including detailed validation errors (zodErrors) in the response is a good practice, enhancing error transparency and aiding in debugging.

@ymc9 ymc9 merged commit c13a1fd into v2 Mar 24, 2024
@ymc9 ymc9 deleted the chore/http-422 branch March 24, 2024 22:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants