Should errors be a prototype accessor that returns the value of an internal slot?

[ljharb]

Currently the proposal has “errors” as an own property that’s a normal array. Are there thoughts on making it a prototype getter that constructs an array from the contents of an internal slot?

[bakkot]

Is there any reason at all to do so? If I recall correctly we discussed the possibility of doing something similar with the .groups property of regex match objects and decided to go with a regular property, which seems like a reasonable precedent to follow here.

[ljharb]

That’s because match objects are normal arrays already and have no prototype; the primary precedent for own data properties (that admittedly is highly relevant) is the message property on Errors. Most things are accessors pulling from slots.

[mathiasbynens]

@hashseed @bmeurer Do you have an opinion here?

[bmeurer]

I'd strongly be in favor to have these on the prototype.

[ljharb]

@bmeurer are there any concrete reasons to avoid an own property for “errors”?

[bmeurer]

Actually, scratch my last comment, my thinking was wrong here. I thought it might be better for implementations to have a prototype getter so that the real errors can be computed lazily, and we don't need to do this weird dance that we have to do with Error.stack in order to get it right.

For the AggregateError, just having on the instance makes a lot more sense, since there's no point in trying to delay the actual value computation.

[mathiasbynens]

Okay, I'm inclined to close this one then. The current spec has errors as an own property (via CreateMethodProperty), which seems fine since its value is never lazily computed. It also matches the message property.

[ljharb]

"lazily computed" isn't the bar for things being on the prototype, though. "message" is an own property, but that seems to be a rare occurrence. RegExps, for example, use slot-reading accessors in ES6+ even for things that were own properties in prior editions.

[jfparadis]

Maybe modeling errors on message for consistency isn't quite the right base.

Error.prototype.message is defined as the empty string, while Error.prototype.errors remains undefined in this proposal. This means errors behaves less like message and more like stack.

Whether Error.prototype.errors remains undefined or becomes an accessor, our uses cases are not affected.

[ljharb]

Given that the two consistencies being chosen between are actually:

1. like message: own data property, plus a prototype data property for the default
2. like most other things: prototype accessor property

… and the first one would cause issues for Moddable and SES (due to the so-called "override mistake"), that suggests to me that it should be an accessor.

[ljharb]

Some relevant examples for instances of builtin things:

prototype accessors

• Error.prototype.name
• Symbol.prototype.decription
• Map.prototype.size
• Set.prototype.size
• RegExp.prototype.flags
• RegExp.prototype.dotAll
• RegExp.prototype.global
• RegExp.prototype.source
• RegExp.prototype.sticky
• RegExp.prototype.unicode
• RegExp.prototype.multiline
• RegExp.prototype.ignoreCase
• Object.prototype.__proto__
• DataView.prototype.buffer
• DataView.prototype.length
• DataView.prototype.byteOffset
• ArrayBuffer.prototype.byteLength
• TypedArray .buffer
• TypedArray .length
• TypedArray .byteLength
• TypedArray .byteOffset
• SharedArrayBuffer.prototype.byteLength

own data properties with defaults on the prototype

• Function.prototype.name
• Function.prototype.length
• Error.prototype.message
• Array.prototype.length, but only because it's an exotic array object

own data properties without defaults on the prototype

• array indices
• string indices

I've omitted things that aren't instances (iterator results, match objects) and also symbol methods on the prototype.

(if i've forgotten anything, please lmk and i'll update the list)

[rbuckton]

If message was an accessor, I'd say that errors should be an accessor, but I'd prefer consistency with message and make errors an own property.

[rbuckton]

Is there additional information on what is meant by "an override mistake"?

[ljharb]

@rbuckton instead of consistency with Errors' name?

[hax]

I have to say "own data properties with defaults on the prototype" is very weird which made the shape (new Error(x).hasOwnProperty('message')) not stable. I don't like it spread to more place. But if it is "always own data property" I will be ok with it.

[hax]

@ljharb When will a function instance use name/length on prototype?

[ljharb]

@hax all anonymous functions lack a name property by spec, and Function.prototype is also itself a function.

[hax]

all anonymous functions lack a name property by spec,

@ljharb

But (function () {}).hasOwnProperty('name') return true...

[ljharb]

