Moving Error.stack to Error.prototype?

[raphael.ku...@intel.com]

Hi, everyone,

I recently filed and started working on https://bugs.chromium.org/p/chromium/issues/detail?id=737497, which is about Blink's handling of DOMException and how it deviated from the WebIDL spec. It ended up prompting some spec changes (under review), one of which is that non-standard %Error% properties (such as stack) should be added to DOMException even though the latter does not inherit from the former. DOMException's prototype should inherit from %ErrorPrototype% though, so moving stack there would automatically solve the problem for us in Blink-land.

I also stumbled upon https://github.com/tc39/proposal-error-stacks, and it looks like Firefox already does this, and the proposal also goes in that direction.

Would this change be OK with the V8 team? Should I send an intent-to-implement email once the spec/web-platform-tests changes are merged?

[Jakob Kummerow]

+Yang (who's OOO so might take a while to respond)

--
--
v8-dev mailing list
v8-...@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

[Adam Klein]

+domenic

My reading of the proposed spec change (https://github.com/heycam/webidl/pull/378) is that it's meant to allow V8's current behavior, in which DOMException gets an own property named "stack". The relevant bit of spec text is:

```

If an implementation gives native Error objects special powers or

nonstandard properties (such as a `stack` property), it should also expose those on

DOMException instances.

```

While the TC39 error stacks proposal would move this to the prototype, that proposal is still at an early point (stage 1) in the standardization process, so it would be premature to change our implementation for the purpose of being compliant with that proposal.

So my reaction is: I don't think it's worth making any change to our stack property, for now. Domenic, does that seem reasonable to you? Does that match your intent with your PR?

On Thu, Jun 29, 2017 at 2:20 PM, Jakob Kummerow <jkum...@chromium.org> wrote:

+Yang (who's OOO so might take a while to respond)

[raphael.ku...@intel.com]

This is mostly an FYI, but keeping stack in Error itself leads to some extra work in Blink: since DOMException does not inherit from Error we need to create our own stack accessor, pass it a v8::Exception::Error and get/set this Error's stack value instead. And we currently do that only when Blink itself throws a DOMException, not when it is created (which is a bug whose fix may require some custom bindings code).

On Thursday, 29 June 2017 14:30:21 UTC+2, Adam Klein wrote:

+domenic

My reading of the proposed spec change (https://github.com/heycam/webidl/pull/378) is that it's meant to allow V8's current behavior, in which DOMException gets an own property named "stack". The relevant bit of spec text is:

```

If an implementation gives native Error objects special powers or

nonstandard properties (such as a `stack` property), it should also expose those on

DOMException instances.

```

While the TC39 error stacks proposal would move this to the prototype, that proposal is still at an early point (stage 1) in the standardization process, so it would be premature to change our implementation for the purpose of being compliant with that proposal.

So my reaction is: I don't think it's worth making any change to our stack property, for now. Domenic, does that seem reasonable to you? Does that match your intent with your PR?

On Thu, Jun 29, 2017 at 2:20 PM, Jakob Kummerow <jkum...@chromium.org> wrote:

+Yang (who's OOO so might take a while to respond)

On Thu, Jun 29, 2017 at 1:33 PM, <raphael.ku...@intel.com> wrote:

Hi, everyone,

I recently filed and started working on https://bugs.chromium.org/p/chromium/issues/detail?id=737497, which is about Blink's handling of DOMException and how it deviated from the WebIDL spec. It ended up prompting some spec changes (under review), one of which is that non-standard %Error% properties (such as stack) should be added to DOMException even though the latter does not inherit from the former. DOMException's prototype should inherit from %ErrorPrototype% though, so moving stack there would automatically solve the problem for us in Blink-land.

I also stumbled upon https://github.com/tc39/proposal-error-stacks, and it looks like Firefox already does this, and the proposal also goes in that direction.

Would this change be OK with the V8 team? Should I send an intent-to-implement email once the spec/web-platform-tests changes are merged?

--
--
v8-dev mailing list
v8-...@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+un...@googlegroups.com.

For more options, visit https://groups.google.com/d/optout.

--
--
v8-dev mailing list
v8-...@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.

To unsubscribe from this group and stop receiving emails from it, send an email to v8-dev+un...@googlegroups.com.

[Jakob Gruber]

On Thu, Jun 29, 2017 at 2:41 PM, <raphael.kubo.da.costa@intel.com> wrote:

This is mostly an FYI, but keeping stack in Error itself leads to some extra work in Blink: since DOMException does not inherit from Error we need to create our own stack accessor, pass it a v8::Exception::Error and get/set this Error's stack value instead. And we currently do that only when Blink itself throws a DOMException, not when it is created (which is a bug whose fix may require some custom bindings code).

I'm still confused what this is trying to achieve. 

The https://github.com/tc39/proposal-error-stacks proposal does you the `stack` getter on `Error.prototype`, but it would only actually format a stack trace when called on a real `Error` object (or a subclass) that has an [[ErrorData] internal slot.

Calling the proposed `Error.prototype.stack` getter on a (non-Error subclass) `DomException` would just return `undefined`.

[raphael.ku...@intel.com]

On Thursday, 29 June 2017 15:02:09 UTC+2, Jakob Gruber wrote:

Hmm you do have a point. That means we'll either have to continue doing what we do or go back to initializing DOMException out of an Error (which is one of the things https://github.com/heycam/webidl/pull/378 is getting rid of because no browser has properly done that so far).

[Domenic Denicola]

Right, given that stack is nonstandard, it's up to the implementation how it works. There's a "should"-level requirement that it work the same way between Error and DOMException, in a vague sense. So either implementation strategy would work.

That said I think putting in on the prototype makes a lot more sense from a JS developer's point of view.

As for the [[ErrorData]] issue, if the TC39 proposal moves forward DOMException would certainly gain [[ErrorData]]. Right now it does not have [[ErrorData]] since adding it would have no useful impact.

[Yang Guo]

I wonder whether the spec proposal will include a way to programmatically access the stack trace similar to V8's Error.prepareStackTrace API, and whether there is any performance considerations/requirements. The former has a few security implications, and the latter is observable and getter/setter obviously behave diffferently than actual properties.

V8's current implementation centers around these issues. To be performant, we format stack traces lazily and cache the result. It has some rough edges if you look into edge cases, and we also have a known memory leak when we hold onto data structures for stack traces that are not yet formatted. This could be solved, but is not yet: https://bugs.chromium.org/p/v8/issues/detail?id=4065&q=owner%3Aulan%40chromium.org%20weak&colspec=ID%20Type%20Status%20Priority%20Owner%20Summary%20HW%20OS%20Component%20Stars

I assume that a spec proposal need to sufficiently answer all these issues before setting things in stone.

Cheers,

Yang