Wrap NullPointerExceptions and throw new Error in dispatchViewManagerCommand for easier debugging

[hsource]

Summary:

This PR is a modified version of #38060 which was reverted because it may sometimes cause sensitive info to be logged. This version no longer includes any parameters in the thrown errors.

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 #23126, #19413, #27633, #23378, #29250, #28262, #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

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

[cortinico]

CI is red, which looks suspicious

[cortinico]

/rebase

[cortinico]

Could you rebase? As your revision is too old and can't be rebased automatically

[analysis-bot]

Platform	Engine	Arch	Size (bytes)	Diff
android	hermes	arm64-v8a	8,835,262	-953
android	hermes	armeabi-v7a	8,144,840	-1,273
android	hermes	x86	9,340,084	-1,517
android	hermes	x86_64	9,183,232	-1,103
android	jsc	arm64-v8a	9,447,078	-971
android	jsc	armeabi-v7a	8,628,708	-1,115
android	jsc	x86	9,529,178	-1,566
android	jsc	x86_64	9,772,950	-1,002

Base commit: 777934e
Branch: main

[hsource]

@cortinico Sorry about that! Rebased and now it seems to pass all checks

[facebook-github-bot]

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[github-actions]

This pull request was successfully merged by @hsource in ff1972d.

^{When will my fix make it into a release? | Upcoming Releases}