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

Make errors property of AggregateError non-enumerable #31

Merged
merged 6 commits into from Aug 3, 2019
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 17 additions & 9 deletions spec.html
Expand Up @@ -74,10 +74,7 @@ <h1>Runtime Semantics: PerformPromiseAny ( _iteratorRecord_, _constructor_, _res
1. Set _iteratorRecord_.[[Done]] to *true*.
1. Set _remainingElementsCount_.[[Value]] to _remainingElementsCount_.[[Value]] - 1.
1. If _remainingElementsCount_.[[Value]] is 0, then
1. Let _errorsArray_ be CreateArrayFromList(_errors_).
1. Let _error_ be a newly created `AggregateError` object.
1. Perform ! CreateDataProperty ( _error_, `"errors"`, _errorsArray_ ).
1. Return ThrowCompletion ( _error_ ).
1. Perform ? ThrowAggregateError(_errors_).
1. Return _resultCapability_.[[Promise]].
1. Let _nextValue_ be IteratorValue(_next_).
1. If _nextValue_ is an abrupt completion, set _iteratorRecord_.[[Done]] to *true*.
Expand Down Expand Up @@ -113,16 +110,26 @@ <h1>`Promise.any` Reject Element Functions</h1>
1. Set _errors_[_index_] to _x_.
1. Set _remainingElementsCount_.[[Value]] to _remainingElementsCount_.[[Value]] - 1.
1. If _remainingElementsCount_.[[Value]] is 0, then
1. Let _errorsArray_ be CreateArrayFromList(_errors_).
1. Let _error_ be a newly created `AggregateError` object.
1. Perform ! CreateDataProperty ( _error_, `"errors"`, _errorsArray_ ).
1. Return ThrowCompletion ( _error_ ).
1. Perform ? ThrowAggregateError(_errors_).
1. Return *undefined*.
</emu-alg>
<p>The `"length"` property of a `Promise.any` resolve element function is 1.</p>
</emu-clause>
</emu-clause>

<emu-clause id="sec-throwaggregateerror">
<h1>ThrowAggregateError ( _errors_ )</h1>
<p>The abstract operation ThrowAggregateError(_errors_) creates and throws an instance of AggregateError</p>
<emu-alg>
1. Assert: _errors_ is a List whose elements are all ECMAScript language values.
1. Let _errorsArray_ be CreateArrayFromList(_errors_).
bakkot marked this conversation as resolved.
Show resolved Hide resolved
1. Let _error_ be a newly created `AggregateError` object.
1. Let _errorsDesc_ be the PropertyDescriptor { [[Value]]: _errorsArray_, [[Writable]]: *true*, [[Enumerable]]: *false*, [[Configurable]]: *true* }.

Choose a reason for hiding this comment

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

Is there a reason not to just use ? Construct(%AggregateError%, « errors »), since the %AggregateError% constructor already has the defined semantics?

Copy link
Member

Choose a reason for hiding this comment

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

what about the message argument?

Choose a reason for hiding this comment

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

We don't seem to care about message elsewhere in the spec as the actual message is implementation dependent and may even be localised, yet this isn't formalized anywhere in the spec for the numerous "throw a TypeError exception" cases. I was wondering if we need to be more formal about this in general. Something like:

  1. Let message be a host implementation defined message.
  2. Perform ? Throw(%Error%, « message »).

Copy link
Member

Choose a reason for hiding this comment

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

i suspect creation of errors typically avoids using Construct precisely to avoid the need for that formalism.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Not just message, but also stack, line, and any other non-standard properties.

1. Perform ! DefinePropertyOrThrow ( _error_, `"errors"`, _errorsDesc_ ).
1. Return ThrowCompletion ( _error_ ).
</emu-alg>
</emu-clause>

<emu-clause id="sec-native-error-types-used-in-this-standard-aggregateerror">
<h1>AggregateError ( errors, message )</h1>
<p>When the *AggregateError* function is called with arguments _errors_ and _message_, the following steps are taken:</p>
Expand All @@ -132,7 +139,8 @@ <h1>AggregateError ( errors, message )</h1>
1. Let _errorsIterator_ be ? GetMethod(_errors_, @@iterator).
1. Let _errorsList_ ? IterableToList(_errors_, _errorsIterator_).
1. Let _errorsArray_ be CreateArrayFromList(_errorsList_).
1. Perform ! CreateDataProperty(_O_, `"errors"`, _errorsArray_).
1. Let _errorsDesc_ be the PropertyDescriptor { [[Value]]: _errorsArray_, [[Writable]]: *true*, [[Enumerable]]: *false*, [[Configurable]]: *true* }.
Copy link
Member

Choose a reason for hiding this comment

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

maybe these two steps should be their own abstract op, so it can be reused in both places?

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 just found other places in current ecma262 spec text where such abstract operation could be reused (plus one usage for message property 3 lines below these two lines). It will need a lot arguments though. IMO duplication here is not a big problem but maybe worth a separate PR to the spec repo

Copy link
Member

Choose a reason for hiding this comment

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

Fair point.

Choose a reason for hiding this comment

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

"CreateBuiltinDataPropertyOrThrow"? Naming based on "CreateBuiltinFunction" and "CreateDataPropertyOrThrow", given that the majority of "built in" data properties are non-enumerable by default.

1. Perform ! DefinePropertyOrThrow ( _O_, `"errors"`, _errorsDesc_ ).
1. If _message_ is not _undefined_, then
1. Let msg be ? ToString(_message_).
1. Let msgDesc be the PropertyDescriptor { [[Value]]: _msg_, [[Writable]]: *true*, [[Enumerable]]: *false*, [[Configurable]]: *true* }.
Expand Down