Implement jsrpc serialization/deserialization for TS web streams - #7073
Open
jasnell wants to merge 14 commits into
Open
Implement jsrpc serialization/deserialization for TS web streams#7073jasnell wants to merge 14 commits into
jasnell wants to merge 14 commits into
Conversation
Contributor
|
LGTM |
jasnell
commented
Aug 19, 2026
Collaborator
Author
|
@kentonv ... just a mention for you here since you wrote the original streams ser/deser code. It gets a bit more complicated with the two possible paths and the fact that JS classes can't be serialized. There's a lot here and I'm not expecting you to review but wanted to at least give a heads up. |
Merging this PR will not alter performance
Comparing 
Footnotes
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #7073 +/- ##
==========================================
+ Coverage 67.68% 67.74% +0.05%
==========================================
Files 458 458
Lines 129315 129846 +531
Branches 21146 21232 +86
==========================================
+ Hits 87533 87966 +433
- Misses 28947 29003 +56
- Partials 12835 12877 +42 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
jasnell
marked this pull request as ready for review
August 19, 2026 23:37
Collaborator
Author
|
/bonk check it again |
![ask-bonk[bot]](https://avatars.githubusercontent.com/in/2454580?s=60&v=4){kind=small}
Contributor
|
Posted 1 inline finding. Time for a pun! That writer needs a more reliable draft. |
Collaborator
Author
|
/bonk check again |
Contributor
|
LGTM! |
This comment was marked as resolved.
This comment was marked as resolved.
jasnell
marked this pull request as draft
August 20, 2026 03:15
jasnell
force-pushed
the
jasnell/streams-fun-phase2
branch
from
August 20, 2026 18:06
6810a2a to
d79c744
Compare
jasnell
force-pushed
the
jasnell/streams-fun-phase2
branch
2 times, most recently
from
August 20, 2026 18:58
e530c1b to
d9ca73a
Compare
Objects implemented in JavaScript (such as the TypeScript-implemented stream classes) have no JSG wrapper, so the serializer cannot recognize them the way it recognizes host objects. Give the ExternalHandler a trySerializeClassInstance() virtual, called from WriteHostObject() for class instances before falling through to DataCloneError, so a handler can recognize such objects by brand check and serialize them as externals. The default implementation declines, preserving existing behavior. Relatedly, let DeserializeInvoker accept deserialize() implementations whose return type wraps to a generic v8::Local<v8::Value> (custom wrappers like JsReadableStream), enforcing at runtime that the wrapped result is an object.
Serializing a stream for RPC transfer preserves the underlying source's preferred encoding (e.g. gzip passthrough without a recompression round trip), which requires the bridge to expose the preference for both backends. The legacy arm forwards through the controller; the TypeScript arm reaches the underlying ReadableStreamNativeSource through the same non-detaching accessor as the encoding-aware tryGetLength arm, answering IDENTITY for JS-sourced (queued) streams. Like tryGetLength, the native source reports IDENTITY while identity bytes are stashed: the remainder is then not entirely in the source's preferred encoding.
Extract the RPC wire plumbing from ReadableStream::serialize into newReadableStreamSerializeSink -- requiring an RPC-backed serializer, pushing the ByteStream to the peer, and writing the external-table entry (encoding + expected length) -- and share it between the legacy serialize and the new TypeScript arm. The arms differ only in how the encoding and expected length are obtained (controller vs. the bridge's getPreferredEncoding/tryGetLength dispatch) and in which pumpTo drives the transfer; lock/disturb validation stays in pumpTo for both, and the TypeScript arm preserves encoding passthrough exactly like the legacy path.
Expose the WritableStreamRpcAdapter revoke machinery from streams/writable.c++ through a newWritableStreamRpcAdapter factory (adopted by the legacy native-sink arm as well), and drive it from a new TsWriterSink: a WritableStreamSink view of the TypeScript stream's writer that enters the isolate per operation, the counterpart of the legacy WritableStreamJsRpcAdapter. The TypeScript arm acquires the writer (which performs the lock validation), wires the sink through the shared adapter with the same revoke-on-IoContext-end and abort-on-disconnect semantics, and transfers in IDENTITY encoding like the legacy JavaScript-backed arm. Extracting a wrapped native sink for encoding-aware transfer is left as a future optimization.
TypeScript-implemented ReadableStream/WritableStream instances are plain JS class instances with no JSG wrapper, so the serializer cannot route them to the stream serialization functions the way it routes the legacy JSG-wrapped streams. Implement the ExternalHandler's trySerializeClassInstance hook on RpcSerializerExternalHandler: recognize the streams by brand check, then write the same serialization tag and wire form the legacy streams use, so the receiving side needs no knowledge of which implementation the sender runs.
ReadableStream::deserialize and WritableStream::deserialize now return JsReadableStream/JsWritableStream and construct the received stream through the bridges' create(), which dispatches on the typescript_implemented_streams compat flag: under the flag a received stream is TypeScript-backed (and an instance of the global stream class), while legacy isolates construct exactly what they did before. The wire protocol is unchanged, so either implementation can send to either.
V8's deserializer forbids JavaScript execution for the entire value graph read (v8::internal::DisallowJavascriptExecution in ValueDeserializer::ReadObject), but constructing a TypeScript-implemented stream means running its class constructor. Constructing received streams inside ReadableStream::deserialize / WritableStream::deserialize therefore cannot work for the TypeScript implementation. Exploit the fact that the externals table arrives complete before deserialization begins: RpcDeserializerExternalHandler::prepare(), called before readValue(), materializes each stream external -- the full construction the deserialize functions previously performed, through the bridges' implementation-dispatching create() -- into per-external slots, and the deserialize functions claim the prebuilt streams without executing JavaScript. Slots record how many externals each value subsumes so multi-external values (sockets) can join the same mechanism. The pre-pass is gated on the new rpc-externals-hydration autogate and runs for both stream implementations, so it can be verified in production on legacy traffic before the gate is retired in favor of the single hydrated path. With the gate off, deserialization constructs legacy streams in place exactly as before the gate existed (under the experimental TypeScript streams flag that configuration degrades to legacy-backed received streams).
Streams received over RPC flow through brand checks and state probes while V8's deserializer still forbids JavaScript execution: unwrap (e.g. of a Request initializer's body) brand-checks the value, and API constructors reached from deserialize functions validate stream state (Body's disturbed check, lock checks). Both previously dispatched into the TypeScript implementation, executing JS. Recognition: the TypeScript constructors now stamp an own, non-enumerable api-symbol brand on every instance, and tryUnwrapTs probes for it -- an own-data-property read, no JS. Proxies are rejected up front (an own-property probe on a proxy would invoke its traps), matching the #-brand's deliberate no-tunneling behavior. This also removes a JS call from every unwrap attempt, the optimization the unwrap perf notes had sketched. State probes: isDisturbed/isLocked answer false without dispatching when JS execution is disallowed. The only no-JS scope in which TypeScript-backed streams are reachable is RPC deserialization, and every stream reachable there is hydration-fresh: constructed by the externals pre-pass within the same task, never seen by user code, with no transition mechanism available inside the scope. As a backstop, the bridge's dispatch helpers now assert that JS execution is allowed, turning any missed probe into a named failure instead of an unsymbolizable V8 fatal. The js-rpc-streams-ts-test config runs the entire js-rpc-test suite with the TypeScript streams implementation and the hydration autogate enabled, pinning stream transfer -- both directions, error and abort propagation, locked-stream rejection, nested Request/Response bodies -- to the same expectations the legacy implementation satisfies.
Socket::deserialize's construction was already carefully split into scope-safe kj work and deferred JS wiring for the legacy streams, but under the TypeScript streams implementation its stream construction and EOF plumbing execute JavaScript, which the deserializer's no-JS scope forbids. Move the whole construction body into hydrateRpcSocket(), called by the externals pre-pass with JavaScript legal: the socket external subsumes its two adjacent stream externals as a span-3 slot, and Socket::deserialize claims the prebuilt socket (unwrapping through its TypeHandler -- an internal-field read, no JS). With the hydration autogate off, the same body runs in place as before, and the socket-rpc-transfer kill switch keeps both its rejection semantics and its gating of hydration itself. Extend the no-JS-scope backstop to getCppExport(), the chokepoint every bridge call into the TypeScript implementation passes through -- including constructor invocations, which bypass dispatchCall() and previously produced the raw V8 fatal. The js-rpc-socket-streams-ts-test config runs the socket-loopback js-rpc suite (including the socket-transfer assertions) under the TypeScript streams implementation with hydration enabled.
…reams The typescript_implemented_streams compat flag cannot function on the RPC receive path without the rpc-externals-hydration autogate (stream construction executes JavaScript, which the graph read forbids), and supporting the combination by degrading to legacy-backed received streams would mean shipping a second, subtly different behavior for a misconfiguration. Reject it instead: when the gate is off and a stream-bearing value arrives at a TypeScript-streams isolate, RpcDeserializerExternalHandler::prepare() fails with an error naming the missing autogate, before the graph read begins. Stream-free RPC and the send side (which has no no-JS scope) are unaffected, and the in-place fallback paths are now reachable only for legacy-streams isolates, which they are scope-safe for.
A runner worker with typescript_implemented_streams calls a peer without it, so every stream RPC crosses stream implementations. Covers all four serializer/deserializer pairings -- TypeScript-serialized to legacy-deserialized in the argument direction (readable and writable) and legacy-serialized to TypeScript-deserialized in the return direction (readable, and both halves of an echo pair nested in an object) -- and asserts that received streams are instances of the receiving isolate's own stream globals.
…tract The own-symbol instance brands and the prohibition on dispatching into the TypeScript implementation while JS execution is disallowed are load-bearing parts of the C++/JS contract (RPC deserialization depends on both); record them alongside the other cross-fence rules.
JsWritableStream::serialize's TypeScript arm acquired the writer via
WritableStream.prototype.getWriter and TsWriterSink drove it through
WritableStreamDefaultWriter.prototype.{write,close,abort} -- all
user-patchable: a replaced method could intercept the transferred
bytes, fake the transfer while leaving the stream unlocked, or drop
remote writes. Route acquisition and every writer operation through new
members of the frozen cppExports object instead
(acquireWritableStreamWriter has the public getWriter's exact
semantics, including the locked TypeError from the shared constructor
path; the write/close/abort operations reuse the existing internal
algorithms, with a writerAbortInternal added beside them for the
released-writer check the public abort performs).
The readable arm was already patch-proof (own-symbol extraction and
frozen-export dispatch throughout). The regression test transfers both
stream directions with booby-trapped prototypes and asserts the traps
never fire; before this change it recorded getWriter, write, and
close.
jasnell
force-pushed
the
jasnell/streams-fun-phase2
branch
from
August 20, 2026 19:34
d9ca73a to
7b94e1a
Compare
jasnell
marked this pull request as ready for review
August 20, 2026 19:40
Collaborator
Author
|
The internal-build issues appear to have been temporary. Running locally shows green. Will see what CI says. |
guybedford
approved these changes
Aug 21, 2026
guybedford
left a comment
guybedford
left a comment
Contributor
There was a problem hiding this comment.
Full read-through, commit by commit. The overall architecture is sound and well-pinned: brand-based recognition through the new serializer class-instance hook, externals hydration before the V8 graph read (behind its own autogate, with the TS-flag-requires-gate rejection rather than half-support), wire-compatible cross-implementation transfer, and internal-algorithm dispatch for the writer sink so user-patched prototypes can't intercept transfers. Test coverage is strong: the full js-rpc suite in TS mode, the socket variant exercising the subsumed-externals slot, the crossflag matrix covering all four serializer/deserializer pairings, and the pollution test pinning the frozen-internals dispatch.
Things I verified while reading: kj::ArrayPtr::write is cursor-advancing, so the TsWriterSink pieces loop is correct; claimPrebuilt's bounds KJ_ASSERT matches the existing read() style and the destructor's consume-all assert stays coherent with socket spans; and the socket-gate × hydration-gate × TS-flag matrix behaves sensibly in all combinations (including the gate-off socket path skipping hydration so the kill-switch rejection is still reached).
Approving with one question and a few minor findings, inline.
This review was written with AI assistance and may contain mistakes; treat each finding on its merits.
The api-symbol brand that isTypeScriptReadableStream() probes stays visible to reflection, so user code can read it off a real stream and stamp it on an object of its own: the check recognizes streams, it does not authenticate them. Say so, along with what makes that safe -- recognition grants nothing on its own, because every operation reached afterwards goes through the TypeScript internal algorithms, whose real #-brand checks reject an impostor with a TypeError before either RPC serialize arm writes anything to the wire, and a symbol-keyed brand cannot arrive over the wire at all. Pin it with a test that brands a bare object, watches it unwrap, and watches the first operation throw. Extract the serializer's RPC-handler requirement into requireReadableStreamRpcSerializer() and resolve it first in both readable serialize arms. Folding that requirement into newReadableStreamSerializeSink() had placed it behind IoContext::current(), so serializing a stream from global scope reported the async-I/O error instead of the DataCloneError that says streams only transfer over RPC -- the order the writable arm already used. Also fix the "has endeded" typo in the RPC disconnect message, in all three copies and the test that asserts it, and drop a stale first line from the comment on TsWriterSink's writer dispatch.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #7071
Implements the mechanisms to support the jsrpc serialization of TS-implemented ReadableStream and WritableStream.
Review commit-by-commit.
Draft while I take my own follow up review pass over it.