Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Rest of AggregateError spec #37

Merged
merged 32 commits into from
Oct 1, 2019

Conversation

chicoxyzzy
Copy link
Member

@chicoxyzzy chicoxyzzy commented Sep 25, 2019

See #27 and #36

Unfinished parts:

spec.html Outdated

<emu-clause id="sec-aggregateerror-object-structure">
<h1>_AggregateError_ Object Structure</h1>
<p>TODO</p>
Copy link
Member Author

Choose a reason for hiding this comment

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

need help here

Copy link
Member Author

@chicoxyzzy chicoxyzzy Sep 25, 2019

Choose a reason for hiding this comment

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

NativeError Object Structure description:

      <p>When an ECMAScript implementation detects a runtime error, it throws a new instance of one of the _NativeError_ objects defined in <emu-xref href="#sec-native-error-types-used-in-this-standard"></emu-xref>. Each of these objects has the structure described below, differing only in the name used as the constructor name instead of _NativeError_, in the `name` property of the prototype object, and in the implementation-defined `message` property of the prototype object.</p>
      <p>For each error object, references to _NativeError_ in the definition should be replaced with the appropriate error object name from <emu-xref href="#sec-native-error-types-used-in-this-standard"></emu-xref>.</p>

Copy link
Member Author

@chicoxyzzy chicoxyzzy Sep 25, 2019

Choose a reason for hiding this comment

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

⚠️ It seems that NativeError Object Structure description should be updated too

Copy link
Member

Choose a reason for hiding this comment

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

How about something like:

<p>When an ECMAScript implementation detects a runtime error, it throws a new instance of one of the _NativeError_ objects defined in <emu-xref href="#sec-native-error-types-used-in-this-standard"></emu-xref> or a new instance of %AggregateError%. Each of these objects has the structure described below, differing only in the name used as the constructor name instead of _NativeError_, in the `name` property of the prototype object, in the implementation-defined `message` property of the prototype object, and in the presence of the %AggregateError%-specific `errors` property.</p>

Copy link
Member Author

Choose a reason for hiding this comment

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

NativeError Object Structure and AggregateError Object Structure are two separate sections so they need different descriptions

Copy link
Member

@mathiasbynens mathiasbynens Sep 25, 2019

Choose a reason for hiding this comment

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

Yeah, I was thinking something like:

For NativeError

<p>When an ECMAScript implementation detects a runtime error, it throws either a new instance of one of the _NativeError_ objects defined in <emu-xref href="#sec-native-error-types-used-in-this-standard"></emu-xref> or a new instance of %AggregateError%. Each of the _NativeError_ objects has the structure described below, differing only in the name used as the constructor name instead of _NativeError_, in the `name` property of the prototype object, in the implementation-defined `message` property of the prototype object, and in the presence of the %AggregateError%-specific `errors` property.</p>

For AggregateError

<p>When an ECMAScript implementation detects a runtime error, it throws either a new instance of one of the _NativeError_ objects defined in <emu-xref href="#sec-native-error-types-used-in-this-standard"></emu-xref> or a new instance of %AggregateError%. Each _AggregateError_ object has the structure described below, differing only in the implementation-defined `message` property of the prototype object.</p>

Copy link
Member Author

Choose a reason for hiding this comment

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

I've hoisted this to Error Objects decription

spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
@chicoxyzzy chicoxyzzy force-pushed the rest-of-aggregateerror branch from e6d18a0 to 79d9bf8 Compare September 25, 2019 13:56
chicoxyzzy and others added 7 commits September 25, 2019 16:58
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
spec.html Outdated Show resolved Hide resolved
spec.html Outdated

<emu-clause id="sec-aggregateerror-object-structure">
<h1>_AggregateError_ Object Structure</h1>
<p>TODO</p>
Copy link
Member

Choose a reason for hiding this comment

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

How about something like:

<p>When an ECMAScript implementation detects a runtime error, it throws a new instance of one of the _NativeError_ objects defined in <emu-xref href="#sec-native-error-types-used-in-this-standard"></emu-xref> or a new instance of %AggregateError%. Each of these objects has the structure described below, differing only in the name used as the constructor name instead of _NativeError_, in the `name` property of the prototype object, in the implementation-defined `message` property of the prototype object, and in the presence of the %AggregateError%-specific `errors` property.</p>

