feat(standard-server): support parsed body in node adapter#1001
feat(standard-server): support parsed body in node adapter#1001
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds support for preferring a pre-parsed request body: Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant A as Adapter (Express/Next)
participant H as oRPC Handler
participant B as toStandardBody
participant P as Parser
C->>A: HTTP Request
A->>A: (Optional) Body-parsing middleware sets `req.body`
A->>H: Call handler with `(req,res)`
H->>B: toStandardBody(req)
alt req.body defined
B-->>H: Return `req.body` (no stream parsing)
else req.body undefined
B->>P: Parse incoming stream (JSON/multipart/urlencoded/text...)
P-->>B: Standardized body
B-->>H: Parsed body
end
H-->>A: Continue handling with body
A-->>C: Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @unnoq, 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 enhances the oRPC Node.js adapter by allowing it to leverage pre-parsed request bodies from common frameworks, improving compatibility and integration. It introduces a mechanism to check for an existing req.body before performing its own parsing, and updates relevant documentation to guide users on best practices and potential considerations when using this feature alongside external body parsing middleware.
Highlights
- Node.js Adapter Body Parsing: The Node.js adapter for oRPC now supports using pre-parsed request bodies, such as those provided by middleware in frameworks like Express.js or Next.js.
- Documentation Updates: Documentation for Express.js and Next.js adapters has been updated to clarify how oRPC interacts with external body parsers, including warnings about potential limitations with features like Bracket Notation and file uploads.
- Type Definition Enhancement: The
NodeHttpRequesttype has been extended to include an optionalbodyproperty, reflecting the ability to receive pre-parsed request bodies. - New Test Case: A new test has been added to ensure that the
toStandardBodyfunction correctly prioritizes and utilizes an already parsedreq.body.
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 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.
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. ↩
dosubot
bot
added
the
size:S
There was a problem hiding this comment.
Code Review
This pull request adds support for using pre-parsed request bodies in the Node.js adapter, which is useful for integrations with frameworks like Express. The changes include updating the core body parsing logic, associated types, and tests. I've reviewed the changes and found a potential issue with how raw Buffer bodies are handled, which could lead to incorrect behavior for binary uploads. I've provided a suggestion to correctly wrap Buffer bodies into File objects. Additionally, I've suggested some minor improvements to the documentation for clarity. Overall, this is a valuable feature addition.
|
|
||
| ::: warning | ||
| oRPC uses its own request parser. To avoid conflicts, register any body-parsing middleware **after** your oRPC middleware or only on routes that don't use oRPC. | ||
| Express's [body-parser](https://expressjs.com/en/resources/middleware/body-parser.html) handles common request body types, and oRPC will use the parsed body if available. However, it doesn't support features like [Bracket Notation](/docs/openapi/bracket-notation), and in case you upload a file with `application/json`, it may be parsed as plain JSON instead of a `File`. To avoid these issues, register any body-parsing middleware **after** your oRPC middleware or only on routes that don't use oRPC. |
There was a problem hiding this comment.
This paragraph is a bit long and dense. For better readability, consider breaking down the information into a list of limitations.
| Express's [body-parser](https://expressjs.com/en/resources/middleware/body-parser.html) handles common request body types, and oRPC will use the parsed body if available. However, it doesn't support features like [Bracket Notation](/docs/openapi/bracket-notation), and in case you upload a file with `application/json`, it may be parsed as plain JSON instead of a `File`. To avoid these issues, register any body-parsing middleware **after** your oRPC middleware or only on routes that don't use oRPC. | |
| Express's [body-parser](https://expressjs.com/en/resources/middleware/body-parser.html) handles common request body types, and oRPC will use the parsed body if available. However, this approach has some limitations: | |
| - It doesn't support features like [Bracket Notation](/docs/openapi/bracket-notation). | |
| - A file uploaded with `application/json` may be parsed as a plain JSON object instead of a `File`. | |
| To avoid these issues, register any body-parsing middleware **after** your oRPC middleware or only on routes that don't use oRPC. |
| ::: warning | ||
|
|
||
| Next.js default [body parser](https://nextjs.org/docs/pages/building-your-application/routing/api-routes#custom-config) blocks oRPC raw‑request handling. Ensure `bodyParser` is disabled in your API route: | ||
| Next.js [body parser](https://nextjs.org/docs/pages/building-your-application/routing/api-routes#custom-config) may handle common request body types, and oRPC will use the parsed body if available. However, it doesn't support features like [Bracket Notation](/docs/openapi/bracket-notation), and in case you upload a file with `application/json`, it may be parsed as plain JSON instead of a `File`. To avoid these issues, disable the body parser: |
There was a problem hiding this comment.
This paragraph is a bit long and dense. For better readability, consider breaking down the information into a list of limitations.
| Next.js [body parser](https://nextjs.org/docs/pages/building-your-application/routing/api-routes#custom-config) may handle common request body types, and oRPC will use the parsed body if available. However, it doesn't support features like [Bracket Notation](/docs/openapi/bracket-notation), and in case you upload a file with `application/json`, it may be parsed as plain JSON instead of a `File`. To avoid these issues, disable the body parser: | |
| Next.js [body parser](https://nextjs.org/docs/pages/building-your-application/routing/api-routes#custom-config) may handle common request body types, and oRPC will use the parsed body if available. However, this approach has some limitations: | |
| - It doesn't support features like [Bracket Notation](/docs/openapi/bracket-notation). | |
| - A file uploaded with `application/json` may be parsed as a plain JSON object instead of a `File`. | |
| To avoid these issues, disable the body parser: |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-event-iterator
@orpc/hey-api
@orpc/interop
@orpc/json-schema
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/otel
@orpc/react
@orpc/react-query
@orpc/experimental-react-swr
@orpc/server
@orpc/shared
@orpc/solid-query
@orpc/standard-server
@orpc/standard-server-aws-lambda
@orpc/standard-server-fetch
@orpc/standard-server-node
@orpc/standard-server-peer
@orpc/svelte-query
@orpc/tanstack-query
@orpc/trpc
@orpc/valibot
@orpc/vue-colada
@orpc/vue-query
@orpc/zod
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
apps/content/docs/adapters/next.md (1)
76-87: Clarify Pages Router scope and tighten wording.This warning applies to the Pages Router only (not App Router). Minor phrasing tweak improves clarity.
-Next.js [body parser](https://nextjs.org/docs/pages/building-your-application/routing/api-routes#custom-config) may handle common request body types, and oRPC will use the parsed body if available. However, it doesn't support features like [Bracket Notation](/docs/openapi/bracket-notation), and in case you upload a file with `application/json`, it may be parsed as plain JSON instead of a `File`. To avoid these issues, disable the body parser: +In the Pages Router, Next.js's [body parser](https://nextjs.org/docs/pages/building-your-application/routing/api-routes#custom-config) may pre‑parse request bodies, and oRPC will prefer `req.body` when present. However, it doesn't support features like [Bracket Notation](/docs/openapi/bracket-notation), and if you upload a file with `application/json`, it may be parsed as plain JSON instead of a `File`. To avoid these issues, disable the body parser for routes that use oRPC:apps/content/docs/adapters/express.md (1)
11-11: Tiny wording nit.Swap “in case you” with “if you” for concision.
-..., and in case you upload a file with `application/json`, it may be parsed as plain JSON instead of a `File`. +..., and if you upload a file with `application/json`, it may be parsed as plain JSON instead of a `File`.packages/standard-server-node/src/body.test.ts (1)
190-202: Avoid ts-expect-error by casting to NodeHttpRequest.Keeps the test clean and future‑proof to type changes.
- await request(async (req: IncomingMessage, res: ServerResponse) => { - // @ts-expect-error fake body is parsed - req.body = { value: 123 } + await request(async (req: IncomingMessage, res: ServerResponse) => { + ;(req as unknown as import('./types').NodeHttpRequest).body = { value: 123 } standardBody = await toStandardBody(req) res.end() }).post('/').send()If you adopt the method-guard suggested in
toStandardBody, consider adding a companion test to assert thatGET/HEADignorereq.body.packages/standard-server-node/src/body.ts (1)
13-15: Guard early-exit by method and keep tracing span.As written, any upstream
req.bodyonGET/HEADwould change prior semantics (previously returnedundefined) and the early return skips theparse_standard_bodyspan. Guarding by method preserves behavior and wrapping insiderunWithSpanretains observability.-export function toStandardBody(req: NodeHttpRequest, options: ToStandardBodyOptions = {}): Promise<StandardBody> { - if (req.body !== undefined) { - return Promise.resolve(req.body) - } - - return runWithSpan({ name: 'parse_standard_body', signal: options.signal }, async () => { +export function toStandardBody(req: NodeHttpRequest, options: ToStandardBodyOptions = {}): Promise<StandardBody> { + return runWithSpan({ name: 'parse_standard_body', signal: options.signal }, async () => { + const method = req.method?.toUpperCase() + // Prefer pre-parsed bodies only for methods that conventionally carry a payload. + if (req.body !== undefined && method !== 'GET' && method !== 'HEAD') { + return req.body + } const contentDisposition = req.headers['content-disposition'] const contentType = req.headers['content-type']Optional: if you want to be extra defensive, also ignore
req.bodywhencontent-type: text/event-stream.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/content/docs/adapters/express.md(1 hunks)apps/content/docs/adapters/next.md(1 hunks)packages/standard-server-node/src/body.test.ts(1 hunks)packages/standard-server-node/src/body.ts(1 hunks)packages/standard-server-node/src/types.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/standard-server-node/src/body.test.ts (2)
packages/standard-server/src/types.ts (1)
StandardBody(5-11)packages/standard-server-node/src/body.ts (1)
toStandardBody(12-51)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: publish-commit
- GitHub Check: test
- GitHub Check: lint
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/standard-server-node/src/types.ts (1)
10-14: LGTM: typed hook for pre-parsed bodies.
body?: unknownonNodeHttpRequestmakes the early-exit explicit without over-constraining types. No action needed.
dosubot
bot
added
size:M
dinwwwh
force-pushed
the
feat/standard-server/support-parsed-body-in-node
branch
from
fabd202 to
2c8a448
Compare
dosubot
bot
added
size:S
dosubot
bot
added
size:M
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/nest/src/implement.ts (1)
129-131: Tiny readability tweak: async instead of Promise.resolve and clearer commentFunctionally equivalent, but a bit clearer and fixes the comment grammar.
Apply this diff:
- // Prefer NestJS parsed body (in nodejs body only allow parse once) - standardRequest.body = () => Promise.resolve( - req.body === undefined ? fallbackStandardBody() : req.body, - ) + // Prefer NestJS‑parsed body; in Node.js the incoming stream can be read only once. + standardRequest.body = async () => + req.body === undefined ? await fallbackStandardBody() : req.body
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/nest/src/implement.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: publish-commit
- GitHub Check: lint
- GitHub Check: test
🔇 Additional comments (1)
packages/nest/src/implement.ts (1)
129-131: Prefer parsed body when defined — LGTMThe strict
undefinedcheck correctly preservesnullbodies and falls back only when needed. This aligns with environments like Firebase/GCF where the body is pre‑parsed. No functional concerns.Please sanity‑check with Nest on both Express and Fastify for:
- application/json (parsed object)
- text/plain (string)
- no body (undefined vs null behavior)
If you want, I can add a tiny e2e test for Nest to cover these cases.
1 participant
Closes: https://github.com/unnoq/orpc/issues/958
Summary by CodeRabbit
Bug Fixes
Documentation
Tests