Skip to content

feat(client, server): batch request/response plugin#329

Merged
dinwwwh merged 31 commits intomainfrom
feat/client-server/batch-plugin
Apr 8, 2025
Merged

feat(client, server): batch request/response plugin#329
dinwwwh merged 31 commits intomainfrom
feat/client-server/batch-plugin

Conversation

@dinwwwh
Copy link
Copy Markdown
Member

@dinwwwh dinwwwh commented Apr 4, 2025

Summary by CodeRabbit

  • New Features

    • Introduced a Batch Request/Response capability that improves efficiency by enabling multiple requests to be processed together.
    • Updated the feature comparison to show full support for batch operations in the oRPC framework.
    • Added a new entry in the documentation sidebar for the Batch Request/Response Plugin.
  • Documentation

    • Added comprehensive documentation outlining how to configure and use the new batch processing feature, now accessible via an updated sidebar entry.
  • Tests

    • Expanded test coverage to ensure robust and reliable handling of batched requests across both client and server environments.

@vercel
Copy link
Copy Markdown

vercel Bot commented Apr 4, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
orpc ✅ Ready (Inspect) Visit Preview 💬 Add feedback Apr 8, 2025 2:41am

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Apr 4, 2025

Walkthrough

This pull request introduces a Batch Request/Response feature across multiple parts of the system. It adds new documentation and sidebar entries, enhances client-side functionality with a BatchLinkPlugin for batching requests, and implements a BatchHandlerPlugin on the server for processing these batched requests. Standard server modules related to batch request creation, parsing, and response handling have been added along with accompanying tests. Additionally, a new helper function (splitInHalf) and its tests have been added to the shared utilities.

Changes

Files/Directory Group Change Summary
apps/content/.vitepress/config.ts, apps/content/docs/comparison.md, apps/content/docs/plugins/batch-request-response.md Added new sidebar entry, updated the comparison table status for oRPC, and introduced new documentation for the Batch Request/Response Plugin.
packages/client/src/plugins/batch.ts, packages/client/src/plugins/batch.test.ts, packages/client/src/plugins/index.ts, playgrounds/nextjs/src/lib/orpc.ts Introduced BatchLinkPlugin for client-side request batching with test coverage and updated exports and configuration in the Next.js playground.
packages/server/src/plugins/batch.ts, packages/server/src/plugins/batch.test.ts, packages/server/src/plugins/index.ts, playgrounds/nextjs/src/app/rpc/[[...rest]]/route.ts Added BatchHandlerPlugin for server-side batch processing along with comprehensive tests and updated exports and playground configuration.
packages/standard-server/package.json, packages/standard-server/src/batch/* (including index.ts, request.ts, response.ts, signal.ts, and their tests) Created a new batch module for the standard server with functions to create/parse batch requests and responses, manage abort signals, and updated the export configuration.
packages/shared/src/array.ts, packages/shared/src/array.test.ts Modified the signature of toArray and added a new helper splitInHalf function with corresponding tests.

Sequence Diagram(s)

sequenceDiagram
    participant C as Client
    participant BL as BatchLinkPlugin
    participant N as Network
    participant BH as BatchHandlerPlugin
    participant S as Server

    C->>BL: Initiate individual requests
    BL->>BL: Enqueue & group requests for batching
    BL->>N: Dispatch batched request
    N->>BH: Forward batched request
    BH->>S: Process each batched request
    S-->>BH: Return individual responses
    BH-->>N: Assemble batch response
    N-->>BL: Deliver batch response
    BL->>C: Resolve individual request promises
Loading

Possibly related PRs

  • unnoq/orpc#252: Addresses similar Batch Request feature enhancements in the oRPC comparison table and navigation updates, indicating a strong code-level connection with this PR.

Poem

I'm a rabbit with delight, hopping through the code so bright,
With batches here and batches there, requests group together without a care.
A plugin here, a test there, streamlining processes everywhere,
In docs and code, the changes shine, making flows so neat and fine,
Hoppity hops and a joyful cheer, for improvements that make our work clear!
🐇🚀 Happy batching, far and near!


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 4, 2025

Codecov Report

Attention: Patch coverage is 99.43020% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/client/src/plugins/index.ts 0.00% 0 Missing and 1 partial ⚠️
packages/server/src/plugins/index.ts 0.00% 0 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented Apr 4, 2025

More templates

@orpc/arktype

npm i https://pkg.pr.new/@orpc/arktype@329

@orpc/client

npm i https://pkg.pr.new/@orpc/client@329

@orpc/openapi

npm i https://pkg.pr.new/@orpc/openapi@329

@orpc/contract

npm i https://pkg.pr.new/@orpc/contract@329

@orpc/openapi-client

npm i https://pkg.pr.new/@orpc/openapi-client@329

@orpc/react

npm i https://pkg.pr.new/@orpc/react@329

@orpc/react-query

npm i https://pkg.pr.new/@orpc/react-query@329

@orpc/server

npm i https://pkg.pr.new/@orpc/server@329

@orpc/shared

npm i https://pkg.pr.new/@orpc/shared@329

@orpc/solid-query

npm i https://pkg.pr.new/@orpc/solid-query@329

@orpc/standard-server

npm i https://pkg.pr.new/@orpc/standard-server@329

@orpc/standard-server-fetch

npm i https://pkg.pr.new/@orpc/standard-server-fetch@329

@orpc/standard-server-node

npm i https://pkg.pr.new/@orpc/standard-server-node@329

@orpc/svelte-query

npm i https://pkg.pr.new/@orpc/svelte-query@329

@orpc/valibot

npm i https://pkg.pr.new/@orpc/valibot@329

@orpc/vue-colada

npm i https://pkg.pr.new/@orpc/vue-colada@329

@orpc/vue-query

npm i https://pkg.pr.new/@orpc/vue-query@329

@orpc/zod

npm i https://pkg.pr.new/@orpc/zod@329

commit: bcee326

@dinwwwh dinwwwh marked this pull request as ready for review April 8, 2025 02:29
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (10)
playgrounds/nextjs/src/lib/orpc.ts (1)

13-20:

Details

❓ Verification inconclusive

Consider refining the batch condition logic for more targeted batching.

The current implementation batches all eligible requests together since the condition always returns true. While this works, consider if more specific batching groups would be more appropriate for your use case.

You could refine the batching strategy by grouping related requests together:

-    new BatchLinkPlugin({
-      groups: [{
-        condition: () => true,
-        context: {},
-      }],
-    }),
+    new BatchLinkPlugin({
+      groups: [
+        {
+          // Group read operations
+          condition: (options) => ['GET', 'HEAD'].includes(options.request.method),
+          context: {},
+        },
+        {
+          // Group write operations
+          condition: (options) => ['POST', 'PUT', 'PATCH'].includes(options.request.method),
+          context: {},
+        }
+      ],
+    }),

Let's verify what types of requests are typically made in the application to determine if targeted batching would be beneficial:


🏁 Script executed:

#!/bin/bash
# Find all RPC method calls to understand the request patterns
rg "orpcClient\." --type ts -A 2 -B 2

Length of output: 39


Targeted Batching for BatchLinkPlugin

The current implementation in playgrounds/nextjs/src/lib/orpc.ts uses an unconditional batching strategy (condition: () => true), which groups all requests together. The proposed diff suggests splitting the batching logic into separate groups for read operations (e.g., GET, HEAD) and write operations (e.g., POST, PUT, PATCH), which could improve efficiency if the request patterns warrant such separation.

-    new BatchLinkPlugin({
-      groups: [{
-        condition: () => true,
-        context: {},
-      }],
-    }),
+    new BatchLinkPlugin({
+      groups: [
+        {
+          // Group read operations
+          condition: (options) => ['GET', 'HEAD'].includes(options.request.method),
+          context: {},
+        },
+        {
+          // Group write operations
+          condition: (options) => ['POST', 'PUT', 'PATCH'].includes(options.request.method),
+          context: {},
+        }
+      ],
+    }),

However, our search for typical RPC method usage (via rg "orpcClient.") did not yield any results, so the evidence for distinct request patterns is inconclusive. Please manually verify the request patterns in the application to ensure that differentiating between read and write operations is beneficial. If the codebase does distinguish request types, the refined strategy could lead to improved batching; otherwise, the existing unconditional approach may suffice.

packages/standard-server/src/batch/request.test.ts (2)

21-57: Consider testing an empty requests array scenario.

The “method GET” test covers typical usage. You might also verify behavior when the requests array is empty (i.e., no sub-requests). This ensures that parseBatchRequest can handle edge cases gracefully.


59-95: Extend tests to cover additional edge cases.

Similarly, for the “method POST” test, consider adding a scenario with zero sub-requests to confirm expected handling of empty arrays.

packages/standard-server/src/batch/signal.ts (1)

6-20: Revisit the all-or-nothing abort logic.

Currently, the controller aborts only if all provided signals are aborted. If the intended behavior is to halt as soon as one signal aborts, consider short-circuiting on the first abort for improved responsiveness. Otherwise, confirm that the all-or-nothing approach is desired.

packages/shared/src/array.ts (1)

1-3: Refine the type assertion for stronger type safety.

The as any assertion can mask potential type mismatches. Consider removing or narrowing the assertion to ensure stricter type checks, unless you rely on broad flexibility.

packages/server/src/plugins/batch.ts (1)

60-149: Large async logic block in init could benefit from modularization.
While the current logic is comprehensive — parsing requests, verifying batch size, dispatching sub-requests, and assembling a streaming response — it’s quite lengthy. Consider extracting chunks of logic (e.g., batch-size checking, request dispatch, response streaming) into helper functions or internal methods. This refactor could improve readability, maintainability, and reusability.

packages/standard-server/src/batch/response.ts (1)

12-14: Consider adding a clarifying comment.

While the implementation is correct, the toBatchResponse function is just a pass-through that returns the input options. Adding a comment explaining the purpose of this function would improve code readability.

 export function toBatchResponse(options: ToBatchResponseOptions): StandardResponse {
+  // This function serves as a type converter and API consistency marker
+  // It makes the API more explicit and allows for future extension
   return options
 }
apps/content/docs/plugins/batch-request-response.md (2)

60-60: Add a hyphen or clarify “auto-fall back.”

“Requests will auto fall back” may be unclear or grammatically incorrect. For better clarity and style, consider adding a hyphen or rephrasing:

- The plugin does not support [AsyncIteratorObject](...) in responses (requests will auto fall back to the default behavior).
+ The plugin does not support [AsyncIteratorObject](...) in responses (requests will auto-fall back to the default behavior).
🧰 Tools
🪛 LanguageTool

[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...data-types) in responses (requests will auto fall back to the default behavior). To exclu...

(AUTO_HYPHEN)


112-112: Fix subject-verb agreement.

“headers” is plural, so the phrase should read “response headers are empty.” Suggest updating the text:

- By default, the response headers is empty.
+ By default, the response headers are empty.
🧰 Tools
🪛 LanguageTool

[grammar] ~112-~112: The verb form ‘is’ does not seem to match the subject ‘headers’.
Context: ...aders By default, the response headers is empty. To customize headers, use the `h...

(SUBJECT_VERB_AGREEMENT_PLURAL)

packages/client/src/plugins/batch.ts (1)

125-139: Consider removing the header instead of setting it to undefined.

Setting 'x-orpc-batch': undefined may keep the header key in some client libraries, which can cause unexpected behavior. Consider deleting it explicitly when not needed:

- 'x-orpc-batch': undefined,
+ // Remove the batch header altogether:
+ ...Object.fromEntries(Object.entries(options.request.headers).filter(([k]) => k !== 'x-orpc-batch')),
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 25532a8 and 5826ba4.

📒 Files selected for processing (21)
  • apps/content/.vitepress/config.ts (1 hunks)
  • apps/content/docs/comparison.md (1 hunks)
  • apps/content/docs/plugins/batch-request-response.md (1 hunks)
  • packages/client/src/plugins/batch.test.ts (1 hunks)
  • packages/client/src/plugins/batch.ts (1 hunks)
  • packages/client/src/plugins/index.ts (1 hunks)
  • packages/server/src/plugins/batch.test.ts (1 hunks)
  • packages/server/src/plugins/batch.ts (1 hunks)
  • packages/server/src/plugins/index.ts (1 hunks)
  • packages/shared/src/array.test.ts (1 hunks)
  • packages/shared/src/array.ts (1 hunks)
  • packages/standard-server/package.json (1 hunks)
  • packages/standard-server/src/batch/index.ts (1 hunks)
  • packages/standard-server/src/batch/request.test.ts (1 hunks)
  • packages/standard-server/src/batch/request.ts (1 hunks)
  • packages/standard-server/src/batch/response.test.ts (1 hunks)
  • packages/standard-server/src/batch/response.ts (1 hunks)
  • packages/standard-server/src/batch/signal.test.ts (1 hunks)
  • packages/standard-server/src/batch/signal.ts (1 hunks)
  • playgrounds/nextjs/src/app/rpc/[[...rest]]/route.ts (1 hunks)
  • playgrounds/nextjs/src/lib/orpc.ts (1 hunks)
🧰 Additional context used
🧬 Code Definitions (9)
packages/standard-server/src/batch/signal.test.ts (1)
packages/standard-server/src/batch/signal.ts (1)
  • toBatchAbortSignal (1-23)
packages/server/src/plugins/batch.ts (5)
packages/server/src/context.ts (1)
  • Context (1-1)
packages/shared/src/value.ts (2)
  • Value (3-3)
  • value (5-14)
packages/standard-server/src/types.ts (2)
  • StandardRequest (13-24)
  • StandardHeaders (1-3)
packages/standard-server/src/batch/response.ts (2)
  • BatchResponseBodyItem (4-6)
  • toBatchResponse (12-14)
packages/standard-server/src/batch/request.ts (1)
  • parseBatchRequest (40-58)
packages/client/src/plugins/batch.test.ts (5)
packages/standard-server/src/batch/response.ts (1)
  • toBatchResponse (12-14)
packages/client/src/adapters/standard/link.ts (2)
  • path (53-65)
  • StandardLink (28-66)
packages/standard-server/src/types.ts (1)
  • StandardRequest (13-24)
packages/client/src/plugins/batch.ts (2)
  • BatchLinkPlugin (61-269)
  • method (193-268)
packages/shared/src/iterator.ts (1)
  • isAsyncIteratorObject (1-7)
playgrounds/nextjs/src/lib/orpc.ts (2)
packages/client/src/adapters/fetch/rpc-link.ts (1)
  • RPCLink (10-19)
packages/client/src/plugins/batch.ts (1)
  • BatchLinkPlugin (61-269)
playgrounds/nextjs/src/app/rpc/[[...rest]]/route.ts (1)
packages/server/src/plugins/batch.ts (1)
  • BatchHandlerPlugin (39-150)
packages/standard-server/src/batch/response.ts (3)
packages/standard-server/src/types.ts (1)
  • StandardResponse (34-41)
packages/shared/src/iterator.ts (1)
  • isAsyncIteratorObject (1-7)
packages/shared/src/object.ts (1)
  • isObject (31-39)
packages/server/src/plugins/batch.test.ts (4)
packages/server/src/plugins/batch.ts (1)
  • BatchHandlerPlugin (39-150)
packages/standard-server/src/types.ts (2)
  • StandardRequest (13-24)
  • StandardLazyRequest (26-32)
packages/standard-server/src/batch/request.ts (1)
  • toBatchRequest (12-38)
packages/standard-server/src/batch/response.ts (1)
  • parseBatchResponse (16-41)
packages/standard-server/src/batch/request.ts (3)
packages/standard-server/src/types.ts (2)
  • StandardHeaders (1-3)
  • StandardRequest (13-24)
packages/shared/src/json.ts (2)
  • stringifyJSON (9-12)
  • parseEmptyableJSON (1-7)
packages/standard-server/src/batch/signal.ts (1)
  • toBatchAbortSignal (1-23)
packages/client/src/plugins/batch.ts (8)
packages/client/src/adapters/standard/link.ts (3)
  • StandardLinkClientInterceptorOptions (18-20)
  • StandardLinkPlugin (9-11)
  • StandardLinkOptions (22-26)
packages/shared/src/value.ts (2)
  • Value (3-3)
  • value (5-14)
packages/standard-server/src/types.ts (3)
  • StandardHeaders (1-3)
  • StandardRequest (13-24)
  • StandardLazyResponse (43-49)
packages/shared/src/interceptor.ts (1)
  • InterceptorOptions (6-12)
packages/shared/src/iterator.ts (1)
  • isAsyncIteratorObject (1-7)
packages/shared/src/array.ts (2)
  • splitInHalf (5-8)
  • toArray (1-3)
packages/standard-server/src/batch/request.ts (1)
  • toBatchRequest (12-38)
packages/standard-server/src/batch/response.ts (1)
  • parseBatchResponse (16-41)
🪛 LanguageTool
apps/content/docs/plugins/batch-request-response.md

[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...data-types) in responses (requests will auto fall back to the default behavior). To exclu...

(AUTO_HYPHEN)


[grammar] ~112-~112: The verb form ‘is’ does not seem to match the subject ‘headers’.
Context: ...aders By default, the response headers is empty. To customize headers, use the `h...

(SUBJECT_VERB_AGREEMENT_PLURAL)

🔇 Additional comments (38)
packages/server/src/plugins/index.ts (1)

1-1: Export changes look correct

This addition properly exports the batch plugin module, following the consistent pattern used for other plugin exports in this file.

packages/client/src/plugins/index.ts (1)

1-1: Export looks good

The batch plugin export follows the same pattern as the existing retry plugin export, maintaining consistency in the module structure.

apps/content/.vitepress/config.ts (1)

112-112: Documentation entry added appropriately

The new sidebar entry for Batch Request/Response is correctly placed in the Plugins section and follows the same format as other plugin entries.

apps/content/docs/comparison.md (1)

36-36: Feature status updated accurately

The status for Batch Request has been correctly updated to indicate first-class built-in support in oRPC, which aligns with the implementation of the new batch functionality.

packages/standard-server/src/batch/index.ts (1)

1-3: LGTM! Clean and concise index file export.

This file serves as a well-organized entry point for batch functionality, making it easy to import all batch-related features from a single location.

playgrounds/nextjs/src/app/rpc/[[...rest]]/route.ts (2)

5-5: LGTM! Good addition of the BatchHandlerPlugin import.

Clean import statement for the new batch handling functionality.


13-15:

Details

✅ Verification successful

BatchHandlerPlugin integration looks good, consider documenting default settings.

The BatchHandlerPlugin is correctly integrated into the RPCHandler, enabling batch request processing on the server side. The plugin is using default configuration settings (maxSize: 10, successStatus: 207).

If you anticipate higher batch volumes, you might want to consider customizing the maxSize:


🏁 Script executed:

#!/bin/bash
# Search for any existing batch size limits or configuration patterns in the codebase
rg -A 5 -B 5 "new BatchHandlerPlugin\(" --type ts

Length of output: 5119


BatchHandlerPlugin integration verified; consider clarifying default settings in documentation

The integration in playgrounds/nextjs/src/app/rpc/[[...rest]]/route.ts correctly instantiates the plugin using its default configuration (implicitly using maxSize: 10 and successStatus: 207). This aligns with the setup seen in test files (e.g., packages/server/src/plugins/batch.test.ts and packages/client/src/plugins/batch.test.ts), where both default and customized configurations (such as { maxSize: 2 }) are demonstrated.

For improved clarity and maintainability, please consider adding inline documentation or updating your project docs to clearly state these default values. This will help future developers understand the behavior of the BatchHandlerPlugin—especially if adjustments are needed when handling higher batch volumes.

packages/shared/src/array.test.ts (2)

1-1: LGTM! Import looks good.

Correctly importing both the existing toArray and new splitInHalf functions.


10-17: Test cases for splitInHalf are comprehensive and well-structured.

The test suite covers all essential scenarios, including:

  • Arrays with odd/even number of elements
  • Edge cases (arrays with 1 element and empty arrays)
  • The behavior is consistent with what would be expected from a function named splitInHalf
playgrounds/nextjs/src/lib/orpc.ts (1)

6-6: LGTM! Good addition of the BatchLinkPlugin import.

Clean import statement for the client-side batch functionality.

packages/standard-server/src/batch/request.test.ts (2)

1-9: Looks good overall.

Imports, mocks, and initial setup are clear and concise. Test isolation via vi.clearAllMocks() is correctly implemented.


98-116: Comprehensive negative test coverage.

The invalid batch request checks (missing body, wrong method, etc.) are well-handled, ensuring robust error handling.

packages/shared/src/array.ts (1)

5-8: Neat approach to splitting arrays.

Using Math.ceil() ensures the first half includes any leftover element when array length is odd. This function is straightforward and seems correct.

packages/standard-server/package.json (2)

21-25: New batch export entries look consistent.
All paths for types, import, and default appear correct and match the newly added batch module structure.


30-31: New public export path for ./batch is aligned with the source structure.
These lines correctly reference ./src/batch/index.ts for the new batch functionality.

packages/standard-server/src/batch/signal.test.ts (3)

3-20: Excellent coverage for partially aborted signals.
The test scenarios (empty array, undefined, partially aborted signals) thoroughly validate that the returned signal is not aborted when not all inputs are aborted. Keep it up!


22-30: Validates the scenario where all input signals are already aborted.
This test confirms that the returned signal is immediately aborted if all signals were aborted beforehand. Great job on ensuring edge-case coverage.


32-58: Abort event handling is well-tested.
Verifies that the abort event is fired only after the last non-aborted signal is aborted. The usage of vi.fn() is appropriate for spying on the event listener.

packages/server/src/plugins/batch.ts (2)

9-37: Extensive and flexible BatchHandlerOptions interface.
The documentation (e.g., @default tags) for each property is well-written, and the optional fields with reasonable defaults demonstrate good design.


39-58: Constructor defaults are aptly handled.
Using ?? operators to supply fallback values for maxSize, mapRequestItem, successStatus, and headers ensures robust handling, reducing risk of undefined behavior.

packages/standard-server/src/batch/response.test.ts (2)

5-30: Test suite looks thorough and well-structured.

The test cases effectively validate both toBatchResponse and parseBatchResponse functions, checking for correct status codes, headers, and ensuring the async iterator properly yields the expected batch response items.


32-54: Good error handling tests.

These tests properly verify that parseBatchResponse throws the appropriate errors for invalid inputs, checking both non-iterable bodies and malformed batch items.

packages/standard-server/src/batch/response.ts (1)

16-41: Implementation looks solid.

The parseBatchResponse function properly validates that the response body is an async iterator and that each item has the required properties. The use of a finally block to clean up resources by calling body.return?() is a good practice.

packages/client/src/plugins/batch.test.ts (6)

21-136: Well-structured batch plugin tests.

The tests thoroughly verify that the BatchLinkPlugin correctly batches multiple requests into a single batch request, properly handles response parsing, and maintains correct context and signal propagation.


138-201: Good coverage of edge cases.

These tests properly verify that the plugin:

  1. Doesn't batch unsupported body types like FormData and AsyncIterator
  2. Processes requests individually when no matching group is found
  3. Maintains proper request isolation and context

This ensures the batching mechanism only activates when appropriate.


203-219: Error handling correctly tested.

Tests verify that invalid batch responses properly propagate errors to all batched requests, ensuring failures are handled gracefully.


221-291: Comprehensive batching logic tests.

These tests verify important batching features:

  1. Correct separation of GET and non-GET requests
  2. Splitting batches that exceed maximum size
  3. Handling URL length limitations

These scenarios are critical for real-world use cases with varying request types and volumes.


293-359: Good testing of additional features.

Tests verify that:

  1. The x-orpc-batch header is properly handled
  2. Exclusion rules work correctly
  3. Error handling for missing responses is appropriate

These edge cases ensure the batching mechanism is robust.


362-421: Integration testing looks good.

Testing the integration between BatchLinkPlugin and BatchHandlerPlugin is important to ensure end-to-end functionality works correctly. Both success and error scenarios are verified.

packages/server/src/plugins/batch.test.ts (6)

10-166: Comprehensive batch handler tests.

The tests thoroughly verify:

  1. Correct handling of batch requests
  2. Proper response status and headers
  3. Response parsing and validation
  4. Handling of concurrent requests with varying response times

The temporal tests (with varying sleep times) validate that responses are yielded in the order they complete rather than the order they were submitted, which is important for async behavior.


168-220: Good coverage of customization options.

Tests validate that custom success status, headers, and request mapping options work correctly, ensuring the plugin is flexible enough for various use cases.


222-256: Error handling tests look good.

These tests verify that non-batch requests are ignored and invalid batch requests result in appropriate error responses rather than exceptions.


258-283: Error propagation test is important.

Verifying that unknown errors are properly propagated ensures that developer errors aren't silently caught by the batch handler.


285-416: Good validation of error handling strategy.

Tests verify that request errors result in appropriate error responses in the batch rather than failing the entire batch. This is an important resilience feature of batch processing.


418-448: Size limit enforcement test is important.

Validating that batch size limits are enforced helps prevent potential DoS vulnerabilities from oversized batch requests.

packages/standard-server/src/batch/request.ts (2)

12-22: Validate URL property of each sub-request.

Although the code maps each sub-request item into { method, url, headers, body }, there's no check that url is a valid URL string. If a user-provided sub-request item has an invalid URL, the new URL(item.url) call in parseBatchRequest (lines 52-52) will throw a TypeError at runtime. Consider validating or catching potential parsing errors.


31-38: Double-check merged signals for concurrency.

toBatchAbortSignal merges multiple abort signals so that if any sub-request is aborted, the entire batch is aborted. Confirm that this is the intended behavior. If you only want to abort the rest of the operations but continue others independently, a different approach might be required.

packages/client/src/plugins/batch.ts (1)

143-149: Watch for unsupported request bodies in the batch.

The plugin excludes requests if the body is a Blob, FormData, or an async iterator, but other unsupported binary or streaming formats might cause failures. Verify that these are indeed the only formats you need to exclude or handle specially.

Comment on lines +40 to +58
export function parseBatchRequest(request: StandardRequest): StandardRequest[] {
const items = request.method === 'GET'
? parseEmptyableJSON(request.url.searchParams.getAll('batch').at(-1))
: request.body

if (!Array.isArray(items)) {
throw new TypeError('Invalid batch request')
}

return items.map((item) => {
return {
method: item.method,
url: new URL(item.url),
headers: item.headers,
body: item.body,
signal: request.signal,
} satisfies StandardRequest
})
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add defensive checks for request fields.

parseBatchRequest reconstructs StandardRequest objects from untrusted sources, relying on Array.isArray(items) alone. Consider additional validation on each property (e.g., method, headers) to avoid malformed requests from leading to unexpected behavior at runtime.

Comment on lines +251 to +259
for await (const item of parsed) {
batchItems[item.index]?.[1]({ ...item, body: () => Promise.resolve(item.body) })
}

/**
* JS ignore the second resolve or reject so we don't need to check if has been resolved
*/
throw new Error('Something went wrong make batch response not contains enough responses. This can be a bug please report it.')
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Prevent unconditional error throw after successful batch handling.

