fix(py): HttpErrorWireFormat JSON serialization in FastAPI/Flask#4954
fix(py): HttpErrorWireFormat JSON serialization in FastAPI/Flask#4954huangjeff5 merged 11 commits intomainfrom
Conversation
Summary of ChangesHello, 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 resolves critical JSON serialization issues in FastAPI and Flask error handling, ensuring that API clients receive properly formatted error responses instead of internal server errors. It also enhances the robustness of stream processing by correctly propagating cancellation and exceptions, preventing unexpected crashes. These changes improve the reliability and user experience of applications built with Genkit's Python framework. Highlights
Using Gemini Code AssistThe 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 by creating a comment using either
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 Limitations & Feedback Gemini Code Assist 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. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request addresses a TypeError that occurred when serializing HttpErrorWireFormat Pydantic models to JSON in FastAPI and Flask error handlers. The fix correctly converts the Pydantic model to a dictionary before JSON serialization. The PR also includes a valuable improvement to streaming actions by properly handling cancellations and exceptions in callbacks, preventing potential crashes. New tests are added for both fixes. My review includes a couple of suggestions to make the new test assertions more specific and robust.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request correctly addresses a JSON serialization issue with HttpErrorWireFormat in FastAPI and Flask handlers. The core fix, which involves changing get_callable_json to return a dictionary, is sound and effectively prevents the TypeError. The changes are well-supported by updated unit tests and new integration tests for FastAPI that confirm the fix. I've included a couple of suggestions to enhance code consistency and test maintainability.
When validation errors or flow exceptions occur, FastAPI and Flask handlers called json.dumps(get_callable_json(ex)) directly. get_callable_json() returns HttpErrorWireFormat (Pydantic BaseModel), which json.dumps cannot serialize — TypeError was raised and clients received 500 instead of 422/400.
Fix is to make sure get_callable_json returns a dict that can be json.dump() immediately.
Also enabled the case where you want to expose a flow directly as an HTTP function.
Previously you had to write: