web: mark as uncloneable when possible

[jazelly]

Rationale

This tells node that the marked instances from undici are not cloneable, so that attempts to cloning those throw DataCloneError.

This is a follow-up work from nodejs/node#55234

Changes

Marked the following as uncloneable.

'FormData', 'Headers', 'Request', 'Response', 'MessageEvent', 'CloseEvent', 'EventSource', 'WebSocket' 

Features

Bug Fixes

Breaking Changes and Deprecations

Status

• I have read and agreed to the Developer's Certificate of Origin
• Tested
• Benchmarked (optional)
• Documented
• Review ready
• In review
• Merge ready

[jazelly]

I am hesitant to write a test here, as it would be like testing the behaviour of structuredClone, which should be placed at Node. I will do a follow up once this is bumped into node.

[mcollina]

lgtm

[mcollina]

I really think having a test would be useful, add it.

[ronag]

Why is Headers unclonable?

[jazelly]

AFAIK, any cloneable that wants to be cloned in node runtime needs to

1. implement kClone
2. implement kDeserialize
3. markTransferMode() as "cloneable"

One example you can find in node is https://github.com/nodejs/node/blob/00b2f07f9ddeb8ffd2fb2108b0ed9ffa81ea000d/lib/internal/webstreams/transfer.js#L49

I don't think the above are exposed, so undici classes are not cloneable in node from that sense.

[ronag]

I didn't mean to close this. Sorry.

[Uzlopak]

How can we access those symbols from userland?

[KhafraDev]

Why is Headers unclonable?

@ronag it's because webidl platform objects cannot be cloned:

A JavaScript value value is a platform object if Type(value) is Object and if value has a [[PrimaryInterface]] internal slot.

Platform objects can be serializable objects if their primary interface is decorated with the [Serializable] IDL extended attribute. Such interfaces must also define the following algorithms:

Notice that the webidl for Headers does not include serializable:

[[Exposed](https://webidl.spec.whatwg.org/#Exposed)=(Window,Worker)] interface Headers { ... }

[jazelly]

I should've mentioned, this is more to tell Node to treat undici instances as a platform object, as this api will attach a private attribute that Node can recognize. Without it, Node will try to find the kTransfer method. If it's not present, it will return {}, which is why we had the original issue on node side.

structuredClone(new Response()) // {} 

I think whether/how we make some classes cloneable in undici should be a separate and broader discussion, since it might involve whether/how Node exposes its internal symbols.

[jazelly]

I also added the test. It works fine on my local but won't be effective on current CI, as the API hasn't been released. The API is on v22.10 release line, so I am happy to hold this PR till then if that's more applicable.

[Uzlopak]

why "maybeMarkAsUncloneable"? If in the future all supported platforms have markAsUncloneable, then do we need to rename this function and all the places where it is called?

More likely that we forget that in 3 years.

[jazelly]

More likely that we forget that in 3 years.

Good point. I updated the naming.

[Uzlopak]

RSLGTM

btw. cloneable or clonable, is the spelling right?

[jazelly]

Yeah, it's correct. In node it's always cloneable.

[KhafraDev]

Does this also need to be done for EventSource, WebSocket, WebsocketStream, CloseEvent, ErrorEvent, MessageEvent, Cache, CacheStorage, and possibly others?

[jazelly]

I think we should. I was focusing on the classes that are exposed to node only.

[KhafraDev]

WebSocket, CloseEvent, and MessageEvent are exposed in node

[jazelly]

Yes, they've been updated within this PR. I think maybe every class under lib/web should be marked so they can be treated as platform objects besides those?

[mcollina]

lgtm

[mcollina]

Can you please fix linting?

[mcollina]

Tagging as backportable

[github-actions]

The backport to v6.x failed:

The process '/usr/bin/git' failed with exit code 1 

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub git fetch # Create a new working tree git worktree add .worktrees/backport-v6.x v6.x # Navigate to the new working tree cd .worktrees/backport-v6.x # Create a new branch git switch --create backport-3709-to-v6.x # Cherry-pick the merged commit of this pull request and resolve the conflicts git cherry-pick -x --mainline 1 1ab238207d84196479428494d52fc922833e8e5b # Push it to GitHub git push --set-upstream origin backport-3709-to-v6.x # Go back to the original working tree cd ../.. # Delete the working tree git worktree remove .worktrees/backport-v6.x

Then, create a pull request where the base branch is v6.x and the compare/head branch is backport-3709-to-v6.x.

[mcollina]

@jazelly would you mind sending a backport commit with the instructions above?

[jazelly]

I am happy to do it.