getOwnPropertyDescriptor side effects
With the advent of Proxy in ES2015, getOwnPropertyDescriptor can always have side-effects:
Object.getOwnPropertyDescriptor(
new Proxy({}, {
getOwnPropertyDescriptor() {
throw "hello world"
}
}),
"foo"
)
That sounds like a bug because error objects are ordinary objects and the
[[GetOwnProperty]] internal method of ordinary objects doesn't have side
effects.
First, the stack property is non-standard. Second, proxies don't have to
be stateful, and many of them aren't.
Even if the stack property is non-standard, getting its property
descriptor must follow the same general rules for ordinary objects.
On 1/10/17 1:56 AM, Raul-Sebastian Mihăilă wrote:
Even if the
stackproperty is non-standard, getting its property descriptor must follow the same general rules for ordinary objects.
Not if its (non-standard) implementation makes the object itself non-ordinary (again, non-standard)...
Do you mean that an implementation is allowed to return an exotic object from the Error constructor?
tc39.github.io/ecma262/#sec-error-message
The Error constructor calls OrdinaryCreateFromConstructor in step 2.
tc39.github.io/ecma262/#sec-ordinarycreatefromconstructor
According to its definition, OrdinaryCreateFromConstructor creates an ordinary object.
Not returning an ordinary object from the Error constructor is non-conformant and, assuming that conformance is a requirement for V8, it's a bug.
Just because an implementation adds a non-standard property to an ordinary object, even if its value is an exotic object, it doesn't turn the ordinary object into an exotic object.
On 1/10/17 2:10 PM, Raul-Sebastian Mihăilă wrote:
Do you mean that an implementation is allowed to return an exotic object from the Error constructor?
No, I'm saying some implementations do that, because they want to implement a non-standard "stack" property and the only way to get reasonable behavior (as those implementations define "reasonable") for it is to have the object be an exotic object.
There are other implementation strategies for "stack" that don't involve an exotic Error object, of course (e.g. SpiderMonkey implements it as an accessor property on Error.prototype). They have their own tradeoffs, though.
Not returning an ordinary object from the Error constructor is non-conformant and, assuming that conformance is a requirement for V8, it's a bug.
Sure, just like arguably the presence of the "stack" property to start with is a bug, because per spec it's totally unexpected.
I disagree regarding the conformance. According to the conformance section ( tc39.github.io/ecma262/#sec-conformance):
A conforming implementation of ECMAScript may provide additional types, values, objects, properties, and functions beyond those described in this specification. In particular, a conforming implementation of ECMAScript may provide properties not described in this specification, and values for those properties, for objects that are described in this specification.
Implementations are allowed to extend objects. Otherwise presence of global/console/// would violate spec...
On 1/10/17 2:31 PM, Michał Wadas wrote:
Implementations are allowed to extend objects. Otherwise presence of global/console/// would violate spec...
www.ecma-international.org/ecma-262/6.0/#sec-global-object explicitly says that the global object may have additional properties, so global .console is clearly not a spec violation.
Actually here spec repeats itself because...
A conforming implementation of ECMAScript may provide additional types, values, objects, properties, and functions beyond those described in this specification. In particular, a conforming implementation of ECMAScript may provide properties not described in this specification, and values for those properties, for objects that are described in this specification.
So implementation is explicitly allowed to add new properties on objects.
Though, internal methods and internal slots are not properties:
Internal slots correspond to internal state that is associated with objects and used by various ECMAScript specification algorithms. Internal slots are not object properties and they are not inherited.
So it's spec violation to have custom [[GetOwnProperty]] implementation on ordinary objects.
To clarify, what engine has the bug here? I've lost that context.
V8 have bug.
Reproduction code:
Error.prepareStackTrace = ()=>{throw 123}
Object.getOwnPropertyDescriptor(new Error, 'stack') // throws 123
It should be probably filled on V8 bug tracker.
On Jan 10, 2017, at 9:57 AM, Boris Zbarsky <bzbarsky at MIT.EDU> wrote:
On 1/10/17 1:56 AM, Raul-Sebastian Mihăilă wrote:
Even if the
stackproperty is non-standard, getting its property descriptor must follow the same general rules for ordinary objects.Not if its (non-standard) implementation makes the object itself non-ordinary (again, non-standard)…
Right, but making it an accessor property would be a standards confirming extension.
On 1/10/17 2:54 PM, Michał Wadas wrote:
Internal slots correspond to internal state that is associated with objects and used by various ECMAScript specification algorithms. Internal slots are not object properties and they are not inherited.
OK, so having an internal slot for the value of .stack would be a spec violation too, yes?
So it's spec violation to have custom [[GetOwnProperty]] implementation on ordinary objects.
Sure. Ideally such things would be brought to the committee so the spec could be adjusted as needed for real-life (or implementations changed if real life does not actually require the exotic behavior). That's what's happened in the past for things like the global object.
Really, .stack or equivalent just needs to be standardized; then this will all get sorted out.
tc39.github.io/ecma262/#sec-object-internal-methods-and-internal-slots
The actual semantics of objects, in ECMAScript, are specified via algorithms called internal methods. Each object in an ECMAScript engine is associated with a set of internal methods that defines its runtime behaviour. These internal methods are not part of the ECMAScript language. They are defined by this specification purely for expository purposes. However, each object within an implementation of ECMAScript must behave as specified by the internal methods associated with it. The exact manner in which this is accomplished is determined by the implementation.
So, the sentence 'having an internal slot for the value of .stack would be a spec violation' doesn't make sense to me because the slots are a specification concept. I guess an implementation can represent this concept one way or another internally, but it doesn't matter as long as the implementation behaves as specified.
Not if it's (likely) throwing from the new Error.
Really, the only thing that matters is in fact observable behavior, so I'd concur. I just get the feeling several people in this thread don't quite understand that part of the spec, or in a few cases, the spec itself and its invariants.
On 1/11/17 6:43 AM, Isiah Meadows wrote:
Not if it's (likely) throwing from the
new Error.
It's not. The "stack" property in V8 quacks like a value property for the most part, but the first access to it invokes some code that does the (lazy) stack string construction. That process involves calling Error.prepareStackTrace if such a thing exists.
Specifically, as of today, see v8/v8/blob/d5a0860e87b5f8d88432cf628f4bbc0cc922317f/src/messages.cc#L927-L954 which is called from v8/v8/blob/d5a0860e87b5f8d88432cf628f4bbc0cc922317f/src/accessors.cc#L1169
The whole setup is basically designed to have things that look like data properties but actually involve executing code to compute the property value (and possibly executing code when the "value" property is set).
SpiderMonkey has similar things as well, though we've been getting rid of them as much as possible. The obvious one that remains is .length on Array objects. This allows Array objects to be non-exotic for practical purposes in terms of their engine representation, and hence not suffer the performance penalties exotic objects suffer. In spec terms, of course, Array instances are just exotic objects. In an ideal world, the implementation detail is just that and is not observable....
Okay. The error stack being constructed that early is odd, though.
On 1/11/17 3:12 PM, Isiah Meadows wrote:
Okay. The error stack being constructed that early is odd, though.
I'm not sure I follow. The error stack in SpiderMonkey and V8 (and JavaScriptCore too, afaict) is captured at the point when the Error object is created. The captured thing is information that can be used to construct a stack string later.
Then getting .stack constructs the stack string. This operation is somewhat expensive, so is deferred until someone asks.
In V8, the stringification process includes an explicit script-modifiable hook: the "prepareStackTrace" property of the Error constructor.
Is the odd part the stack capture during Error object construction? Were you expecting it to only be captured at the throw point?
I was expecting the error to throw on invoking the getter. Calling
Object.getOwnPropertyDescriptor should never do that (spec invariant).
Per tc39.github.io/ecma262/#sec-object.getownpropertydescriptor
, Object.getOwnPropertyDescriptor will throw if you pass it null or
undefined as the first argument, if you pass it something as the second
argument that can't be coerced to a primitive (ie, a valueOf or toString
throws, or both are missing), or if the object you pass is a Proxy (or
other exotic object) whose [[GetOwnProperty]] trap throws or returns
anything other than an Object or undefined.
On 1/11/17 8:55 PM, Isiah Meadows wrote:
I was expecting the error to throw on invoking the getter. Calling
Object.getOwnPropertyDescriptorshould never do that (spec invariant).
There is no getter, from the JS point of view. It's a value property. That's the whole point of this conversation.
We seem to be in violent agreement that what v8 is doing is a spec violation, fwiw.
So to sum up, then, and circle back to Francisco Tolmasky's original question:
- For ordinary objects,
Object.getOwnPropertyDescriptorshouldn't have side-effects because none of the ordinary operations it uses has side effects. - For exotic objects, it may well have side effects as a result of an
exotic version of [[GetOwnProperty]]; for instance, Adam Klein's
Proxyexample. Errorobjects are specified as ordinary objects.- V8's
Errorobject has astackproperty that claims to be a value property (not an accessor). - V8's
Errorobject is a exotic object, it has exotic behavior for [[GetOwnProperty]], because it triggers filling in the string for the captured stack trace if you call it forstack(it has to, in order to provide thevalueproperty of the descriptor, sincestackclaims to be a value property). - This aspect of V8's
Errorcould be in-spec by makingstackan accessor instead (or by building the string earlier, but it's deferred for performance reasons).
Is that a reasonable summary?
Additionally, I believe the only exotic object defined by the
specification that has a [[GetOwnProperty]] with potential side
effects is Proxy.
Provided that's all correct, Francisco's answer is: Per spec, you
can't rely on Object.getOwnPropertyDescriptor not having side
effects unless you can guarantee you're not dealing with a Proxy.
Per spec, you could for non-Proxy objects defined by the
specification, but that's not currently the case with V8 (at least).
And there's always the possibility of host objects having exotic
[[GetOwnProperty]] behavior.
-- T.J.
Okay, so it's a V8 bug. Filed it here: bugs.chromium.org/p/v8/issues/detail?id=5834
The beginnings of the Error Stacks proposal is now up at ljharb/proposal-error-stacks
I'm presenting it this month at TC39, hoping for it to be stage 1.
As its stands, the proposal would indeed make v8's behavior noncompliant, were it to become stage 4.
It still seems useful. My only nit is shouldn't they be static methods of
Error, not System? (They only deal with an error-specific internal
property, so it seems odd to put it in the generic system stuff.)
On 1/19/17 2:33 AM, Jordan Harband wrote:
The beginnings of the Error Stacks proposal is now up at ljharb/proposal-error-stacks
I can't speak for other browsers, but the description of the Firefox behavior in that proposal does not look correct.
Here's what I understand the Firefox behavior to be:
- The getter does NOT throw on a non-Error receiver. Doing that would be very much not web-compatible.
- The behavior of the getter is as follows:
a) If the receiver is not an object, throw. b) Walk up the prototype chain (note: this can invoke proxy [[GetPrototype]] traps) until we find either an Error object or Error.prototype. If we reach null before doing either of those, throw. c) Return the stack string for the object we found. For Error.prototype this would be the empty string; for an Error object it's the stack captured when it was created.
-
The setter doesn't care what the receiver is, as long as it's an object. Again, throwing for non-Error would not be web-compatible.
-
The actual behavior of the setter is to throw if called with no arguments. Otherwise, the setter invokes its receiver's [[DefineOwnProperty]] with the property name "stack" and a property descriptor that looks like this:
{ [[Value]]: setterArg, [[Configurable]]: true, [[Writable]]: true, [[Enumerable]]: true }
where setterArg is the first argument that was passed to the setter.
I should note, per items 1 and 3 above, that the proposal at ljharb.github.io/proposal-error-stacks as of today is in fact not web-compatible.
I agree that no setter (I didn't see one in the spec) is not Web compatible. In fact, I recall Bluebird running into serious issues with erroneous warnings over read-only stacks in PhantomJS, and had to disable them in my tests as a result. (I eventually dropped the dependency in favor of native promises + a smaller polyfill, but not until later.)
But I do have a couple questions:
- What does Firefox do with the getter on non-errors? Does it delegate to
the own
stack? (I'm not familiar) - How breaking is having the getter and setter throwing on non-errors? I'm
struggling to see how it'd be that breaking. It's a getter, not a method,
so it requires a call to
__locateGetter__orObject.definePropertyto even access. Also, V8 returns a lazily computed value descriptor (spec violation, unlikely to be fixed before this is implemented).
On 1/19/17 12:24 PM, Isiah Meadows wrote:
- What does Firefox do with the getter on non-errors?
Oh, I didn't make that clear, did I? On a non-error in the getter, we have an Error object or Error.prototype (which appeared somewhere on our receiver's proto chain). Those objects all have, in Spidermonkey, an internal slot that stores information about the stack. The getter uses the information in that internal slot to create a string and return it.
- How breaking is having the getter and setter throwing on non-errors?
Well, when we tried to do it it didn't even pass our test automation, so...
In particular it would throw on anyone doing ES5-style subclassing of Error and then doing anything with .stack.
I'm struggling to see how it'd be that breaking. It's a getter, not a method, so it requires a call to
__locateGetter__orObject.definePropertyto even access.
No, it just requires that you have an Error on your prototype chain but not be an Error yourself, and suddenly you have exceptions everywhere.
Thanks! I see now.
On Thu, Jan 19, 2017 at 9:30 AM, Boris Zbarsky <bzbarsky at mit.edu> wrote:
On 1/19/17 12:24 PM, Isiah Meadows wrote:
- What does Firefox do with the getter on non-errors?
Oh, I didn't make that clear, did I? On a non-error in the getter, we have an Error object or Error.prototype (which appeared somewhere on our receiver's proto chain). Those objects all have, in Spidermonkey, an internal slot that stores information about the stack. The getter uses the information in that internal slot to create a string and return it.
- How breaking is having the getter and setter throwing on non-errors?
Well, when we tried to do it it didn't even pass our test automation, so...
In particular it would throw on anyone doing ES5-style subclassing of Error and then doing anything with .stack.
That makes perfect sense.
We could have the Error.prototype.getter not throw but have System.getStack throw. There was no strong reason for the getter and System.getStack to have precisely the same behavior; it was just that there was no reason not to. Now there is. Thanks.
On Thu, Jan 19, 2017 at 10:52 AM, Mark S. Miller <erights at google.com> wrote:
On Thu, Jan 19, 2017 at 9:30 AM, Boris Zbarsky <bzbarsky at mit.edu> wrote:
On 1/19/17 12:24 PM, Isiah Meadows wrote:
- What does Firefox do with the getter on non-errors?
Oh, I didn't make that clear, did I? On a non-error in the getter, we have an Error object or Error.prototype (which appeared somewhere on our receiver's proto chain). Those objects all have, in Spidermonkey, an internal slot that stores information about the stack. The getter uses the information in that internal slot to create a string and return it.
- How breaking is having the getter and setter throwing on non-errors?
Well, when we tried to do it it didn't even pass our test automation, so...
In particular it would throw on anyone doing ES5-style subclassing of Error and then doing anything with .stack.
That makes perfect sense.
We could have the Error.prototype.getter
I meant: the Error.prototype.stack getter
If you have feedback on the proposal, please file issues on the repo instead of replying in the thread.
Also note that for changing existing behavior to be "web incompatible", all the browsers have to agree on it - in other words, if you're currently relying on behavior that only works in some browsers (as opposed to all), then it's totally fine for your code to be broken by a spec change.
On 1/20/17 2:26 PM, Jordan Harband wrote:
Also note that for changing existing behavior to be "web incompatible", all the browsers have to agree on it
That may or may not be true.
But in any case, this proposal specifies behavior that disagrees with all browsers, who agree with each other. So it's "web incompatible" even by your very restrictive definition.
In that case I would be delighted if you filed this as an issue on the repo.
On 1/21/17 1:14 AM, Jordan Harband wrote:
In that case I would be delighted if you filed this as an issue on the repo.
I would be delighted to do it, if it had not been filed back in November, including the comments about how your description of the Firefox behavior doesn't match the actual behavior.
On 1/21/17 1:19 AM, Boris Zbarsky wrote:
I guess that doesn't cover the setter. I filed ljharb/proposal-error-stacks#7
Is there any position on whether
getOwnPropertyDescriptorshould not have side-effects? I ask because some v8getOwnPropertyDescriptordo have side effects (for example,Object.getOwnPropertyDescriptor(new Error, “stack”)will callError.prepareStackTrace(it used to not)). Currently I treatgetOwnPropertyDescriptoras a “safe” way to observe an object — (unlike say calling each member on an object which could set off getters).