1787546 - structuredClone of Error doesn't clone cause if it wasn't provided at construction time

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P1)

Description

[Andrew Swan [:aswan]]

Consider this code:

let e = new Error("outer", { cause: new Error("inner") });
let e2 = new Error("outer");
e2.cause = new Error("inner");

let eClone = structuredClone(e);
console.log(eClone.cause);  // all good, cause is the "innner" error!

let e2Clone = structuredClone(e2);
console.log(e2Clone.cause);  // huh, undefined?

It seems that this isn't specifically covered by any standard, but it is confusing for developers. For what it is worth, e2Clone.cause is present when running the above code in V8.

Comment 1

[Steve Fink [:sfink] [:s:]]

In SpiderMonkey, cause is implemented as an internal slot rather than a regular property. My reading of the current spec is that it should be a plain property, and the above should work. It was probably a difference in the earlier proposal, but does anyone know if there's a reason to keep it as an internal slot?

Comment 2

[André Bargull [:anba]]

fileName, lineNumber, columnNumber, message, and cause are all plain data properties which are implemented as reserved slots. IIRC I've implemented cause to match how the other properties are implemented, maybe also because it was faster? Changing cause to a plain data property now might be tricky, because the callers of ErrorObject::getCause() assume no side-effects are possible.

Comment 3

[André Bargull [:anba]]

Attachment: Bug 1787546: Handle custom added "cause" property in structured clone of Error objects. r=sfink!

Comment 4

[Tom S]

Using GetOwnPropertyDescriptor similar to what is done for message already in the structured cloning code. So that is totally fine with me.

To maybe emphasize: while cause is implemented as a reserved slot it looks like a plain property to JS.

One thing to think about are the other places where access ErrorObject::getCause directly. However it looks like we also access the message and other slots directly, so it doesn't seem that important to change now.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/06762e6914a0
Handle custom added "cause" property in structured clone of Error objects. r=sfink

Comment 6

[Natalia Csoregi [:nataliaCs]]

https://hg.mozilla.org/mozilla-central/rev/06762e6914a0