Hermes: Hermes: Cannot set property 'preventInvalidHostname' of undefined

[aarondail]

In our RN 0.62.2 app, we are seeing crashes when we enable Hermes related to our use of URI.js https://medialize.github.io/URI.js/ ... in one case it seems like the UriJs object gets lost or something and becomes undefined, and we get exceptions like "Cannot set property 'preventInvalidHostname' of undefined" when we are trying to run code like UriJs.preventInvalidHostname = ...;

In more detail, we have a utility function called isUrlValid defined like this:

import UriJs from 'urijs';
// ...
export function isUrlValid(url: string): boolean {
  const preventInvalidHostname = UriJs.preventInvalidHostname;
  try {
    UriJs.preventInvalidHostname = true;
    UriJs.parse(url);
    return true;
  } catch (e) {
    return false;
  } finally {
    UriJs.preventInvalidHostname = preventInvalidHostname;
  }
}

This works totally fine w/ Hermes off, and with Hermes on it is usually fine. But, only in release builds and only in one case when isUrlValid is called, it will throw "Cannot set property 'preventInvalidHostname' of undefined".

It is totally weird.

I'd like to provide a repro... and I did try, but I failed. Practically speaking the only case where I can definitely say it happens is hard to separate from the rest of the code... and just calling the function by itself _wont fail_. So I am not sure how much of the rest of the code or even the state of the app is relevant to cause the bug to repro, and I can't invest a huge amount of time into it.

I know this without a repro there probably isn't too much that can be done, but I thought maybe you would have some thoughts or suggestions I could try. Or maybe this is related to another known issue (I did find some other issues that seemed somewhat similar, but not exactly the same as mine.)

• [x] I have run gradle clean and confirmed this bug does not occur with JSC"

Hermes version: 0.4.1
React Native version (if any): 0.62.2
Android version (if any): multiple, 7-10 at least
Platform: not 100% sure, but in Bugsnag crashes we see cpuAbi with values ["arm64-v8a", "armeabi-v7a", "armeabi"]

Thanks!

[neildhar]

Hi @aarondail, thanks for reporting this! Unfortunately it's difficult to say what the issue might be without a repro. If your app is public, we can take a look at the JS bundle and figure out what might be triggering it. You can fetch it from Metro by running curl 'http://localhost:8081/index.bundle?platform=android&dev=false&minify=false' > bundle.js.

Otherwise, if possible, it would be helpful if you could share just the portion that is triggering this issue. We'd also be interested in which codepath you're seeing the problem on (for instance, does this only happen when an exception is caught)

[aarondail]

HI @neildhar thanks for the quick response. Yeah our app isn't public so I probably can't share the bundle.

The code path itself is a bit complicated but boils down to something like this:
We are showing a web view using react-native-webview. We get a "message" from the JS running in the web view (via the onMessage prop). We parse a URL out of the message, and call the isUrlValid function that I linked above.

And yes your intuition is right, this error only happens in the exception case.

I also played around with the function a little bit and if I change isUrlValid to:

export function isUrlValid(url: string): boolean {
  // const preventInvalidHostname = UriJs.preventInvalidHostname;
  try {
    UriJs.preventInvalidHostname = true;
    UriJs.parse(url);
    return true;
  } catch (e) {
    return false;
  // } finally {
  //   UriJs.preventInvalidHostname = preventInvalidHostname;
  }
}

The only changes there are reading UriJs.preventInvalidHostname before entering the try, and removing the finally.

I sorta take that (+ that this error doesn't occur in the valid url case) to indicate there is some kind of problem with catching an error, then doing something in a finally... if that something is setting UriJs.preventInvalidHostname = something?

[tmikov]

@aarondail can you try this version of the function:

export function isUrlValid(url: string): boolean {
  const tmp = UriJs;
  const preventInvalidHostname = tmp.preventInvalidHostname;
  try {
    tmp.preventInvalidHostname = true;
    tmp.parse(url);
    return true;
  } catch (e) {
    return false;
  } finally {
    tmp.preventInvalidHostname = preventInvalidHostname;
  }
}

[aarondail]

@tmikov wow, yeah that fixed it!

[tmikov]

@aarondail thanks for trying it. Keep in mind, there is still an underlying problem that we must figure out - this is giving us a hint where it might be, but is not a real fix because we don't really understand why it is working.

[tmikov]

@aarondail I wonder whether it would be feasible for us (the Hermes team) to get an .apk of your app (if presumably you are going to publish it to the Play Store anyway). In this way we can try debugging the bytecode or replacing the Hermes.so with a debug version without actually getting access to your source. This could really help us diagnose this problem.

[aarondail]

@tmikov I think that should be ok, so I'll check and follow up. (FWIW we are released on the Play Store now, but with Hermes off)

[aarondail]

@tmikov
OK I have an .apk that makes it easy to repro the problem. (FWIW I tried again to create a sample RN app that would also have the problem but I didn't have any luck. Even using the same version of RN and hermes. I am not sure what to make of that.)

Hope this helps:
app-hermes-on-crash-on-isUrlValid.apk.zip

After you start the app there is a button that says CLICK TO TEST ISURLVALID. Click that to cause the crash.

[tmikov]

@aarondail thank you so much! Sorry for the delayed reply - I was on PTO - we are focusing on this.

[aarondail]

One other thing I noticed while trying (and failing) to create a sample RN app with the problem... the Android build for the sample app w/ Hermes on is significantly faster than what we experience with our app. Like, it takes 10+ minutes more to build for us w/ Hermes on vs. w/ Hermes off.

I don't know if that is relevant, probably not, but JIC it is I thought I'd mention it.

[tmikov]

@aarondail we figured it out! It turns out that this is a known bug in Hermes (fix has been is WIP for a long time). We just didn't realize it might affect real users. Thank you so much for helping us debug this!