-
Notifications
You must be signed in to change notification settings - Fork 0
Fix un propagated exception messages #18
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
The errorHandler middleware was not adding CORS headers to the error responses it generated. This caused browsers to block client-side applications from reading the response body due to the Same-Origin Policy, resulting in generic network errors instead of specific API error messages. This change refactors the errorHandler to use a helper function that constructs the JSON error response while also adding the necessary `Access-Control-Allow-Origin` header. This ensures that all error responses are readable by the client, fixing the bug.
The errorHandler middleware was not adding CORS headers to error responses in a production-ready way. This caused browsers to block client-side applications from reading the response body, resulting in generic network errors instead of specific API error messages. This change refactors the errorHandler to use an environment-aware helper function. It now checks for a `CORS_ALLOWED_ORIGIN` environment variable for production and falls back to allowing any `localhost` origin for development. This aligns the error response behavior with the main CORS middleware and the project's documentation, fixing the bug.
Adds the `CORS_ALLOWED_ORIGIN` environment variable to the example configuration file. This synchronizes the `.env.example` with the existing documentation and recent CORS-related code changes, ensuring developers are aware of this required setting for production deployments.
Adds the `CORS_ALLOWED_ORIGIN` environment variable to the example configuration file. This synchronizes the `.env.example` with the existing documentation and recent CORS-related code changes, ensuring developers are aware of this required setting for production deployments.
Updates the environment variable example in the README.md to match the format and content of `.env.example`. This change adds comments explaining each variable and comments them out by default, improving clarity and ensuring consistency across project documentation.
Updates the comments in the `.env.example` file to be more explicit and unambiguous. The new comments clearly state when each variable is required (e.g., "REQUIRED" vs. "REQUIRED FOR PRODUCTION") and provide better context for their purpose. This resolves confusion around the previous "Optional for Production" wording.
The errorHandler middleware was missing the `Access-Control-Allow-Credentials` header in its CORS configuration. This caused browsers to block credentialed requests (e.g., those with an Authorization header) that resulted in an error, leading to a specific CORS failure. This change adds the `Access-Control-Allow-Credentials: true` header to all error responses when the origin is allowed, resolving the issue and allowing the client to correctly read API error messages for authenticated or credentialed requests.
Updates the "Configuration" section in the README.md to be clearer and more direct. Instead of instructing users to create a .env file from scratch and duplicating the example content, it now instructs them to copy the existing `.env.example` file. This removes redundancy and improves the developer setup experience.
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.
Summary of Changes
Hello @fulleni, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request addresses the issue of unpropagated exception messages by significantly enhancing the API's error handling and Cross-Origin Resource Sharing (CORS) configuration. It centralizes the generation of error responses and integrates dynamic CORS header management directly into these responses, ensuring that clients can consistently access detailed error information. The changes also include updates to environment variable documentation for improved developer experience.
Highlights
- Centralized Error Response Handling: The error handling middleware has been refactored to use a new private helper function (
_jsonErrorResponse
) for generating all JSON error responses. This standardizes the format and ensures consistent error propagation. - CORS Header Management for Error Responses: The new error response helper dynamically adds
Access-Control-Allow-Origin
and other necessary CORS headers to error responses. This ensures that client-side applications can properly read error message bodies, preventing issues where browsers might block access to these details due to CORS policies. - Environment Configuration Updates: A new environment variable,
CORS_ALLOWED_ORIGIN
, has been introduced to allow explicit configuration of the allowed origin for CORS in production environments. The.env.example
file andREADME.md
have been updated to reflect this new configuration option and provide clearer setup instructions.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/
folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
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.
Code Review
This pull request introduces a centralized _jsonErrorResponse
helper in the errorHandler
middleware to ensure that all error responses include the necessary CORS headers. I've identified a few areas for improvement:
- A potential breaking change where specific error codes (
'invalidField'
,'invalidFormat'
) are now generalized to'invalidInput'
. - Minor inconsistencies in the new CORS handling logic compared to the existing CORS middleware for successful responses.
Status
READY/IN DEVELOPMENT/HOLD
Description
Type of Change