fix(ext/web): structuredClone of non-serializable Web types throws DataCloneError - #33465
Merged
bartlomieju merged 3 commits intoApr 25, 2026
Merged
Conversation
…taCloneError
`structuredClone(new Response())` (and Request/Headers/ReadableStream/
WritableStream/TransformStream) silently produced an empty `{}` instead
of throwing `DataCloneError` like Node and the Web Platform spec
require. V8's structured-clone serialiser sees these as plain JS objects
with no enumerable own properties, so the fast `core.structuredClone`
path round-trips them as `{}`.
Pre-check the input against a lazily-resolved list of known
non-serializable Web globals before invoking V8's serialiser. The list
covers the types whose specs explicitly mark them as not [Serializable]
(`Headers`, `Request`, `Response`, `ReadableStream`,
`WritableStream`, `TransformStream`); throw `DataCloneError` for them.
Plain objects, arrays, dates, etc. continue to clone via the fast path
unchanged.
Fixes denoland#32914.
bartlomieju
reviewed
Apr 25, 2026
Per @bartlomieju's review on denoland#33465, replace the brittle global-name lookup with a non-enumerable, non-configurable Symbol property installed on each non-serializable prototype. - Define `kNotSerializable = Symbol("[[NotSerializable]]")` and a `markNotSerializable(target)` helper in `13_message_port.js`. The property descriptor is `{ enumerable: false, writable: false, configurable: false }` so it can't be hidden, deleted, or overridden. - Mark the stream prototypes (`ReadableStream`, `WritableStream`, `TransformStream`) inline in `13_message_port.js` since they're already imported there. - Mark `Headers`, `Request`, `Response` prototypes from their respective modules in `ext/fetch`, importing `markNotSerializable` from `ext:deno_web/13_message_port.js`. No cycle: `13_message_port` doesn't import from `ext/fetch`. - `structuredClone` now does a single property check (`value[kNotSerializable]`) instead of iterating a list of prototype objects.
Member
|
Nice fix! Two observations: Nested objects are not caught. The check only inspects the root value, so Missing test coverage for WritableStream / TransformStream. They're marked with |
- Add WritableStream/TransformStream to test coverage - Add comment about nested non-serializable object limitation - Update WPT expectations: "Serializing a non-serializable platform object fails" now passes, add new expected failure for transferable subclass test - Fix fetch.json dangling-markup entry ordering
1 task
This was referenced Apr 25, 2026
nathanwhitbot
pushed a commit
to nathanwhitbot/deno
that referenced
this pull request
Apr 25, 2026
Same regression flagged on denoland#33411: test fails on aarch64 after denoland#33465 made structuredClone stricter for non-serializable Web types. Mark flaky on this branch too so the merge doesn't block CI.
nathanwhitbot
pushed a commit
to nathanwhitbot/deno
that referenced
this pull request
Apr 25, 2026
…nd#33465 Flaky marker didn't help — DataCloneError fires deterministically on every retry after denoland#33465 made structuredClone stricter for non-serializable Web types. Ignore until the polyfill divergence is resolved.
nathanwhitbot
pushed a commit
to nathanwhitbot/deno
that referenced
this pull request
Apr 25, 2026
…nd#33465 Flaky marker didn't help — DataCloneError fires deterministically on every retry after denoland#33465 made structuredClone stricter for non-serializable Web types. Ignore until the polyfill divergence is resolved.
3 tasks
bartlomieju
added a commit
that referenced
this pull request
Apr 25, 2026
…Clone (#33491) Fixes a regression from #33465 where `structuredClone(value, { transfer: [value] })` threw `DataCloneError` for `ReadableStream`, `WritableStream`, and `TransformStream`. The `kNotSerializable` check was firing even when the value was being **transferred**, not cloned. The fix skips the check when the value is present in the transfer list.
nathanwhitbot
pushed a commit
to nathanwhitbot/deno
that referenced
this pull request
Apr 26, 2026
…denoland#33465" Per bartlomieju's review, don't mask the structuredClone regression introduced by denoland#33465; the underlying fix should land separately.
nathanwhitbot
pushed a commit
to nathanwhitbot/deno
that referenced
this pull request
Apr 26, 2026
…denoland#33465" Reverts both the flaky and ignore workarounds per bartlomieju's request. The test entry returns to the default {} state.
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
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.
Summary
structuredClone(new Response())(and the same withRequest,Headers,ReadableStream,WritableStream,TransformStream) silently produced an empty{}instead of throwingDataCloneErrorlike Node and the Web Platform spec require:These types have no enumerable own properties — they're effectively branded JS objects whose state lives in internal slots. V8's
ValueSerializerdoesn't know they're "platform" types, so it treats them as ordinary objects with no entries to serialise, and the fastcore.structuredClone(value)path returns{}.Pre-check the input against a lazily-resolved list of known non-serializable Web globals before handing off to V8. Each prototype is looked up at first call (so the fix doesn't depend on bootstrap ordering / wouldn't introduce circular ext imports between
webandfetch), thenObjectPrototypeIsPrototypeOfis used for the brand check.The list:
Headers,Request,Response— explicitly not[Serializable]per Fetch §2.2.5 / §6 / §7.ReadableStream,WritableStream,TransformStream—[Transferable](only via the transfer list), not[Serializable].Fixes #32914.
Test plan
tests/specs/run/structured_clone_non_serializablecovering each non-serializable type plus positive controls (plain object / array /Date) to confirm valid types still clone.structuredClone succeeded unexpectedly: {}, after printsstructuredClone threw: DataCloneError Cannot clone object of unsupported type.(matches Node)../x fmtclean../x lintclean (full Rust + JS).