@hax oops, "by spec" is premature - that change is here, and will land once PR issues are resolved.

[hax]

It seems after that PR land, Error's message will be the only case which have "unstable" own property?

[ljharb]

Not sure what you mean by unstable; but that PR doesn’t change my list above.

[hax]

I mean instance.hasOwnProperty(property) would return true in some cases but return false (fallback to prototype) in some other cases.

[ljharb]

In that case, after the PR lands, function name will be sometimes-own/sometimes-prototype, Error message will be always-own, Error name will be always-prototype, etc.

[hax]

I'm confused, it seems the PR never touch Error? Why it will change Error message behavior (though I prefer that)?

[ljharb]

It won't; that's its current behavior (the prototype property is there, but shadowed by the own property on an instance)

[hax]

Error message will be always-own

@ljharb new Error().hasOwnProperty('message') is false

[erights]

@caridy said

I don't think the language should care about preventing communication channels

OMG @caridy I could not disagree more with what you said in this quote. However, I suspect that I may not disagree with what you meant.

ES3 had crazy communications channels, such as function.caller. At this point I have spent extraordinary efforts to kill that one, or rather, to quarantine it to sloppy mode and to enable sloppy mode itself to be suppressed. We have kept hidden mutable state out of the primordials as best we could, with the exception of the two grandfathered forms that we knew we could not suppress by default: Date.now() and Math.random(). We have spent considerable effort figuring out how to co-exist with this default.

An example of primordial hidden mutable state that we missed in ES5 is Date.prototype. In ES5, Date.prototype was itself an exotic date instance, with the internal slots that the date builtins mutate and observe, with no way to suppress its mutability. This caused a security leak in Google Caja, that we fixed in ES6, that is clearly only a communications channel --- unlike Date.now it provided no communications channel to the outside world. At the time, Caja had to fix this by replacing all the Date builtins. In ES6 we fixed this by making Date.prototype be a non-exotic normal object. We (including you and I) have worried about other remaining primordial hidden mutable state, such as timezone leaking through dates.

Another example is the legacy RegExp statics like RegExp.$1, that make globally available the results of the last regexp match by any regexp instance in that realm. Again, the fatal problem is "only" an information leak. It does not enable any I/O to the external world. Another is Error.prototype.stack. For both of these, we're taking care to make them normative optional so that they may be omitted on a conforming implementation. And making them deletable, so a shim starting on a system where they are present can remove them, leaving the resulting environment as one that conforms to the spec of an initial environment.

On the particular issue here, the slogan to remember is "If it is not fresh or frozen, then it is rotten". The array of error objects in the aggregate error is fresh. It is indeed consistent with the general language style to have this be a fresh mutable plain array of mutable error objects, just like the containing aggregate error object is mutable. The key thing is that it is fresh. Any fresh mutable object is a communications channel among those who share direct access to it. But the fact that it is fresh means that this sharing relationship must already exist, and must already constitute a communications channel --- hopefully a legitimately authorized one. The array only needs to be as fresh as the aggregate error. Unlike the current accessor spec text, the array need not be fresh on every access.

[erights]

Writing this down also helps highlight why storing this array in an exotic internal slot is more dangerous than most internal slots that are already in the spec. Consider all of the following:

• The ES5 era Date.prototype or the internal slot that remains on date instances
• RegExp.$1 and its ilk.
• The proposed internal slot on error objects that the Error.prototype.stack accessor accesses
• The one implicit in Math.random
• Date.now
• timezone info
• All the leaks I am aware of in Intl
• The current time proposed for the Temporal namespace object

Every single one of those above, even including the ones we've worked so hard to fix, leak only information. The proposed internal slot for aggregate errors provide access to first class mutable objects --- the array of component errors. Even if the array is frozen, the component errors it hold presumably are not.

The only other place I can think of where shared access to a first class object leaks though a hidden channel is templates, via the template cache. For this one case, we purposefully violated the normal ES style by specifying that this template consists of transitively frozen arrays of strings. Thus no mutable state and no communications channel.

I am not saying that, for example, membrane security would not be able to cope with this new internal slot, if we did go that way. But it is indeed a much harder case to think about than all of the bullets listed above.

My conclusion is that errors should be a configurable own data property holding a fresh plain mutable array of component errors. Whether the data property is writable or not, I don't care.

[caridy]

@caridy I could not disagree more with what you said in this quote. However, I suspect that I may not disagree with what you meant.

@erights: Probably to strong wording from my side, should have been more specific. We are in agreement there for sure.

My conclusion is that errors should be a configurable own data property holding a fresh plain mutable array of component errors. Whether the data property is writable or not, I don't care.

I also agree with that conclusion.

[chicoxyzzy]

It seems to me, that this issue should be discussed at March TC39 meeting. @mathiasbynens WDYT? Do you have time to present an update on Promise.any? If not, I can try to find time to prepare something for the June meeting.

[mathiasbynens]

I'd be happy to present this at a future TC39 meeting but it would be ideal to come to agreement in this thread beforehand if at all possible, to reduce plenary discussion time. @ljharb, what do you think? Would you be okay with this changing back to an own data property?

---

At the meeting, several of us kept asking "Why not simply make it an own data property? Then all the other worries are simply avoided. What's the benefit of doing anything else?" No one in the meeting had any idea why not.

I'm hearing echoes of the 2019-10-02 TC39 meeting:

PD: V8 and LBR expressed preference for messages as an own property, and there is nothing strong that says this should be an accessor for any reason. So i would put it the other way around is there any reason why it shouldn't be an own property?

[ljharb]

My reply in the notes remains applicable imo:

To be clear, consistency goes both ways. Consistency with message dictates an own property, consistency with the rest of the spec means an accessor.

I vastly prioritize consistency with the rest of the spec here. Own properties are rare (and I think, mostly all legacy), and we should keep them that way.

[bakkot]

For the accessors you're talking about, the value they're reflecting is otherwise observable, either by behavior or in other properties. That is not the case here. I do not think we need to copy that decision when the logic which justified it does not apply.

Contrast, for example, the decidedly non-legacy Symbol.toStringTag, which is a data property and which makes sense as a data property because it does not reflect any otherwise-accessible state.

[ljharb]

It's a data property on the prototype - not an own property - that contains a primitive; it's hardly similar to this case.

[bakkot]

The accessors you're talking about almost all contain primitives as well (and, again, much more importantly, they reflect values which are otherwise observable). They are also hardly similar to this case.

[erights]

For purposes of this thread, I refer to an exotic internal slot that proxies don't know about, like that on Date instances, as an "exotic internal slot".

My objection isn't to adding an accessor. My objection is to adding a new exotic internal slot,
especially one that violates an important invariant obeyed by all other standard exotic internal slots:

Today, all standard exotic internal slots only contain data. Shared access to get or set the slot only leaks information. Shared access does not imply the ability to communicate direct references to objects.

Even if it didn't violate this invariant, I am resistant to adding new exotic internal slots. If a proposed exotic internal slot can only hold data, then my resistance could yield in the face of a compelling enough need. The error stack proposal, for example, adds an internal slot to all error instances, so it is much more pervasive than what is proposed here. But the new internal error slot contains only data. The case for the error slot is compelling enough that I am in favor of it. (Ironically, @ljharb and I are its co-champions.) In the case proposed here, the own-property alternative demonstrates there is no compelling enough need.

Given that it also conveys direct access to objects, we simply should not add it. As to following spec precedent, that cuts both ways. Nothing in the existing spec violates this invariant, so precedent says not to violate it here either.

Let's do own property. It solves the problem without creating new problems.

[jorendorff]

@ljharb wrote:

I vastly prioritize consistency with the rest of the spec here. Own properties are rare (and I think, mostly all legacy), and we should keep them that way.

This came as a bit of a surprise to me and I wonder if the committee at large agrees it's the case.

I'm OK with either outcome here, but Mozilla's reluctant to ship this feature without TC39 approving a direction.

[erights]

I remain opposed to an exotic internal slot holding objects. There is no precedent and it is dangerous.

On Fri, May 22, 2020 at 7:01 AM Jason Orendorff ***@***.***> wrote: @ljharb <https://github.com/ljharb> wrote: I vastly prioritize consistency with the rest of the spec here. Own properties are rare (and I think, mostly all legacy), and we should keep them that way. This came as a bit of a surprise to me and I wonder if the committee at large agrees it's the case. I'm OK with either outcome here, but Mozilla's reluctant to ship this feature without TC39 approving a direction. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#38 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AACC3TCZFEQHKY6LFGCWQXTRS2ATZANCNFSM4I2OBKEQ> .

-- Cheers, --MarkM

[ljharb]

@erights is it dangerous only because it's on the prototype? iow, if it were an own accessor that returned the same mutable object every time from an internal slot (not that i'm advocating for this), would that be OK? If not, can you help me understand why not?

[erights]

Would the same behavior be described by saying that the accessor's getter function encapsulates the object, so the same getter always returns the same object regardless of how it is called? The primordial heap has none of these getters, since it has no error instances?

If both are true, it is likely safe. But I'm glad you're not advocating this position ;)

[ljharb]

I've thought some more about this, and had some more discussion with a few folks.

I feel very strongly that the current inability to robustly distinguish Error types is a mistake. My hope here was that AggregateError would not expand/worsen/repeat that mistake.

However, unless we plan to add many more than 6 additional new error types, what we'd end up with is two "clouds" of errors: the majority and legacy ones, that do not properly distinguish between themselves; and the new ones (which would likely just be AggregateError). That inconsistency doesn't seem worth the very minor victory of not repeating a mistake.

As such, I'll withdraw my objection to changing errors to be an own (configurable, writable, enumerable) property on AggregateError instances. In case I'm the only one who had an objection to it being an own data property, I've filed #64 making the change in the proposed spec.

[erights]

For that goal, I am currently in the vicinity of working on the error stack shim, with expectation of getting back to our error stack proposal. That does add a new internal slot to all error objects, which would enable one to reliably distinguish errors from everything else, but not distinguish between types of errors. I remember this was one of your motivations for co-championing it.

As always, I am reluctant to introduce a new internal slot, but I am in favor of this one. This slot does have the same problems that the existing exotic internal slots do. So what's different? Like the internal slot on Date and every other exotic internal slot, this one reveals only data. It cannot be used to hide and convey an object reference. That's why I am more relaxed about it than the one we just avoided (whew!) on AggregateError. Thank you!

[jorendorff]

@erights I don't object to the conclusion here, but aren't there a lot of internal slots that can be used to pass mutable JS state? I'm thinking of slots like obj.[[Prototype]], fun.[[Environment]], method.[[HomeObject]], boundFn.[[BoundArguments]], typedArr.[[ViewedArrayBuffer]], moduleNamespace.[[Module]].[[Realm]], proxy.[[ProxyHandler]], and so on. The DOM also seems to have a lot of similar slots.

Asking because iterator helpers may end up being specified as new kinds of exotic objects, with an internal slot that holds the source iterator. (Edit: these objects would be ordinary, but they would have an internal slot that refers to another object.)

[littledan]

I'd like to have a bit of a broader discussion here before concluding that, in general, we wouldn't have internal slots with objects in them. I'm OK with this applying to errors for reasons specific to the conventions around errors, but more skeptical about this applying to JS objects overall.

I think internal slots are very normal--private fields give user code the equivalent capability--and I'm surprised by the discussion of them being "exotic". This has implications for other proposals, such as Intl.Segmenter and Temporal. I don't understand what kinds of needs are considered compelling enough, and why we're reversing course on the ES6 conventions to move from data properties towards internal slots (which feels compelling enough to me).

[littledan]

In the June 2020 TC39 meeting, I tried to express that I was only comfortable settling on an own data property if this was based on some reason other than the exotic internal slot argument. I didn't fully understand if there were such other reasons; at the time, I assumed there were, but now I'm having trouble finding them. Does anyone have any pointers here?

[bakkot]

@littledan Above @jorendorff gives a couple of reasons. My argument is that I think we should avoid adding brands unnecessarily and own data properties are the least idiosyncratic option given that these don't reflect any otherwise-available internal state and since implementors don't see value in delaying the computation.

[littledan]

OK, I'm comfortable with this conclusion then.

[littledan]

Conclusion of some further analysis of the "exotic internal slot hazard" noted at tc39/proposal-intl-segmenter#96 (comment) . tl;dr it doesn't seem like this should be a concern for Promise.any or AggregateError.