getOwnPropertyDescriptor side effects

# Francisco Tolmasky (8 years ago)

Is there any position on whether getOwnPropertyDescriptor should not have side-effects? I ask because some v8 getOwnPropertyDescriptor do have side effects (for example, Object.getOwnPropertyDescriptor(new Error, “stack”) will call Error.prepareStackTrace (it used to not)). Currently I treat getOwnPropertyDescriptor as a “safe” way to observe an object — (unlike say calling each member on an object which could set off getters).

# Adam Klein (8 years ago)

With the advent of Proxy in ES2015, getOwnPropertyDescriptor can always have side-effects:

Object.getOwnPropertyDescriptor(
  new Proxy({}, {
    getOwnPropertyDescriptor() {
      throw "hello world"
    }
  }),
  "foo"
)
# Raul-Sebastian Mihăilă (8 years ago)

That sounds like a bug because error objects are ordinary objects and the [[GetOwnProperty]] internal method of ordinary objects doesn't have side effects.

# Isiah Meadows (8 years ago)

First, the stack property is non-standard. Second, proxies don't have to be stateful, and many of them aren't.

# Raul-Sebastian Mihăilă (8 years ago)

Even if the stack property is non-standard, getting its property descriptor must follow the same general rules for ordinary objects.

# Boris Zbarsky (8 years ago)

On 1/10/17 1:56 AM, Raul-Sebastian Mihăilă wrote:

Even if the stack property 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)...

# Raul-Sebastian Mihăilă (8 years ago)

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.

# Boris Zbarsky (8 years ago)

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.

# Raul-Sebastian Mihăilă (8 years ago)

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.

# Michał Wadas (8 years ago)

Implementations are allowed to extend objects. Otherwise presence of global/console/// would violate spec...

# Boris Zbarsky (8 years ago)

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.

# Michał Wadas (8 years ago)

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.

# Isiah Meadows (8 years ago)

To clarify, what engine has the bug here? I've lost that context.

# Michał Wadas (8 years ago)

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.

# Allen Wirfs-Brock (8 years ago)

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 stack property 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.

# Boris Zbarsky (8 years ago)

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.

# Raul-Sebastian Mihăilă (8 years ago)

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.

# Isiah Meadows (8 years ago)

Not if it's (likely) throwing from the new Error.

# Isiah Meadows (8 years ago)

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.

# Boris Zbarsky (8 years ago)

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....

# Isiah Meadows (8 years ago)

Okay. The error stack being constructed that early is odd, though.

# Boris Zbarsky (8 years ago)

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?

# Isiah Meadows (8 years ago)

I was expecting the error to throw on invoking the getter. Calling Object.getOwnPropertyDescriptor should never do that (spec invariant).

# Jordan Harband (8 years ago)

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.

# Boris Zbarsky (8 years ago)

On 1/11/17 8:55 PM, Isiah Meadows wrote:

I was expecting the error to throw on invoking the getter. Calling Object.getOwnPropertyDescriptor should 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.

# T.J. Crowder (8 years ago)

So to sum up, then, and circle back to Francisco Tolmasky's original question:

  • For ordinary objects, Object.getOwnPropertyDescriptor shouldn'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 Proxy example.
  • Error objects are specified as ordinary objects.
  • V8's Error object has a stack property that claims to be a value property (not an accessor).
  • V8's Error object 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 for stack (it has to, in order to provide the value property of the descriptor, since stack claims to be a value property).
  • This aspect of V8's Error could be in-spec by making stack an 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.

# Isiah Meadows (8 years ago)

Okay, so it's a V8 bug. Filed it here: bugs.chromium.org/p/v8/issues/detail?id=5834

# Jordan Harband (8 years ago)

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.

# Isiah Meadows (8 years ago)

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.)

# Boris Zbarsky (8 years ago)

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:

  1. The getter does NOT throw on a non-Error receiver. Doing that would be very much not web-compatible.
  2. 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.

  1. 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.

  2. 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.

# Isiah Meadows (8 years ago)

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:

  1. What does Firefox do with the getter on non-errors? Does it delegate to the own stack? (I'm not familiar)
  2. 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__ or Object.defineProperty to even access. Also, V8 returns a lazily computed value descriptor (spec violation, unlikely to be fixed before this is implemented).
# Boris Zbarsky (8 years ago)

On 1/19/17 12:24 PM, Isiah Meadows wrote:

  1. 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.

  1. 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__ or Object.defineProperty to 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.

# Isiah Meadows (8 years ago)

Thanks! I see now.

# Mark S. Miller (8 years ago)

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:

  1. 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.

  1. 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.

# Mark Miller (8 years ago)

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:

  1. 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.

  1. 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

# Jordan Harband (8 years ago)

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.

# Boris Zbarsky (8 years ago)

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.

# Jordan Harband (8 years ago)

In that case I would be delighted if you filed this as an issue on the repo.

# Boris Zbarsky (8 years ago)

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.

See ljharb/proposal-error-stacks#3

# Boris Zbarsky (8 years ago)

On 1/21/17 1:19 AM, Boris Zbarsky wrote:

See ljharb/proposal-error-stacks#3

I guess that doesn't cover the setter. I filed ljharb/proposal-error-stacks#7