1740472 - Consider making Error.prototype.stack an own data property

Status: NEW

(Core :: JavaScript Engine, task, P3)

Description

[Jan de Mooij [:jandem]]

The stack property of Error objects currently lives on the prototype as an accessor property. JSC and V8 have this as an own data property.

This can result in compat issues, as reported on Twitter: https://twitter.com/fvsch/status/1458369672349470722

var err = new Error();
Object.defineProperty(err, 'stack', { value: 'hello', writable: false });
Object.defineProperty(err, 'stack', { value: 'goodbye', writable: false });

Chrome: I’ll allow it.
Firefox: Uncaught TypeError: can't redefine non-configurable property "stack".

The thread mentions this makes it harder to run certain node.js libraries in Firefox.

We might be able to implement this as a custom data property...

Comment 1

[Jan de Mooij [:jandem]]

I'm curious how people feel about this...

Comment 2

[Jan de Mooij [:jandem]]

Attachment: WIP: Bug 1740472 - WIP: Error.stack own data property

Comment 3

[Jan de Mooij [:jandem]]

I posted a simple prototype that uses a resolve hook instead of a getter/setter on the prototype. This probably needs additional changes in the browser, for example Xrays...

https://treeherder.mozilla.org/jobs?repo=try&group_state=expanded&revision=097996a565b23f483c109fe48cd0d8d67396da4a

Comment 4

[Tom S [:evilpie]]

I am pretty sure this is going to make some people unhappy. One of the main advantages of an accessor is that is deletable, this allows censoring for some "safe javascript" approaches.

There is also an existing proposal that defines it as an accessor: https://github.com/tc39/proposal-error-stacks/. I would suggest first opening an issue there with this problem.

Comment 5

[Jan de Mooij [:jandem]]

Good points, this definitely needs more discussion. If there really is Node code depending on V8's current behavior that's not a great sign though :/

Comment 6

[André Bargull [:anba]]

Adding another slot to ErrorObject will also cause performance regressions (bug 1691426).

Comment 7

[Florens Verschelde :fvsch]

Hi, Florens (fvsch) from StackBlitz here. We’re running code written for Node.js directly in the browser using the browser's JavaScript VM. We started with Chrome/Blink compat and have partial compat for Firefox (and hopefully Safari when they ship SharedArrayBuffer).

We’re running into issues with Node.js packages that use the V8-specific stack trace API, including:

1. Error.prepareStackTrace
2. Error.captureStackTrace
3. Redefining an Error's stack property, apparently (not sure when this happens in library code)

For instance the resolve package has 20 million downloads per week and sets a custom Error.prepareStackTrace:
https://github.com/browserify/resolve/blob/1382486e2c1063393f6e6856f2e68330e67a7bc5/lib/caller.js

We’re not sure yet what we could do to be able to run this code in Firefox. Perhaps polyfilling Error.captureStackTrace, if there are no plans to implement V8's stack trace API; but things like redefining an Error's stack throwing is definitely a blocker.

Comment 8

[Tom S [:evilpie]]

but things like redefining an Error's stack throwing is definitely a blocker

Can you change the callers of Object.defineProperty? The problem here is that you don't pass configurable: true, and because there is no existing stack property, this behaves like configurable: false.

Comment 9

[Florens Verschelde :fvsch]

Can you change the callers of Object.defineProperty?

Nope, we are an execution environment, and run code from the npm ecosystem — which we don’t control.
(Same situation as if we were publishing a fork of Node.js with SpiderMonkey in lieu of V8.)