spec.html Show resolved Hide resolved
spec.html Outdated
<emu-clause id="sec-error-objects">
<h1>Error Objects</h1>
<p>Instances of Error objects are thrown as exceptions when runtime errors occur. The Error objects may also serve as base objects for user-defined exception classes.</p>
<p>When an ECMAScript implementation detects a runtime error, it throws a new instance of one of the _NativeError_ objects defined in <emu-xref href="#sec-native-error-types-used-in-this-standard"></emu-xref> or a new instance of %AggregateError%. Each of these objects has the structure described below, differing only in the name used as the constructor name instead of _NativeError_, in the `name` property of the prototype object, in the implementation-defined `message` property of the prototype object, and in the presence of the %AggregateError%-specific `errors` property.</p>
Copy link
Member

Choose a reason for hiding this comment

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

why is aggregate error not just another one of the native error types?

Copy link
Member Author

Choose a reason for hiding this comment

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

AggregateError has different constructor behavior than NativeErrors as @bakkot mention in #27

Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I’m not sure that suggestion is in fact the best solution - it ends up duplicating a lot of spec text. Is there a way we could modify the NativeError approach so it could handle AggregateErrors?

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that it's possible. Will try to figure out

spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
Copy link
Member

@ljharb ljharb left a comment

Choose a reason for hiding this comment

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

I'm not a super big fan of leaving the NativeError magic in place when it won't work for all native errors - I'd prefer to see it handled in a more explicit way that also encompasses AggregateError.

spec.html Outdated Show resolved Hide resolved
spec.html Outdated Show resolved Hide resolved
</emu-clause>

<emu-clause id="sec-aggregate-error.prototype.errors">
<h1>_AggregateError_.prototype.errors</h1>
Copy link
Member

Choose a reason for hiding this comment

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

repeating: there must not be a mutable object data property on the prototype.

AggregateError.prototype.errors must either be absent, a primitive, an accessor that returns a new array every time (either empty, or with the errors if #38 moves forward), or a data property using a frozen array.

Copy link
Member Author

@chicoxyzzy chicoxyzzy Sep 30, 2019

Choose a reason for hiding this comment

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

Should it be added to Properties of the _AggregateError_ Prototype Object part?

@chicoxyzzy
Copy link
Member Author

chicoxyzzy commented Sep 26, 2019

Just added "Native Error Types Used in This Standard" prose and AggregateError.prototype.toString implementation. Both need some 👀 on them

Unfortunately, I won't have time for further changes until Monday :(
I would be happy if someone will take care of this PR until then!

@chicoxyzzy chicoxyzzy marked this pull request as ready for review September 26, 2019 10:34
spec.html Outdated Show resolved Hide resolved
@chicoxyzzy chicoxyzzy force-pushed the rest-of-aggregateerror branch from 15b2f1c to 2fa9b4d Compare September 30, 2019 13:43
@chicoxyzzy
Copy link
Member Author

@mathiasbynens I've just updated toString method. Not sure if I did it right

spec.html Outdated
1. Let _errorsNumString_ ! ToString(_errorsNum_).
1. If _name_ is the empty String, return _msg_.
1. If _msg_ is the empty String, return _name_.
1. Return the string-concatenation of _name_, the code unit 0x003A (COLON), the code unit 0x0020 (SPACE), _msg_, "(", _errorsNumString_, " messages)".
Copy link
Member

Choose a reason for hiding this comment

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

This results in "1 messages".

FWIW, I'd be totally okay with leaving this up to implementations, by making it part of the message property. We could have a non-normative note that recommends implementers to include the message count in the error message.

@mathiasbynens mathiasbynens requested a review from ljharb October 1, 2019 13:27
@mathiasbynens
Copy link
Member

Alright, let's merge this one soon. Any follow-up work can happen in separate PRs. Thanks, @chicoxyzzy!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants