nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
107.22k stars 29.39k forks source link

`stucturedClone` defective for `DataCloneError` #49181

Open corrideat opened 1 year ago

corrideat commented 1 year ago

Version

v19.9.0

Platform

Linux WORKSTATION 5.15.90.1-microsoft-standard-WSL2 #1 SMP Fri Jan 27 02:56:13 UTC 2023 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

Run the following snippet:

const [e, c] = (() => {
  try {
    // Fails because functions can't be cloned
    structuredClone(()=>{});
  } catch (e) {
    // Return the original error and a clone
    return [e, structuredClone(e)];
  }
})();
console.log({
    'e instanceof Error': e instanceof Error,
    'e.name': e.name,
    'e.message': e.message,
    'e.stack': e.stack,
}, {
    'c instanceof Error': c instanceof Error,
    'c.name': c.name,
    'c.message': c.message,
    'c.stack': c.stack,
});

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior? Why is that the expected behavior?

Since structuredClone is supposed to support errors, the result should not be an empty object. It should be either a copy of the original error, or a copy of the origininal error but with name changed to Error.

What do you see instead?

The cloned DataCloneError cloned as an empty object.

Welcome to Node.js v19.9.0.
Type ".help" for more information.
> const [e, c] = (() => {
...   try {
...     // Fails because functions can't be cloned
...     structuredClone(()=>{});
...   } catch (e) {
...     // Return the original error and a clone
...     return [e, structuredClone(e)];
...   }
... })();
undefined
> console.log({
...     'e instanceof Error': e instanceof Error,
...     'e.name': e.name,
...     'e.message': e.message,
...     'e.stack': e.stack,
... }, {
...     'c instanceof Error': c instanceof Error,
...     'c.name': c.name,
...     'c.message': c.message,
...     'c.stack': c.stack,
... });
{
  'e instanceof Error': true,
  'e.name': 'DataCloneError',
  'e.message': '()=>{} could not be cloned.',
  'e.stack': 'DataCloneError: ()=>{} could not be cloned.\n' +
    '    at new DOMException (node:internal/per_context/domexception:53:5)\n' +
    '    at structuredClone (node:internal/structured_clone:23:17)\n' +
    '    at REPL9:4:5\n' +
    '    at REPL9:9:3\n' +
    '    at Script.runInThisContext (node:vm:128:12)\n' +
    '    at REPLServer.defaultEval (node:repl:570:29)\n' +
    '    at bound (node:domain:433:15)\n' +
    '    at REPLServer.runBound [as eval] (node:domain:444:12)\n' +
    '    at REPLServer.onLine (node:repl:900:10)\n' +
    '    at REPLServer.emit (node:events:525:35)'
} {
  'c instanceof Error': false,
  'c.name': undefined,
  'c.message': undefined,
  'c.stack': undefined
}

Additional information

This error limits sending back useful error information through MessagePorts

This is the result of running the same snippet in other runtimes.

Deno

Deno 1.35.2
exit using ctrl+d, ctrl+c, or close()
REPL is running with all permissions allowed.
To specify permissions, run `deno repl` with allow flags.
> const [e, c] = (() => {
  try {
    // Fails because functions can't be cloned
    structuredClone(()=>{});
  } catch (e) {
    // Return the original error and a clone
    return [e, structuredClone(e)];
  }
})();
console.log({
    'e instanceof Error': e instanceof Error,
    'e.name': e.name,
    'e.message': e.message,
    'e.stack': e.stack,
}, {
    'c instanceof Error': c instanceof Error,
    'c.name': c.name,
    'c.message': c.message,
    'c.stack': c.stack,
});

{
  "e instanceof Error": true,
  "e.name": "DataCloneError",
  "e.message": "()=>{} could not be cloned.",
  "e.stack": "DOMException: ()=>{} could not be cloned.\n" +
    "    at new DOMException (ext:deno_web/01_dom_exception.js:"... 288 more characters
} {
  "c instanceof Error": false,
  "c.name": undefined,
  "c.message": undefined,
  "c.stack": undefined
}
undefined

Microsoft Edge

const [e, c] = (() => {
  try {
    // Fails because functions can't be cloned
    structuredClone(()=>{});
  } catch (e) {
    // Return the original error and a clone
    return [e, structuredClone(e)];
  }
})();
console.log({
    'e instanceof Error': e instanceof Error,
    'e.name': e.name,
    'e.message': e.message,
    'e.stack': e.stack,
}, {
    'c instanceof Error': c instanceof Error,
    'c.name': c.name,
    'c.message': c.message,
    'c.stack': c.stack,
});

{
    "e instanceof Error": true,
    "e.name": "DataCloneError",
    "e.message": "Failed to execute 'structuredClone' on 'Window': ()=>{} could not be cloned.",
    "e.stack": "Error: Failed to execute 'structuredClone' on 'Window': ()=>{} could not be cloned.\n    at <anonymous>:4:5\n    at <anonymous>:9:3"
}
{
    "c instanceof Error": true,
    "c.name": "DataCloneError",
    "c.message": "Failed to execute 'structuredClone' on 'Window': ()=>{} could not be cloned."
}

Mozilla Firefox

const [e, c] = (() => {
  try {
    // Fails because functions can't be cloned
    structuredClone(()=>{});
  } catch (e) {
    // Return the original error and a clone
    return [e, structuredClone(e)];
  }
})();
console.log({
    'e instanceof Error': e instanceof Error,
    'e.name': e.name,
    'e.message': e.message,
    'e.stack': e.stack,
}, {
    'c instanceof Error': c instanceof Error,
    'c.name': c.name,
    'c.message': c.message,
    'c.stack': c.stack,
});

{
  "e instanceof Error": true,
  "e.name": "DataCloneError",
  "e.message": "The object could not be cloned.",
  "e.stack": "@debugger eval code:4:20\n@debugger eval code:9:3\n"
}
{
  "c instanceof Error": true,
  "c.name": "DataCloneError",
  "c.message": "The object could not be cloned.",
  "c.stack": ""
}
inctnce commented 1 year ago

I can take it

Matheuss1 commented 10 months ago

Hello!

I've looked at @inctnce to check if he was taking care of this issue, but apparently he is not, so I have been trying to solve this by myself.

I haven't solved it yet, but I've discovered things that can be useful to find a solution...

Before any explanations, I would like to say that I am new here and I don't know much about V8 and node internals yet, but I did my best to understand what was possible until now.

So, here we go:

To say the truth, this problem does not happen with DataCloneError only, but with any type of error that is also a "DomException" (based on my tests). With that said, we can also see that a DomException object is a little different from a standard Error object. I think it's due this object being "different" that makes the function to not work as excepted.

Where the problem probably is

When using the structuredClone function directly on browser (like Chrome, that uses V8) we get a different result from Node, and that is because Node has implemented its own custom function (can be found in src/node_messaging.cc).

The node structuredClone function uses a custom serializer to serialize an object (can be found on line 295 of src/node_messaging.cc file). The custom serializer includes a function called IsHostObject that apparently indicates if the structuredClone first argument is a BaseObject or a JSTransferable, and apparently our DomException is neither of them, returning nothing.

So, searching for the method WriteJSReceiver on deps/v8/objects/value-serializer.cc, we can see that when the function IsHostObject does not have a result, we fall on a case (and I am not sure if I am correctly here) where an empty result is also returned and apparently that is why we ends with an empty object on JS.

But look, just changing the IsHostObject function to return true for our Dom Exception does not solve the problem. I think that would be necessary to change the function WriteHostObject too, but my knowledge was limited here.

Final considerations

Like I said, I am new here on NodeJS community, and I have little knowledge of how V8 and Node uses C++ code to allow we to run JS. So, if I said something that does not make sense, I would be grateful to be warned about :)