fix(a2a-server): mount express.json before a2a sdk routes - #29126
fix(a2a-server): mount express.json before a2a sdk routes#29126Anurag-M1 wants to merge 3 commits into
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 an issue where A2A SDK routes were receiving undefined request bodies due to incorrect middleware registration order. By moving the JSON parsing middleware before the route initialization, the application now correctly processes incoming JSON-RPC requests. 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 the 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 counterproductive. 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
|
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
|
📊 PR Size: size/S
|
There was a problem hiding this comment.
Code Review
This pull request moves the registration of the JSON body parser middleware before setting up the A2A SDK routes to ensure proper request parsing, and adds a corresponding E2E test. The reviewer suggests optimizing the new test by reusing the existing shared application instance instead of re-initializing it with createApp(), which is redundant and inefficient.
| const app = await createApp(); | ||
| const res = await request(app) |
There was a problem hiding this comment.
Re-initializing the application by calling createApp() inside an individual test case is redundant and inefficient. The test suite already sets up a shared app instance in the beforeAll hook (line 115). Re-running createApp() performs expensive operations like loading settings, config, and changing the working directory (process.chdir), which can lead to test flakiness and slow down the test suite. You should reuse the existing app instance instead.
| const app = await createApp(); | |
| const res = await request(app) | |
| const res = await request(app) |
Fixes #29073
Description
In
packages/a2a-server/src/http/app.ts,express.json()was mounted afterappBuilder.setupRoutes(expressApp, ''). As a result, A2A SDK routes (such asPOST /) receivedreq.bodyas undefined, breaking JSON-RPC parsing.Changes
expressApp.use(express.json())prior toappBuilder.setupRoutesincreateApp().packages/a2a-server/src/http/app.test.tsto verify JSON-RPC body parsing on SDK routes.