Skip to content

Throw a JS error when TurboModule constants can't be converted - #59698

Open
tomekzaw wants to merge 2 commits into
react:mainfrom
tomekzaw:fix-get-constants-exception
Open

tomekzaw wants to merge 2 commits into
react:mainfrom
tomekzaw:fix-get-constants-exception

Conversation

@tomekzaw

@tomekzaw tomekzaw commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Summary:

JavaTurboModule converts a module's getConstants() result with Arguments.makeNativeMap(). That throws IllegalArgumentException for values it can't convert, such as a Date or a Uri. The result wasn't checked, so the null map was used with the Java exception still pending, and ART aborted the app with JNI DETECTED ERROR IN APPLICATION: obj == null.

This change:

  • Calls Arguments.makeNativeMap() through an fbjni JavaClass, so a value it can't convert throws through fbjni's usual error handling and becomes a JS error.
  • Looks up Arguments and makeNativeMap once, instead of calling FindClass and leaving its local reference behind on every getConstants() call.
  • Releases the local reference to the constants map.

Changelog:

[ANDROID] [FIXED] - Throw a JS error instead of crashing when a TurboModule's constants contain a value that can't be converted

Test Plan:

Release RNTester (./gradlew :packages:rn-tester:android:app:assembleRelease -PreactNativeArchitectures=arm64-v8a) with these temporary, uncommitted changes:

  • SampleTurboModule adds a java.util.Date constant when asked to.
  • A Playground screen that then calls getConstants().
  • R8 turned off, because SampleTurboModule can't initialize in a minified build, which is a separate issue.

On a Galaxy A16 (Android 16):

Call main This PR
getConstants() with a Date constant SIGABRT: JNI DETECTED ERROR IN APPLICATION: obj == null, from GetObjectField in JavaTurboModule::invokeJavaMethod JS error: Exception in HostFunction: java.lang.IllegalArgumentException: Could not convert class java.util.Date, followed by the Java stack trace
getConstants() with valid constants {"const3":"something","const1":true,"const2":1080} Same value

Not run: an automated test. JavaTurboModule has no host-side tests, and Fantom doesn't load Java modules.

🤖 Generated with Claude Code

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Oct 30, 2026
@facebook-github-tools facebook-github-tools Bot added p: Software Mansion Partner: Software Mansion Partner labels Oct 30, 2026
@tomekzaw
tomekzaw marked this pull request as ready for review October 30, 2026 22:05
@facebook-github-tools facebook-github-tools Bot added the Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. label Oct 30, 2026
Comment on lines +532 to +540
static const auto jArguments =
jni::findClassStatic("com/facebook/react/bridge/Arguments");
static jmethodID jMakeNativeMap = env->GetStaticMethodID(
jArguments,
jArguments.get(),
"makeNativeMap",
"(Ljava/util/Map;)Lcom/facebook/react/bridge/WritableNativeMap;");
auto constants = env->CallStaticObjectMethod(jArguments, jMakeNativeMap, arg);
auto jResult = jni::adopt_local(constants);
auto result = jni::static_ref_cast<NativeMap::jhybridobject>(jResult);
return jsi::valueFromDynamic(rt, result->cthis()->consume());
return jni::adopt_local(
static_cast<NativeMap::jhybridobject>(
env->CallStaticObjectMethod(jArguments.get(), jMakeNativeMap, arg)));

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's migrate this to fbjni, so we get the normal error handling.

@tomekzaw tomekzaw Oct 31, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in abae08e. Arguments.makeNativeMap() is now called through an fbjni JavaClass, so a value it can't convert throws a JniException and becomes a JS error.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Software Mansion Partner: Software Mansion Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants