Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AggregateError serialization is standard track #25744

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

ddbeck
Copy link
Collaborator

@ddbeck ddbeck commented Jan 22, 2025

Summary

Note: This PR is somewhat incomplete. Please read fully before merging.

AggregateError's serialization is potentially standard track, depending on how you look at it. I'm opening this to settle the question. After some head scratching, I'm proposing that AggregateError's serialization be marked as standard track, with all implementations being marked as partial, but I'm open to other views.

Test results and supporting details

The HTML spec is somewhat odd when it comes to the serialization of errors. Note step 17 in section 2.7.3 StructuredSerializeInternal ( value, forStorage [ , memory ] ).

There are two noteworthy things here:

  • It describes handling for any object with [[ErrorData]], which includes AggregateError but not AggregateError specifically (as it does for other error types).
  • It also says that "User agents should attach a serialized representation of any interesting accompanying data […]" with a note that says the "interesting" data isn't yet specified.

Taken together, the compat data tells a maybe odd story, that AggregateError is not serializable (it is though some ambiguously "interesting" information is, depending on your perspective, being lost) and that Firefox's behavior is non-standard (which it is, when it comes to the cloned object's type).

whatwg/html#5749 looks to settle this more comprehensively, but it hasn't merged yet.

To thread the needle, I suggest marking AggregateError's serialization as standard track (shown in the diff now), with all implementations being marked as partial (to be done), but I'm open to other views.

Related issues

@github-actions github-actions bot added data:js Compat data for JS/ECMAScript features. https://developer.mozilla.org/docs/Web/JavaScript size:xs [PR only] 0-6 LoC changed labels Jan 22, 2025
@caugner
Copy link
Contributor

caugner commented Jan 22, 2025

Yes, AggregateError seems to be serializable per the HTML Standard: The referenced step 17 applies to AggregateError, because it has an [[ErrorData]] internal slot (whose value is undefined, which is not a platform object).

@ddbeck Why would you consider the Firefox implementation as partial? Because the serialize representation is missing the [[Type]] field (provided that the current note is accurate), or because it has additional fields?

As for Chrome and Safari, did you research their current behavior regarding serialization of AggregateError? Or does "all implementations" only refer to Firefox?

@ddbeck
Copy link
Collaborator Author

ddbeck commented Jan 23, 2025

Why would you consider the Firefox implementation as partial? Because the serialize representation is missing the [[Type]] field (provided that the current note is accurate), or because it has additional fields?

As for Chrome and Safari, did you research their current behavior regarding serialization of AggregateError? Or does "all implementations" only refer to Firefox?

Firefox's non-standard behavior is to serialize to an AggregateError with errors information, where the spec says it should be an Error. That is, in Firefox:

structuredClone(new AggregateError([new Error("some error")], "Hello")).name
 AggregateError
structuredClone(new AggregateError([new Error("some error")], "Hello")).errors
 /* an array of Errors */

So Firefox is doing the right thing with respect to step 17.6 ("User agents should attach a serialized representation of any interesting accompanying data which are not yet specified […] to serialized"), but the wrong thing with respect to step 17.2 ("If name is not one of […], then set name to 'Error'").

Where Safari and Chrome do as the spec (currently) says with respect to the name, but loses the interesting errors:

structuredClone(new AggregateError([new Error("some error")], "Hello")).name
 Error
structuredClone(new AggregateError([new Error("some error")], "Hello")).errors
 undefined

Basically, I don't think anyone is doing this correctly with respect to the specification (though Firefox's implementation is plainly more useful).

@caugner
Copy link
Contributor

caugner commented Jan 23, 2025

Thanks for the clarifications, @ddbeck!

So currently the BCD data for AggregateError states no support in Chrome and Safari, and we should fix that as part of this PR, adding notes about how they deviate from the spec.

Next up, we should fix the Firefox note to be a bit more clear, and explain the deviation from the specification.

When compared to other cases where we decide between partial and "just a note", I feel like partial is a bit harsh here, as all browsers, especially Firefox, seem to have a usable implementation with minor limitations. At the same time, marking the implementations as partial could increase the chances of these limitations to be fixed.

@Elchi3 What do you think?

@@ -173,7 +173,7 @@
},
"status": {
"experimental": true,
"standard_track": false,
"standard_track": true,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add a link to this text fragment:

"spec_url": "https://html.spec.whatwg.org/multipage/structured-data.html#:~:text=if%20value%20has%20an%20%5B%5Berrordata%5D%5D%20internal%20slot",

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a big fan of these text fragment URLs as I can't validate them (#23958) but I will not stay in the way if we think they are adding value.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In our October 29, 2024 meeting we reached consensus to use text fragments where anchors aren't available.

In this case, we could link to the section instead, but it wouldn't be as straight-forward.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion, any text fragment spec link is better than none.

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this can land as is. Seems like specification work is a bit behind unfortunately

@caugner caugner changed the title AggregateError serialization is standard track (maybe) AggregateError serialization is standard track Jan 28, 2025
@caugner caugner merged commit 521ec84 into mdn:main Jan 28, 2025
8 checks passed
@caugner
Copy link
Contributor

caugner commented Jan 28, 2025

PS: I mentioned the spec text fragment in the commit message.

@ddbeck ddbeck deleted the aggregateerror-serializable branch January 28, 2025 15:51
ddbeck added a commit to ddbeck/browser-compat-data that referenced this pull request Jan 28, 2025
I wasn't super satisfied with merging
mdn#25744 as is.
ddbeck added a commit to ddbeck/browser-compat-data that referenced this pull request Jan 28, 2025
ddbeck added a commit to ddbeck/browser-compat-data that referenced this pull request Jan 28, 2025
I wasn't super satisfied with merging
mdn#25744 as is.
ddbeck added a commit to ddbeck/browser-compat-data that referenced this pull request Jan 28, 2025
I wasn't super satisfied with merging
mdn#25744 as is.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data:js Compat data for JS/ECMAScript features. https://developer.mozilla.org/docs/Web/JavaScript size:xs [PR only] 0-6 LoC changed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants