Expand JS error serialization

[domenic]

trafficstars

• Generalize the framework to work on any NativeError types introduced by the JS spec, instead of listing them explicitly in a way that could require future updates.
• Also include WebAssembly Error classes.
• Switch semantics for serializing/deserializing "message". Previously, we would check for the presence of the property, and if it was a data property, get its value, and then ToString() it. Now, we just Get() it and then structured-serialize the result. We also unconditionally install it on the other side, regardless of whether it was present on the original. This new property-cloning procedure is more general.
• Use these new more general property cloning procedure on "cause" (for all errors) and "errors" (for objects that present as "AggregateError" objects).

Closes #5716.

• [x] At least two implementers are interested (and none opposed):
  • In this thread and in #5716 @yutakahirano (Chromium) and @evilpie (Gecko) have been supportive, but they haven't yet reviewed the full spec or signed up to implement
• [ ] Tests are written and can be reviewed and commented upon at:
  • TODO; see below.
• [ ] Implementation bugs are filed:
  • Chrome: …
  • Firefox: …
  • Safari: …

(See WHATWG Working Mode: Changes for more details.)

Suggested tests for the semantics as specced here:

• Expand the tests to cover WebAssembly.CompileError, WebAssembly.LinkError, and WebAssembly.RuntimeError.
• Test the following for both message and cause (updating message tests as appropriate since we're changing the semantics):
  • Deleting the property from one side results in an own property with value undefined on the destination
  • Setting the property to something uncloneable results in a DataCloneError
  • Overwriting the property with a getter results in that getter being called and its return value being cloned
  • Overwriting the property's enumerable/configurable/writable-ness does not have any impact; the other side always gets nonenumerable/configurable/writable.
  • Setting the property to something complex-but-serializable (e.g. a Map) results in the value bieng cloned
• Test all of the above for errors too, but only on AggregateError
• errors specific tests:
  • Any errors property on a non-AggregateError is ignored, except:
  • Mutating a non-AggregateError to have .name === "AggregateError" makes errors get cloned (and the other side getting an AggregateError)

---

/infrastructure.html ( diff ) /references.html ( diff ) /structured-data.html ( diff )

[annevk]

Did you consider using deep? Would that also cover cases such as stack automatically?

[domenic]

Since these properties are non-enumerable, that would not work.

[annevk]

I guess we could add a branch to that algorithm though and keep a list of desired properties around in a different way.

[domenic]

That seems pretty indirect, compared to just serializing AggregateError-specific data in the AggregateError branch.

[annevk]

It is a bit, but if we want message, errors, and at some point stack it does kind of lean in that direction to me. And we probably want the same kind of value retrieval and copying semantics for these fields as we do for deep?

[domenic]

I don't think so. In particular the main characteristic of deep is that it iterates over an unbounded set of enumerable properties. If you don't reuse that part, then you're basically just "reusing" property gets, which is a lot of indirection to just get a one-liner. (Literally the only commonality is step 26.4.1.1; note that 26.4.1's predicate is only necessary because of the unbounded enumeration.)

[domenic]

If we did want consistency with deep though, then it would argue for my

If we were to change .message behavior, here are some potential semantics we could consider that would keep them in sync:

from the OP.

[domenic]

Ping @yutakahirano @evilpie for thoughts. Summary of the open question: The PR as-drafted contains reasonable semantics, but we could make things more reasonable if we also revisited how message is serialized. Is that churn to implementations + tests worth it, or should we just go ahead with the slight inconsistency?

[yutakahirano]

Ping @yutakahirano @evilpie for thoughts. Summary of the open question: The PR as-drafted contains reasonable semantics, but we could make things more reasonable if we also revisited how message is serialized. Is that churn to implementations + tests worth it, or should we just go ahead with the slight inconsistency?

The last discussion was: https://github.com/whatwg/html/pull/4665#issuecomment-499056397 https://github.com/whatwg/html/pull/4665#issuecomment-499415738. I think the argument is still valid.

I can implement this in either way, I believe.

[evilpie]

am I correct to assume that you filing the issue means you'd be interested in implementing in Firefox?)

I am not currently looking into implementing structured cloning for native errors. However if we are going to implement it, it would be better to have AggregateError squared away so that the on-disk format doesn't have to be changed again.

[evilpie]

@domenic Is this still on your radar? We probably want to support the cause property now as well.

[domenic]

I got a bit discouraged by the lack of clear answer, and also the fact that the tests for error cloning are annoying to update.

I am willing to try again on the spec front if others do the tests and we can ensure implementation interest. It sounds like there's at least support from yourself and @yutakahirano for doing something here in the spec, even if the implementation priority is not very high.

As for what the spec should say: at this point I am convinced it should be more generic so that it can adapt to future such error extensions from TC39. I am not sure exactly how, but probably some recursive cloning of object properties (preserving their enumerable/writable/configurable attributes??) would be best. I'll try to write something up and see how I like it... stay tuned.

[domenic]

OK, I rewrote it to be reasonably general, although new properties will still need to get added one by one. I'll copy and paste the new commit description here and also into the OP:

• Generalize the framework to work on any NativeError types introduced by the JS spec, instead of listing them explicitly in a way that could require future updates.
• Also include WebAssembly Error classes.
• Switch semantics for serializing/deserializing "message". Previously, we would check for the presence of the property, and if it was a data property, get its value, and then ToString() it. Now, we just Get() it and then structured-serialize the result. We also unconditionally install it on the other side, regardless of whether it was present on the original. This new property-cloning procedure is more general.
• Use these new more general property cloning procedure on "cause" (for all errors) and "errors" (for objects that present as "AggregateError" objects).

Somewhat related: https://github.com/tc39/proposal-error-cause/issues/35

This will require a lot of test updates, which I'm still not signing up for. I kind of wish the TC39 process did not get to go to stage 3 until we had this host integration figured out, or stage 4 until we had tests written; I'm talking with @syg about whether moving some of this responsibility into the ES spec might help with that.

[evilpie]

At least for SpiderMonkey it would be nicer if message was either a String or undefined. We can of course just overwrite the message property to an arbitrary value, but the internal Error object design is meant for (nullable) strings.

[zloirock]

Hey, maybe makes sense to finish it before exposing structuredClone in all stable modern browsers?

[domenic]

We'd welcome your help in working on a spec that can get implementer interest, and/or doing such implementation work!

[zloirock]

@domenic this PR is already not so bad excepting some mentioned above minor moments.

[evilpie]

Fwiw. We landed serialization support for cause and errors (on AggregateError) in Firefox. https://bugzilla.mozilla.org/show_bug.cgi?id=1556604

[jasnell]

Just want to check in on the status here. Is this expected to move forward? Is there anything someone like myself could help with to get it moving forward?

Also have a couple other questions:

1. Currently, structured clone of an Error does not clone any additional arbitary own properties set on the object. Will this change allow those to be serialized/deserialized? e.g.

const err = new Error();
err.abc = 1;
const err2 = structuredClone(err);
console.log(err2.abc);  // undefined

2. Will this support the new SupressedError that's being introduced as part of the Explicit Resource Management TC-39 proposal?