feat(server): allow RPC calls without data#853
Conversation
- underline rpc serializer support deserialize `undefined` data - any serializer should accept `undefined` as valid deserialize data
WalkthroughUpdates docs to alter RPC routing example, adds undefined-input handling in StandardRPCSerializer.deserialize, and introduces a unit test asserting deserialize(undefined) returns undefined. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Serializer as StandardRPCSerializer
participant Json as StandardRPCJsonSerializer
Caller->>Serializer: deserialize(data)
alt data is undefined
Serializer-->>Caller: return undefined
else data is not FormData
Serializer->>Json: deserialize(data)
Json-->>Serializer: parsed
Serializer-->>Caller: result
else data is FormData
Serializer->>Serializer: parse entries/blobs
Serializer->>Json: deserialize(parsed JSON fields)
Json-->>Serializer: parsed
Serializer-->>Caller: result
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested labels
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
dosubot
Bot
added
the
size:S
![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!
I've implemented a new feature that allows RPC calls to be made without explicitly providing data. Previously, even for calls with no data, a ?data={} parameter was required. This change streamlines the RPC protocol by enabling the StandardRPCSerializer to correctly deserialize undefined data, making the ?data={} parameter unnecessary for such calls.
Highlights
- Documentation Update: The RPC protocol documentation has been updated to reflect the new way of making RPC calls without data, removing the need for
?data={}in the example. - RPC Serializer Enhancement: The
StandardRPCSerializernow includes logic to explicitly handle and deserializeundefineddata, allowing RPC endpoints to be called without any arguments. - New Test Case: A new test has been added to
standardRPCSerializer.test.tsto ensure that the serializer correctly deserializesundefinedinput.
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 or fill out our survey 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.
Code Review
This pull request aims to allow RPC calls without data by enabling the client-side RPC serializer to handle undefined during deserialization. The changes include updating the documentation to reflect this, adding a test case for deserializing undefined, and modifying the StandardRPCSerializer.
The changes are generally good, but I've identified a couple of areas for improvement:
- The documentation update could be more explicit about the purpose of the change.
- There's a potential crash in the deserializer when handling
nullinput, which is closely related to theundefinedhandling being added.
I've provided specific suggestions for both points in the review comments.
More templates
@orpc/arktype
@orpc/client
@orpc/contract
@orpc/experimental-durable-event-iterator
@orpc/hey-api
@orpc/json-schema
@orpc/json-schema-typed
@orpc/nest
@orpc/openapi
@orpc/openapi-client
@orpc/otel
@orpc/react
@orpc/react-query
@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
🔭 Outside diff range comments (1)
packages/client/src/adapters/standard/rpc-serializer.ts (1)
80-103: Server-side RPCCodec: addundefinedguard before deserializing
The client’sStandardRPCSerializernow correctly handlesundefined, but the server counterpart inpackages/server/src/adapters/standard/rpc-codec.tsdoes not. If an RPC is called without arguments,await request.body()may beundefined, leading to an unexpected error when callingthis.serializer.deserialize(undefined).Please update as follows:
• File:packages/server/src/adapters/standard/rpc-codec.ts
• Location: around thedeserializecall (currently line 19)--- packages/server/src/adapters/standard/rpc-codec.ts @@ async decode(request) { - const serialized = await request.body(); - return this.serializer.deserialize(serialized); + const serialized = await request.body(); + if (serialized === undefined) { + return undefined; + } + return this.serializer.deserialize(serialized); }Optionally, review the OpenAPI server codec in
packages/openapi/src/adapters/standard/openapi-codec.ts—its body path also callsthis.serializer.deserialize(await request.body())without anundefinedcheck.
🧹 Nitpick comments (5)
apps/content/docs/advanced/rpc-protocol.md (1)
18-26: Provide context foros.handlerin the snippetThe snippet uses
os.handlerwithout showing whereoscomes from. Add a one-line import or a short note to avoid confusion.Would you confirm the intended import? For example:
// e.g., if this is correct for your docs: import { os } from '@orpc/standard-server'packages/client/src/adapters/standard/rpc-serializer.test.ts (2)
197-200: Nit: improve test name phrasingConsider renaming for readability.
-it('standardRPCSerializer support deserialize undefined data', () => { +it('standardRPCSerializer supports deserializing undefined data', () => {
197-200: Consider adding a complementary round-trip test forserialize(undefined)To lock in expectations documented in the serializer comment, add a test asserting that:
serialize(undefined)returns an object (notundefined), and- deserializing that object yields
undefined.This helps ensure future changes don’t inadvertently alter the serialization shape for no-arg calls.
I can draft this test if helpful.
packages/client/src/adapters/standard/rpc-serializer.ts (2)
81-90: Docstring polishMinor clarity tweak.
-/** - * Only deserializing undefined is supported, while the return of serialize(undefined) is always an object. - * This is for supporting calling RPC endpoints without arguments. - * - * @todo Consider supporting serialize(undefined) -> undefined in the future - */ +/** + * Only deserialization of `undefined` is special-cased here; `serialize(undefined)` always returns an object. + * This supports calling RPC endpoints without arguments. + * + * @todo Consider supporting `serialize(undefined) -> undefined` in the future + */
80-103: Optional: tighten the private method parameter typeIf convenient, consider
#deserialize(data: unknown)and then narrow. This improves type safety at the cost of some type guards/casts. Not required for this PR.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/content/docs/advanced/rpc-protocol.md(1 hunks)packages/client/src/adapters/standard/rpc-serializer.test.ts(1 hunks)packages/client/src/adapters/standard/rpc-serializer.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
packages/client/src/adapters/standard/rpc-serializer.test.ts (1)
packages/client/src/adapters/standard/rpc-serializer.ts (1)
StandardRPCSerializer(7-104)
packages/client/src/adapters/standard/rpc-serializer.ts (1)
packages/openapi-client/src/adapters/standard/openapi-serializer.ts (1)
data(34-69)
⏰ 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: test
- GitHub Check: publish-commit
- GitHub Check: lint
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
apps/content/docs/advanced/rpc-protocol.md (1)
15-16: Good: Routing example without data aligns with no-argument RPC callsShows that calling a procedure via path alone is valid. This complements the new undefined-deserialization support.
packages/client/src/adapters/standard/rpc-serializer.test.ts (1)
197-200: Covers the undefined path—LGTMThe test correctly asserts that
deserialize(undefined)returnsundefined, matching the new behavior.packages/client/src/adapters/standard/rpc-serializer.ts (1)
81-90: Correct and necessary: safely handleundefinedinput in#deserializeEarly-returning
undefinedavoids runtime errors and enables no-argument RPC calls. This aligns with the PR objective.
1 participant
undefineddataundefinedas valid deserialize dataSummary by CodeRabbit
Bug Fixes
Documentation
Tests