fix(react-native): Ensure Java17 is used on android builds

[JoseLion]

Summary

React Native's Android build works perfectly fine on Java 21, except when the react-native-flipper dependency is added. When this happens, the build fails with the error below, which I believe is related to this bug in android-gradle-plugin (apparently fixed in v8.2.1):

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':react-native-flipper:compileDebugJavaWithJavac'.
> Could not resolve all files for configuration ':react-native-flipper:androidJdkImage'.
   > Failed to transform core-for-system-modules.jar to match attributes {artifactType=_internal_android_jdk_image, org.gradle.libraryelements=jar, org.gradle.usage=java-runtime}.
      > Execution failed for JdkImageTransform: C:\DevTools\AndroidSDK\platforms\android-34\core-for-system-modules.jar.
         > Error while executing process C:\Program Files\Java\jdk-21\bin\jlink.exe with arguments {--module-path C:\Users\josel\.gradle\caches\transforms-4\4c11f83178b8b05ef6bb8aa8733cb824-2b06f07d-2086-4edc-82ec-39eda73a7cd3\transformed\output\temp\jmod --add-modules java.base --output C:\Users\josel\.gradle\caches\transforms-4\4c11f83178b8b05ef6bb8aa8733cb824-2b06f07d-2086-4edc-82ec-39eda73a7cd3\transformed\output\jdkImage --disable-plugin system-modules}

This PR ensures that Java 17 is always used on the Android build of react-native-flipper by using Gradle's Java Toolchain. This allows developers using Java 21 to build with Flipper. Even though it solves the problem, it'd be more beneficial to provide full support of Java 21 since it was released ~6 months ago, and it will be the LTS for the next year and a half. I'm not sure what needs to be done to provide that support, but I'll be happy to help if possible 🙂

Changelog

Ensures Java 17 is always used on the Android build of react-native-flipper, allowing developers to use more recent Java version on their React Native environments.

Test Plan

Unfortunately, the ReactNativeFlipperExample app cannot be used to test this change because it needs to be heavily updated first. But it's possible to test the change with a new React Native app:

1. Create a new React Native app (at least at v0.73.6)
2. Ensure you are using Java 21 on your environment
3. Update the Gradle Wrapper to make it Java 21 compatible:
   • cd android/
   • gradle wrapper --distribution-type BIN --gradle-version 8.6
4. Add react-native-flipper dependency and configure Flipper for Android
5. Attempt to build the React Native app: yarn android
6. You'll see the error mentioned before
7. Apply the PR changes
8. Build the React Native app again
9. The build should succeed on Java 21

[tmikov]

Hi, clearly this source is getting transformed by Babel or another tool first. Can you please provide the source that is actually executed by Hermes, after the transformation pipeline?

[rogeriomoura]

@tmikov here is what Babel transformed it to:

        try {
          var data = yield resolvePromise(promise);
          isCurrentlyMounted && setState(function (prev) {
            return Object.assign({}, prev, {
              loading: false,
              data: data
            });
          });
        } catch (error) {
          isCurrentlyMounted && setState(function (prev) {
            return Object.assign({}, prev, {
              loading: false,
              error: error
            });
          });
        }

[tmikov]

@rogeriomoura this lowering doesn't seem to correspond exactly to the original code: fetching is missing, debounceSetState() has been changed. Can you confirm that this is exactly what Hermes is running when the problem occurs?

[rogeriomoura]

@tmikov Yeah, that's the exact code, I'm not sure why the debauncing part of it is not there. Just for reference, here is the implementation of that debaunceSetState:

import {debounce} from 'lodash';
...
const debounceSetState = debounce(setState, 100);

[tmikov]

@rogeriomoura are you saying that your transformation pipeline deleted the property fetching and changed the call debounceSetState() to setState()?

[rogeriomoura]

@tmikov my bad! We have a library that imports a useAsync but it is not our implementation, here is the right one:

        try {
          var result = yield resolvePromise(promise);
          isCurrentlyMounted && debounceSetState(function (prev) {
            return Object.assign({}, prev, {
              fetching: false,
              loading: false,
              result: result
            });
          });
        } catch (error) {
          isCurrentlyMounted && debounceSetState(function (prev) {
            return Object.assign({}, prev, {
              fetching: false,
              loading: false,
              error: error
            });
          });
        }

[tmikov]

@rogeriomoura I couldn't reproduce your problem by running the following code:

var gPrev = {}
var doErr = 0;

function debounceSetState(func) {
    gPrev = func(gPrev);
    if (++doErr == 1)
        throw Error("fake errror");
}

function *bad() {
    try {
      var result = yield 10;
      debounceSetState(function (prev) {
        return Object.assign({}, prev, {
          fetching: false,
          loading: false,
          result: result
        });
      });
    } catch (error) {
      debounceSetState(function (prev) {
        return Object.assign({}, prev, {
          fetching: false,
          loading: false,
          error: error
        });
      });
    }
}

it = bad();
it.next();
it.next("fake result");
print(JSON.stringify(gPrev));

I am sorry, but without a concrete repro, there isn't anything more we can do. In order to constructively investigate a compiler or runtime bug, we need a minimal reproduction case: a small piece of code to feed directly into Hermes, producing the incorrect result.

For the time being I am removing the "bug" tag, but feel free to restore it when you have the repro.

[rpopovici]

I have a similar problem in react-native. Looks like it's an older bug https://github.com/facebook/react-native/issues/18087

The native exception is not properly handled in JS if you try to fetch from a malformed url:

try {
    const res = await fetch("https://malformed/api");
} catch (err) {
    // throw err; // this does not work. err is undefined

    // throw { ...err } // this does not work. err is undefined 

    // unpacking and recreating the error object works :)
    const { name, message, stack, status } = err;
    throw { name, message, stack, status };
}

Strangely enough, if you unpack the error object and re-throw a new error object, then it works I don't know if this a react-native, babel or hermes bug. We use both typescript and babel to transpile for hermes

[tmikov]

@rpopovici can you provide the transpiled version of the above code? Also, does print(err) output undefined?

[rpopovici]

@tmikov this is how the transpiled code looks like:

  var fetchData = function () {
    var _ref4 = _$$_REQUIRE(_dependencyMap[9])(function* (input, init) {
      try {
        var res = yield fetch("https://malformed/api");
      } catch (err) {
        console.log("Error: " + JSON.stringify(err));
        console.log(err.toString());
        console.log(err);
        throw err;
      }
    });
    return function fetchData(_x3, _x4) {
      return _ref4.apply(this, arguments);
    };
  }();
  exports.fetchData = fetchData;

console.log("Error: " + JSON.stringify(err)) returns Error: {} console.log(err.toString()) returns TypeError: Network request failed console.log(err) returns [TypeError: Network request failed] flipper breakpoint in throw block shows undefined

[tmikov]

@rpopovici, thanks!

Hmm, to me this seems to show that err has the correct value (not 'undefined'), but there might be a problem with displaying it in the debugger? The latter wouldn't surprise me, since we have to play some scoping tricks with the catch variable even in ES5. It is conceivable that we have a bug in accessing the value of a catch variable from a debugger.

Can you try assigning it to a different variable and trying to view that from the debugger (e.g. var xxx = err;)?

[rpopovici]

@tmikov Assigning err to another variable works. I can see the error object in flipper:

jsEngine: "hermes"
message: "Network request failed"
stack: "TypeError: Network request failed...

[tmikov]

@rpopovici I was able to reproduce the problem with the hdb tool and this simple source:

function foo(a) {
    try {
        throw a+10;
    } catch (err) {
        print(err);
        var xxx = err;
        debugger;
    }
}

foo(5);

And the following debugging session:

hdb catch.js
15
Break on 'debugger' statement in foo: catch.js[2]:7:9
(hdb) exec a
5
(hdb) exec xxx
15
(hdb) exec err
Exception: ReferenceError: Property 'err' doesn't exist
  0: eval: JavaScript:1:1
  1: foo: catch.js:7:9
  2: global: catch.js:11:4
Thrown value is: { message: Property 'err' doesn't exist }
(hdb) info variables
this = undefined
 0:          a = 5
 0:        xxx = 15
 0: ?anon_0_err = 15

Clearly, the debugger doesn't know about the variable err.

It doesn't seem like this is the same problem as the one reported by @rogeriomoura, but this is a legitimate bug. I am opening a new issue for tracking it.

[woowalker]

in my case, i require none exist file, it still crash

try {
  require('module')
} catch (e) {
  console.err(e)
}

React Native version: 0.73.0 OS: android

[tmikov]

@woowalker this is not enough information for us to work with. What do you mean by "crash"? Do you have a small reproducible sample that can be executed in the Hermes CLI?

[woowalker]

@tmikov thanks for your quickly response. what i mean is try catch seems cannot catch require none exist file error, it cause program crash, i try from new project, React Native verison is 0.73.1; run in android emulator

• start a new project

npx react-native init MyApp --template react-native-template-typescript

• require none exist file, try catch cannot catch the error

• red screen, and even release to an apk, it cannot open just crash

[martinbigio]

@woowalker the example you shared is unrelated to the GH issue. I don't think the require (defined on RN itself) is expected to throw. The "red box error" you're seeing is triggered by the require function call itself. In other words, the call finished gracefully after pushing a view to display the error. require has different semantics that fopen. Maybe that's where the confusion is coming from?

[woowalker]

@martinbigio not much understand, i just want to figure out why try catch cannot caught the require error, i can`t use require to detect whether module is exist, make me crazy.

[martinbigio]

I work on Hermes (not RN) but require is not meant to be called with a JS module that doesn't exist. Please look at alternative ways to achieve what you're trying to do. I also kindly ask to keep this GH issue on topic - the issue you brought up is unrelated to it. Thanks!