Currently, the code always throws an error after yielding the responses, even if all items were successfully resolved. Add a condition to check for mismatched responses or return early when everything is handled:

        yield item as unknown as BatchResponseBodyItem
      }
    }
    finally {
      await body.return?.()
    }
+   // Return early if all items were processed
+   if (batchItems.length === parsedItemCount) {
+     return
+   }

    throw new Error('Something went wrong...')

Committable suggestion skipped: line range outside the PR's diff.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (3)
apps/content/docs/plugins/batch-request-response.md (3)

14-17: Setup Section Provides Essential Overview

The setup section explains the need for configurations on both the client and server sides. Consider adding a reference link or note that elaborates on the configuration steps if further details are available in the documentation elsewhere.


20-29: Effective Server-Side Code Example

The server code snippet clearly demonstrates how to integrate the BatchHandlerPlugin with an RPC handler. However, the use of the // ---cut--- marker might confuse new users. Consider adding a brief explanation (either as a comment or in the adjacent text) that this marker indicates omitted code for brevity.


58-60: Limitations Section – Consider Rephrasing for Clarity

The explanation on limitations is clear; however, the phrase "requests will auto fall back to the default behavior" could be improved for clarity. For example, consider rephrasing to "requests will automatically fall back to the default behavior."

Proposed diff:

- in responses (requests will auto fall back to the default behavior).
+ in responses (requests will automatically fall back to the default behavior).
🧰 Tools
🪛 LanguageTool

[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...data-types) in responses (requests will auto fall back to the default behavior). To exclu...

(AUTO_HYPHEN)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5826ba4 and bcee326.

📒 Files selected for processing (1)
  • apps/content/docs/plugins/batch-request-response.md (1 hunks)
🧰 Additional context used
🪛 LanguageTool
apps/content/docs/plugins/batch-request-response.md

[uncategorized] ~60-~60: It appears that a hyphen is missing (if ‘auto’ is not used in the context of ‘cars’).
Context: ...data-types) in responses (requests will auto fall back to the default behavior). To exclu...

(AUTO_HYPHEN)

🔇 Additional comments (9)
apps/content/docs/plugins/batch-request-response.md (9)

1-4: Front Matter Metadata Looks Solid

The front matter is clean and clearly specifies the title and description for the plugin documentation. Verify that these metadata fields align with the overall documentation standards.


6-9: Clear Plugin Introduction

The introductory header and description succinctly state the purpose of the Batch Request/Response Plugin. The language is straightforward and informative.


10-12: Well-Formatted Info Block

The info block clearly communicates that the plugin streams responses asynchronously to prevent blocking. This helps set the right expectations for the reader.


31-33: Comprehensive Handler Context

The info block following the server example effectively describes the possible handler types and notes the custom protocol used by the plugin. This additional context is helpful.


35-38: Client-Side Overview is Clear

The brief description before the client code example provides a good overview of what is expected when using the BatchLinkPlugin.


39-56: Solid Client-Side Code Example

The client example precisely demonstrates how to configure the BatchLinkPlugin by defining groups and contexts. The example is easy to follow and clearly formatted.


62-82: Good Example for Excluding Unsupported Procedures

The code snippet using the exclude option demonstrates clearly how to filter out procedures that return unsupported types. The example is concise and well-commented.


84-108: Request Headers Customization is Clearly Documented

The section on Request Headers provides a straightforward example of how to customize headers for batched requests. The configuration shown is concise and appropriate.


110-127: Response Headers Example is Informative

The Response Headers section offers a clear demonstration of how to customize headers on the server side via the BatchHandlerPlugin. The example and accompanying explanation are well structured.

@dinwwwh dinwwwh merged commit 32a6de9 into main Apr 8, 2025
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant