Facebook/react-native
1
0
Fork 0
mirror of https://github.com/facebook/react-native.git synced 2025-11-01 09:14:26 +00:00
Code Issues Packages Projects Releases Wiki Activity

Wrap NullPointerExceptions when thrown and throw new Error in dispatchViewManagerCommand for readability (#38060)

Browse Source 
Summary:
### Motivation

I had a crash-causing error in our error reporting console (Bugsnag) that was extremely hard to debug due to the lack of specificity in the thrown error.

```
java.lang.NullPointerException: Attempt to invoke virtual method 'double java.lang.Double.doubleValue()' on a null object reference
        at com.facebook.react.bridge.ReadableNativeArray.getDouble(ReadableNativeArray.java:92)
        at com.facebook.react.bridge.JavaMethodWrapper$4.extractArgument(JavaMethodWrapper.java:64)
        at com.facebook.react.bridge.JavaMethodWrapper$4.extractArgument(JavaMethodWrapper.java:60)
        at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:356)
        at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:188)
        at com.facebook.jni.NativeRunnable.run(NativeRunnable.java:-2)
        at android.os.Handler.handleCallback(Handler.java:883)
        at android.os.Handler.dispatchMessage(Handler.java:100)
        at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
        at android.os.Looper.loop(Looper.java:214)
        at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:228)
        at java.lang.Thread.run(Thread.java:919)
```

I noticed that `invoke` in `JavaMethodWrapper` tacks on the JS method name on other errors, so wanted to change this to handle NullPointerExceptions more gracefully too.

This helps make it easier to debug issues like https://github.com/facebook/react-native/issues/23126, https://github.com/facebook/react-native/issues/19413, https://github.com/facebook/react-native/issues/27633, https://github.com/facebook/react-native/issues/23378, https://github.com/facebook/react-native/issues/29250, https://github.com/facebook/react-native/issues/28262, https://github.com/facebook/react-native/issues/34001 and likely many more.

### After adding NullPointerException

Even after adding the NullPointerException, I got errors like this:

```
com.facebook.react.bridge.NativeArgumentsParseException: Attempt to invoke virtual method 'double java.lang.Double.doubleValue()' on a null object reference (constructing arguments for UIManager.dispatchViewManagerCommand at argument index 0) with parameters [null, 4.0, []]
        at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:371)
        at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:188)
        at com.facebook.jni.NativeRunnable.run(NativeRunnable.java:-2)
        at android.os.Handler.handleCallback(Handler.java:942)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
        at android.os.Looper.loopOnce(Looper.java:226)
        at android.os.Looper.loop(Looper.java:313)
        at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:228)
        at java.lang.Thread.run(Thread.java:1012)
```

This helped, but still didn't help me easily find which library calling `dispatchViewManagerCommand` was causing this.

## Changelog:

[ANDROID] [CHANGED] - Wrap NullPointerExceptions when thrown by native code called from JS for readability
[GENERAL] [CHANGED] - Throw Error in dispatchViewManagerCommand when non-numeric tag is passed for easier debugging

Pull Request resolved: https://github.com/facebook/react-native/pull/38060

Test Plan:
Test change on our app via a beta release.

With these changes, we got better stacktraces like these:

### Java stacktrace

```
com.facebook.react.bridge.NativeArgumentsParseException: Attempt to invoke virtual method 'double java.lang.Double.doubleValue()' on a null object reference (constructing arguments for UIManager.dispatchViewManagerCommand at argument index 0) with parameters [null, 4.0, []]
        at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:371)
        at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:188)
        at com.facebook.jni.NativeRunnable.run(NativeRunnable.java:-2)
        at android.os.Handler.handleCallback(Handler.java:942)
        at android.os.Handler.dispatchMessage(Handler.java:99)
        at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:27)
        at android.os.Looper.loopOnce(Looper.java:226)
        at android.os.Looper.loop(Looper.java:313)
        at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:228)
        at java.lang.Thread.run(Thread.java:1012)
```

### JS stacktrace (after dispatchViewManagerCommand change)

```
Error dispatchViewManagerCommand: found null reactTag with args []
    node_modules/react-native/Libraries/ReactNative/PaperUIManager.js:120:21 dispatchViewManagerCommand
    node_modules/react-native-maps/lib/MapMarker.js:68:67 _runCommand
    node_modules/react-native-maps/lib/MapMarker.js:60:24 redraw
    templates/Components/MapViewWithMarkers/PlaceMarker.tsx:88:32 anonymous
    (native) apply
    node_modules/react-native/Libraries/Core/Timers/JSTimers.js:213:22 anonymous
    node_modules/react-native/Libraries/Core/Timers/JSTimers.js:111:14 _callTimer
    node_modules/react-native/Libraries/Core/Timers/JSTimers.js:359:16 callTimers
    (native) apply
    node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:427:31 __callFunction
    node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:113:25 anonymous
    node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:368:10 __guard
    node_modules/react-native/Libraries/BatchedBridge/MessageQueue.js:112:16 callFunctionReturnFlushedQueue
```

Reviewed By: javache

Differential Revision: D47015785

Pulled By: cortinico

fbshipit-source-id: 33e0b8fbc7dc6a88d4c3383630126f99d2cf6098
This commit is contained in:
Harry Yu
2023-07-10 06:12:05 -07:00
committed by Facebook GitHub Bot
parent 177d97d8ea
commit 0519c11acd
2 changed files with 30 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/react-native/Libraries/ReactNative/UIManager.js
Vendored
+16
View File
@@ -180,6 +180,22 @@ const UIManager = {
commandName: number | string,
commandArgs: any[],
) {
// Sometimes, libraries directly pass in the output of `findNodeHandle` to
// this function without checking if it's null. This guards against that
// case. We throw early here in Javascript so we can get a JS stacktrace
// instead of a harder-to-debug native Java or Objective-C stacktrace.
if (typeof reactTag !== 'number') {
let stringifiedArgs = '(failed to stringify commandArgs)';
try {
stringifiedArgs = JSON.stringify(commandArgs);
} catch (err) {
// Do nothing. We have a default message
}
throw new Error(
`dispatchViewManagerCommand: found null reactTag with args ${stringifiedArgs}`,
);
}
if (isFabricReactTag(reactTag)) {
const FabricUIManager = nullthrows(getFabricUIManager());
const shadowNode =
packages/react-native/ReactAndroid/src/main/java/com/facebook/react/bridge/JavaMethodWrapper.java
+14 -7
View File
@@ -356,7 +356,7 @@ public class JavaMethodWrapper implements NativeModule.NativeMethod {
mArgumentExtractors[i].extractArgument(jsInstance, parameters, jsArgumentsConsumed);
jsArgumentsConsumed += mArgumentExtractors[i].getJSArgumentsNeeded();
}
} catch (UnexpectedNativeTypeException e) {
} catch (UnexpectedNativeTypeException | NullPointerException e) {
throw new NativeArgumentsParseException(
e.getMessage()
+ " (constructing arguments for "
@@ -364,29 +364,36 @@ public class JavaMethodWrapper implements NativeModule.NativeMethod {
+ " at argument index "
+ getAffectedRange(
jsArgumentsConsumed, mArgumentExtractors[i].getJSArgumentsNeeded())
+ ")",
+ ") with parameters "
+ parameters.toArrayList(),
e);
}
try {
mMethod.invoke(mModuleWrapper.getModule(), mArguments);
} catch (IllegalArgumentException ie) {
throw new RuntimeException("Could not invoke " + traceName, ie);
} catch (IllegalAccessException iae) {
throw new RuntimeException("Could not invoke " + traceName, iae);
} catch (IllegalArgumentException | IllegalAccessException e) {
throw new RuntimeException(createInvokeExceptionMessage(traceName, parameters), e);
} catch (InvocationTargetException ite) {
// Exceptions thrown from native module calls end up wrapped in InvocationTargetException
// which just make traces harder to read and bump out useful information
if (ite.getCause() instanceof RuntimeException) {
throw (RuntimeException) ite.getCause();
}
throw new RuntimeException("Could not invoke " + traceName, ite);
throw new RuntimeException(createInvokeExceptionMessage(traceName, parameters), ite);
}
} finally {
SystraceMessage.endSection(TRACE_TAG_REACT_JAVA_BRIDGE).flush();
}
}
/**
* Makes it easier to determine the cause of an error invoking a native method from Javascript
* code by adding the function and parameters.
*/
private static String createInvokeExceptionMessage(String traceName, ReadableArray parameters) {
return "Could not invoke " + traceName + " with parameters " + parameters.toArrayList();
}
/**
* Determines how the method is exported in JavaScript: METHOD_TYPE_ASYNC for regular methods
* METHOD_TYPE_PROMISE for methods that return a promise object to the caller. METHOD_TYPE_SYNC