Skip to content

Conversation

@pull
Copy link

@pull pull bot commented Mar 15, 2021

See Commits and Changes for more details.


Created by pull[bot]

Can you help keep this open source service alive? 💖 Please sponsor : )

luluwu2032 and others added 2 commits March 15, 2021 03:09
…agerBinding.cpp

Summary:
Right now there are places in UIManagerBinding.cpp where native java exceptions will be thrown if calling JSI functions results in errors, such as:

```Trying to report error getPropertyAsObject: property '__fbBatchedBridge' is undefined, expected an Object
Error: getPropertyAsObject: property '__fbBatchedBridge' is undefined, expected an Object
```

https://www.internalfb.com/intern/logview/details/facebook_android_javascripterrors/358181062b47b9561e60427bbb3816a9

In this diff I added LOG(ERROR) and checks because:
1, Throwing errors neither prevents the JSI errors nor handles them properly, checks prevent JSI errors from happening.
2, Errors are aggregated in Logview with other JSI errors as "Error: android_crash:com.facebook.react.devsupport.JSException:facebook::jni::throwNewJavaException" which keeps the SLA task open forever, checks can prevent JSI errors so they won't lead to exceptions, and  LOG(ERROR) will make sure we have enough info for debugging.

Changelog:
[General][Changed] -  Add checks and logs to for better error handling

Reviewed By: JoshuaGross

Differential Revision: D26885562

fbshipit-source-id: c0c1c057342e9efc0ff708188703f4332036e7a9
Summary:
Changelog: [internal]

Creates new function `RCTInstallNativeComponentRegistryBinding` which can be called during JS setup to register JS function `__nativeComponentRegistry__hasComponent`.

Note: This diff doesn't call `RCTInstallNativeComponentRegistryBinding`.

Reviewed By: shergin

Differential Revision: D26946176

fbshipit-source-id: 0625b8dd6090bc9e08baa38ba60b9cbe48268184
@pull pull bot added the ⤵️ pull label Mar 15, 2021
@pull pull bot merged commit 23cf995 into MLH-Fellowship:master Mar 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants