From 129aeacf484fc84e90a817528a76b3ea61c1ad33 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Tue, 16 Apr 2019 09:14:12 -0700 Subject: [PATCH] Add exception handling to method invocations Summary: If you call into a Java method (from C++ using JNI) that raises an exception, the JNI call won't actually raise a C++ error. Instead, the `JNIEnv` will record the pending Java exception and the C++ will continue executing as normal. This is bad because the next time you call into JNI, the app will actually crash, unless you explicitly cleared the exception using `JNIEnv::ExceptionClear()` before the JNI call. With respect to TurboModules, we need to make sure that RedBoxes show up whenever a native methods raise an exception. We also don't want the app to crash when a JNI method call fails because of a raised exception. Therefore, in this diff, I raise a C++ exception if `JNIEnv::ExceptionCheck()` is true. Reviewed By: mdvacca Differential Revision: D14738540 fbshipit-source-id: 4c3063aa93ae7aef025bd2dab6b45059bb8fb409 --- .../core/platform/android/JavaTurboModule.cpp | 41 +++++++++++++++++-- 1 file changed, 37 insertions(+), 4 deletions(-) diff --git a/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.cpp b/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.cpp index fbedfeff623..fa54312c483 100644 --- a/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.cpp +++ b/ReactCommon/turbomodule/core/platform/android/JavaTurboModule.cpp @@ -143,6 +143,23 @@ jsi::Value JavaTurboModule::get(jsi::Runtime& runtime, const jsi::PropNameID& pr } } +static void throwIfJNIReportsPendingException() { + JNIEnv *env = jni::Environment::current(); + if (env->ExceptionCheck()) { + jthrowable ex = env->ExceptionOccurred(); + + // There should be no pending exceptions before we call into JNI + env->ExceptionClear(); + + auto exception = jni::adopt_local(ex); + auto getMessage = + exception->getClass()->getMethod("getMessage"); + auto message = getMessage(exception)->toStdString(); + + throw std::runtime_error(message); + } +} + jsi::Value JavaTurboModule::invokeJavaMethod( jsi::Runtime &runtime, TurboModuleMethodValueKind valueKind, @@ -166,19 +183,29 @@ jsi::Value JavaTurboModule::invokeJavaMethod( switch (valueKind) { case VoidKind: { env->CallVoidMethodA(instance, methodID, jargs.data()); + throwIfJNIReportsPendingException(); + return jsi::Value::undefined(); } case BooleanKind: { - return jsi::Value( - (bool)env->CallBooleanMethodA(instance, methodID, jargs.data())); + bool returnBoolean = + (bool)env->CallBooleanMethodA(instance, methodID, jargs.data()); + throwIfJNIReportsPendingException(); + + return jsi::Value(returnBoolean); } case NumberKind: { - return jsi::Value( - (double)env->CallDoubleMethodA(instance, methodID, jargs.data())); + double returnDouble = + (double)env->CallDoubleMethodA(instance, methodID, jargs.data()); + throwIfJNIReportsPendingException(); + + return jsi::Value(returnDouble); } case StringKind: { auto returnString = (jstring)env->CallObjectMethodA(instance, methodID, jargs.data()); + throwIfJNIReportsPendingException(); + if (returnString == nullptr) { return jsi::Value::null(); } @@ -190,6 +217,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( case ObjectKind: { auto returnObject = (jobject)env->CallObjectMethodA(instance, methodID, jargs.data()); + throwIfJNIReportsPendingException(); + if (returnObject == nullptr) { return jsi::Value::null(); } @@ -200,6 +229,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( case ArrayKind: { auto returnObject = (jobject)env->CallObjectMethodA(instance, methodID, jargs.data()); + throwIfJNIReportsPendingException(); + if (returnObject == nullptr) { return jsi::Value::null(); } @@ -254,6 +285,8 @@ jsi::Value JavaTurboModule::invokeJavaMethod( jsi::Value promise = Promise.callAsConstructor(runtime, promiseConstructorArg); + throwIfJNIReportsPendingException(); + return promise; } default: