Cap RPC streaming decoder buffers - #6802
Conversation
🦋 Changeset detectedLatest commit: 3c002a7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 30 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Bundle Size AnalysisGenerated from PR build output; treat the content below as untrusted.
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Buffer cap for NDJSON and MessagePack streaming decoders: Adds a configurable
maxBufferSize(default 16 MiB) tondjson()andmakeMsgPack(), raisingMaxBufferSizeExceededwhen an incomplete frame exceeds the limit. - Forwarding through JSON-RPC framing:
ndJsonRpcandlayerNdJsonRpcnow accept and forwardmaxBufferSizeto the underlyingndjsonparser. - Graceful socket closure on overflow: The socket protocol catches
MaxBufferSizeExceededand writes aCloseEventwith code 1009 instead of encoding a defect response and continuing to read. - API migration:
ndjsonchanged from a staticRpcSerialization["Service"]value to a factory function acceptingStreamingOptions; all internal callers (McpServer.ts,layerNdjson,ndJsonRpc) updated accordingly. - Tests: Unit-level tests for ndjson, ndJsonRpc, and makeMsgPack buffer caps, plus a socket-level integration test verifying the close-on-overflow path.
DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Switched
instanceofguard toPredicate.isTagged: The socket protocol'sMaxBufferSizeExceededcatch inRpcServer.tsnow usesPredicate.isTagged(cause, "MaxBufferSizeExceeded")instead ofinstanceof, and passesString(cause)rather thancause.messageto theCloseEvent— both following reviewer feedback and the preferred pattern for tagged-error discrimination in the codebase.
DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
- Exposed
StreamingOptionsthroughlayerNdjson,layerMsgPack,ndJsonRpc, andlayerNdJsonRpc: ConvertedlayerNdjsonandlayerMsgPackfrom staticLayerconstants to factory functions so callers can opt intomaxBufferSizeat layer construction time.layerMsgPack()with no arguments preserves the existingmsgPackdefault. AddedmaxBufferSizeforwarding throughndJsonRpcandlayerNdJsonRpc. - Updated all 14 call sites in core and platform packages: Mechanical
layerNdjson→layerNdjson()andlayerMsgPack→layerMsgPack()across Bun, Deno, Node cluster modules and their tests. - Added layer-forwarding tests: New
layerNdjson,layerMsgPack, andndJsonRpctests verify thatmaxBufferSizereaches the underlying decoder when provided through the layer or framing functions.
DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) ｜ 𝕏
|
Aligned the v4 API with merged #6803 in 9cdd882: restored Validation is clean: 45 Effect RPC/MCP tests, 97 Node RPC/cluster tests, targeted formatting/lint, and the full TypeScript check. |
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
The new commit aligns the API shape with the naming conventions used elsewhere in the codebase, so ndjson and layerNdjson remain backward-compatible while the configurable variants sit behind make* and *With names.
- Restored
ndjson,layerNdjson, andlayerMsgPackas constants: The previous commit converted these to factory functions, which is a compile-time break.makeNdjson,layerNdjsonWith, andlayerMsgPackWithare the new factory entry points; the original constants are back to being plain values calling the defaults. - Added
"unbounded"as a validmaxBufferSize: A sentinel string that explicitly disables the buffer limit. TheisBufferSizeExceededpredicate uses a type guard (maxBufferSize is number) to narrow correctly when the limit applies. - Forwarded
serializationMaxBufferSizethrough six cluster layers: Each platform's HTTP and Socket cluster layer now accepts an optionalserializationMaxBufferSizeoption and passes it to the appropriate*Withlayer factory. - Added unbounded-regression tests: New test cases for both NDJSON and MessagePack verify that
"unbounded"buffers large incomplete frames without erroring. - Added cluster changeset: Patch-level changeset for
@effect/platform-node,@effect/platform-bun, and@effect/platform-denodocumenting the newserializationMaxBufferSizeoption.
DeepSeek Pro (free via Pullfrog for OSS) (Kimi K2 not used — the program covers this model; add its provider key to run your pick) ｜ 𝕏
1 participant
Summary
ndjson,layerNdjson, andlayerMsgPackvalues while addingmakeNdjson,layerNdjsonWith, andlayerMsgPackWithfor custom limitsmaxBufferSize: "unbounded"as an explicit opt-outRoot cause
The framed decoders retained incomplete NDJSON and MessagePack input in per-connection closure state without any upper bound. Socket transports caught decoder failures as ordinary defects, wrote a response, and continued reading from the same connection.
Impact
Incomplete frames can no longer grow a connection's parser state without bound. Applications can tune or explicitly disable the limit per serializer and through cluster layers, while existing value-based call sites retain their source-compatible API and receive the safe default.
Validation
nix develop -c pnpm checknix develop -c pnpm test packages/effect/test/rpc packages/effect/test/unstable/ai/McpServer/McpServer.test.ts --run(45 tests)nix develop -c pnpm test packages/platform-node/test/RpcServer.test.ts packages/platform-node/test/cluster/SocketRunner.test.ts --run(97 tests)dprintandoxlintchecksCloses EFF-258