-
Notifications
You must be signed in to change notification settings - Fork 24.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Wrap NullPointerExceptions when thrown and throw new Error in dispatchViewManagerCommand for readability #38060
Conversation
Base commit: c803a5b |
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
Please hold off on merging this! I accidentally wrapped the wrong spot. I'll send a follow-up |
@cortinico We've tested this in production for about 1.5 weeks now and the errors thrown are much better. This should be good to merge! |
@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
@cortinico merged this pull request in 0519c11. |
Sorry, having to revert this as we have concerns about this leaking user information to logcat. Consider making these extra logs available in development builds only so they're not shipped in release. |
This pull request has been reverted by 52698e7. |
@hsource don't you mind sending the same PR without printing parameters to the log? |
@cortinico Done! Created #38444 |
…Command for easier debugging (#38444) 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 Pull Request resolved: #38444 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: D47546672 Pulled By: cortinico fbshipit-source-id: 68379561d84d0ef2ed5c6d66f3f49943c5d7cb7e
…Command for easier debugging (facebook#38444) Summary: This PR is a modified version of facebook#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 facebook#23126, facebook#19413, facebook#27633, facebook#23378, facebook#29250, facebook#28262, facebook#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: facebook#38444 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: D47546672 Pulled By: cortinico fbshipit-source-id: 68379561d84d0ef2ed5c6d66f3f49943c5d7cb7e
…Command for easier debugging (facebook#38444) Summary: This PR is a modified version of facebook#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 facebook#23126, facebook#19413, facebook#27633, facebook#23378, facebook#29250, facebook#28262, facebook#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: facebook#38444 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: D47546672 Pulled By: cortinico fbshipit-source-id: 68379561d84d0ef2ed5c6d66f3f49943c5d7cb7e
…Command for easier debugging (facebook#38444) Summary: This PR is a modified version of facebook#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 facebook#23126, facebook#19413, facebook#27633, facebook#23378, facebook#29250, facebook#28262, facebook#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: facebook#38444 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: D47546672 Pulled By: cortinico fbshipit-source-id: 68379561d84d0ef2ed5c6d66f3f49943c5d7cb7e
…Command for easier debugging (facebook#38444) Summary: This PR is a modified version of facebook#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 facebook#23126, facebook#19413, facebook#27633, facebook#23378, facebook#29250, facebook#28262, facebook#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: facebook#38444 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: D47546672 Pulled By: cortinico fbshipit-source-id: 68379561d84d0ef2ed5c6d66f3f49943c5d7cb7e
…chViewManagerCommand for easier debugging (facebook#38444) Summary: This PR is a modified version of facebook#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. 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 facebook#23126, facebook#19413, facebook#27633, facebook#23378, facebook#29250, facebook#28262, facebook#34001 and likely many more. 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. [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: facebook#38444 Test Plan: Test change on our app via a beta release. With these changes, we got better stacktraces like these: ``` 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) ``` ``` 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: D47546672 Pulled By: cortinico fbshipit-source-id: 68379561d84d0ef2ed5c6d66f3f49943c5d7cb7e
…chViewManagerCommand for easier debugging (facebook#38444) Summary: This PR is a modified version of facebook#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. 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 facebook#23126, facebook#19413, facebook#27633, facebook#23378, facebook#29250, facebook#28262, facebook#34001 and likely many more. 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. [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: facebook#38444 Test Plan: Test change on our app via a beta release. With these changes, we got better stacktraces like these: ``` 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) ``` ``` 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: D47546672 Pulled By: cortinico fbshipit-source-id: 68379561d84d0ef2ed5c6d66f3f49943c5d7cb7e
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.
I noticed that
invoke
inJavaMethodWrapper
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:
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
JS stacktrace (after dispatchViewManagerCommand change)