Many libraries can cause "Error.stack getter called with an invalid receiver"

[thomasttvo]

Bug Description

When an error is thrown while rendering, RN crashes of course as expected. However, for some errors, when ExceptionsManager calls error.stack, it throws new errors, masking the original errors, which makes it impossible to know what the original error was for users in production (using tools like sentry).

This happens when libraries using a technique like this to create their error classes:

    const MyError = function(message) {
      this.message = message;
    };
    MyError.prototype = new Error;
    MyError.prototype.name = 'MyError';

I can find at least 5 popular libraries that use this technique, and if it works in classic JS, I think it's supposed to work with hermes?

• [x] The issue is reproducible with the latest version of React Native.

React Native version: 0.73.5 OS: iOS, Android

Steps To Reproduce

 useEffect(() => {
    const MyError = function(message) {
      this.message = message;
    };
    MyError.prototype = new Error;
    MyError.prototype.name = 'MyError';

    throw new MyError('1234')
  }, []);

The Expected Behavior

Calling new MyError().stack doesn't throw Error.stack getter called with an invalid receiver

[tmikov]

@thomasttvo hi, Error.stack is not part of the ECMAScript spec, so any behavior is valid. However, the example you have shared doesn't seem to work in any engine. I just tried the following:

const MyError = function(message) {
      this.message = message;
    };
    MyError.prototype = new Error;
    MyError.prototype.name = 'MyError';


function foo() {
    throw new MyError('1234')
}
function bar() {
    foo();
}


try {
    bar();
} catch (e) {
    print(e.stack);
}

If you run it with v8 or jsc, you get a useless stack trace:

v8 test.js
MyError
    at test.js:4:25

So, why is this a useful?

[thomasttvo]

@tmikov Even though the error doesn't report the stack trace that points to where it is actually thrown, it's useful enough to find out which library it comes from. We have no control over what libraries do with their errors, some throw plain objects, some throw strings. In any of those cases, some info is good enough (When you run with v8, the stack actually points to the library where the error was initially defined), whereas the stack trace and data of Error.stack getter called with an invalid receiver provides no useful info. We don't even have the message of the original error.

[tmikov]

@thomasttvo Do you have an explanation of what the point is of putting an Error object in the prototype? What does it accomplish? More importantly, is there a formal specification anywhere of how this should work?

[thomasttvo]

@tmikov I believe these libraries originated from a time pre-ES6 when class inheritance wasn't a feature. That was how ~you~ a bunch of libraries do class inheritance back then.

Here's a non-error pre-es6 inheritance snippet from Bluebird

Here's a custom error in handlebars.js

base-64

What's worse, sometimes we may not even know we're using these libraries because they're dependencies of dependencies of dependencies

[tmikov]

@thomasttvo this is most definitely not how inheritance was implemented pre-ES6. At the very least it should call the parent constructor.

[tmikov]

We understand this is a real problem users of Hermes are having and we want to help. But we need to understand what the solution is.

[thomasttvo]

@tmikov you're right, that's not how you're supposed to do inheritance, but many extremely popular libraries use that method, and it didn't throw any error. I think the fix is to display the stack trace of the original error in the prototype and avoid throwing further errors when you perform operations on Error. If that's impossible, then just return the string "<Error.stack getter called with an invalid receiver>" for error.stack would be great. The goal I'm trying to get to is to not mask the original error with another error.

[tmikov]

@thomasttvo yes, I think I am beginning to get the picture. One way or another we will make sure the stack getter doesn't throw.

[thomasttvo]

thank you! with that said, if the stack getter returns at test.js:4:25 like above, it would still be really really helpful 🙏 although I know this might be hard to achieve.

[tmikov]

This is the fix, landing in Static Hermes. The getter returns the stack from the prototype. https://github.com/facebook/hermes/pull/1497

[wobsoriano]

@tmikov looks like the fix was cancelled?

[tmikov]

@tmikov looks like the fix was cancelled?

It wasn't cancelled. As can be seen clearly above, it was committed in https://github.com/facebook/hermes/pull/1497

[wobsoriano]

I see, so it was just labeled "Closed" but it's merged?

[tmikov]

@wobsoriano yeah, I am not sure why it doesn't say "Merged", when it clearly was:

[wobsoriano]

Thanks @tmikov! and this is not yet available in the latest RN release right?

[tmikov]

@wobsoriano it will be in RN 0.76

[benjamin-sweney]

I am on 0.76.6 and experiencing numerous of these errors in my production application. @tmikov can you please confirm this did indeed make it into 0.76? If yes then I will open another ticket to understand this issue further.

[tmikov]

@benjamin-sweney if you click on the commit, you can see the release tags it is in: https://github.com/facebook/hermes/commit/db6d12e202e15f7a446d8848d6ca8f7abb3cfb32

It is definitely in 0.76.

[exzos28]

We also see a significant increase in these errors using React Native 0.76.7

[tmikov]

@exzos28 if you can provide a standalone reproduction, we will look into it.

Also, please keep in mind that this pattern is objectively bad code, it isn't spec-compliant, and it should always be avoided in new code. But we do understand that old libraries relying on it exist.

[exzos28]

@tmikov

from mobx:

export function FlowCancellationError() {
    this.message = “FLOW_CANCELLED”
}
FlowCancellationError.prototype = Object.create(Error.prototype)

then:

throw new FlowCancellationError()

and try to read:

try {
...
} catch (error) {
   error.stack // fatal
}

[tmikov]

@exzos28 thank you, I see the problem. We will fix it.

Note that it doesn't "fatal" - it correctly throws an exception that the getter is applied to an incorrect target, namely an Object, when it should be applied to an instance of Error.

[exzos28]

@tmikov "fatal" - I meant it would crash the application.

[tmikov]

@tmikov "fatal" - I meant it would crash the application.

Sorry, I am not sure why a JS exception crashes the application. But it doesn't really matter.

[exzos28]

@tmikov It's not the JS exception that crashes the application, it's the attempt to read the stack property.

My case is that I have an unhandled FlowCancellationError exception, it goes to the global scrope where Sentry gets it.

Next from sentry code:

...
typeof potentialError.stack === 'string'
...

and the application crashes with an error Error.stack getter called with an invalid receiver

[tmikov]

The fix is in: https://github.com/facebook/hermes/pull/1621

[tmikov]

thank you! with that said, if the stack getter returns at test.js:4:25 like above, it would still be really really helpful 🙏 although I know this might be hard to achieve.

@thomasttvo turns out you were right, sorry! 😢