From 77561142505a1d4397f68ca662ca2b032dc49de9 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Tue, 3 Sep 2019 13:57:01 -0700 Subject: [PATCH] Print stacktraces of exceptions thrown when creating NativeModules Summary: ## Summary When an exception is raised by Java in a synchronous call from JS to Java, the Java portion of the stack trace is simply ignored when the exception is forwarded to JS. This problem doesn't exist for async calls. I did some digging to figure out why this works with async calls, but not sync calls, to get to the bottom of T52585087. In T52585087, `TurboModuleRegistry.get('I18nAssets')` fails because of a `NullPointerException` in Java. However, since there's no stack trace associated with the `NullPointerException`, it's hard to diagnose the problem. ## Asynchronous calls ReactNative's NativeModules thread is a background thread on which we schedule asynchronous NativeModule method invocations. We spawn our background threads in Java using the [`MessageQueueThreadImpl`](https://fburl.com/diffusion/u0vdm5k3). Each thread is associated with a queue on which you can schedule some work. These `MessageQueueThread`s have a [C++ API](https://fburl.com/diffusion/596740d8) that we can use to schedule some work from C++. NativeModule method invocations in JS produce a C++/Java boundry, because our JS executes C++, which executes the Java NativeModule method. But since the method is asynchronous, instead of invoking it immediately, we wrap it in a C++ lambda and use the C++ API of `MessageQueueThread` to schedule it to be invoked later. Therefore, when it actually invokes, we'll get Java invoking C++, which invokes Java. When the NativeModule method (implemented in Java) throws an exception, fbjni will convert that exception into a `jni::JniException` and start unwinding the C++ stack. Eventually, this exception will reach the outermost Java/C++ boundry. Typically, at this point, the program would crash, but because we used fbjni to register all our native functions, our `jni::JniException` will automatically be converted into a regular Java exception. This exception will be caught by MessageQueueThreadHandler [here](https://fburl.com/diffusion/c4thoca7), and handled using our ExceptionHandler NativeModule. ## Synchronous calls In synchronous execution, JS uses a `JSI::HostObject` to call the Java method directly. If the Java method throws an error, because we're using fbjni, that Java exception will be converted to a `jni::JniException` object, which will contain the stack tract of the Java object. However, from what I could gather, Hermes doesn't know about `jni::JniException`. So, simply ignore the stack trace associated with the Java exception, understanding only the exception message. Hence, all synchronous calls into Java only display the JS stack trace. What we really want is to build an platform-agnostic abstraction that understands `jni::JniException` (and whatever its analogue is in iOS, if any) and use that to bridge between Native and JS. ## Temporary Solution We know that when NativeModules are created, we do a synchronous call from JS to C++ to Java. This synchronous call happens when you do a property access on the `NativeModules` object. Therefore, at the very least, to get to the bottom of T52585087, we could log all exceptions that are thrown whenever a NativeModule is created. This should help us get to the bottom of T52585087. Reviewed By: mdvacca Differential Revision: D17126667 fbshipit-source-id: a6fb27aaea094b9559939ddcc260d3a2c6e71492 --- .../java/com/facebook/react/bridge/ModuleHolder.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java b/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java index f89cc658016..b552a105a2d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java +++ b/ReactAndroid/src/main/java/com/facebook/react/bridge/ModuleHolder.java @@ -12,6 +12,7 @@ import static com.facebook.systrace.Systrace.TRACE_TAG_REACT_JAVA_BRIDGE; import androidx.annotation.GuardedBy; import androidx.annotation.Nullable; +import com.facebook.common.logging.FLog; import com.facebook.debug.holder.PrinterHolder; import com.facebook.debug.tags.ReactDebugOverlayTags; import com.facebook.infer.annotation.Assertions; @@ -197,6 +198,17 @@ public class ModuleHolder { if (shouldInitializeNow) { doInitialize(module); } + } catch (Throwable ex) { + /** + * When NativeModules are created from JavaScript, any exception that occurs in the creation + * process will have its stack trace swallowed before we display a RedBox to the user. Really, + * we should have our HostObjects on Android understand JniExceptions and log the stack trace + * to logcat. For now, logging to Logcat directly when creation fails is sufficient. + * + * @todo(T53311351) + */ + FLog.e("NativeModuleInitError", "Failed to create NativeModule \"" + getName() + "\"", ex); + throw ex; } finally { ReactMarker.logMarker(CREATE_MODULE_END, mName, mInstanceKey); SystraceMessage.endSection(TRACE_TAG_REACT_JAVA_BRIDGE).flush();