Skip to content

Commit

Permalink
Avoid crash by handling missing views in dispatchViewManagerCommand (#…
Browse files Browse the repository at this point in the history
…33795)

Summary:
Based on Google Play crash logs, crashes due to the assertions in `dispatchViewManagerCommand` is our top source of crashes:

```
com.facebook.react.uimanager.IllegalViewOperationException: Unable to execute operation dispatchViewManagerCommand: blur on view with tag: 17549, since the view does not exists
        at com.facebook.react.uimanager.UIImplementation.assertViewExists(UIImplementation.java:830)
        at com.facebook.react.uimanager.UIImplementation.dispatchViewManagerCommand(UIImplementation.java:713)
        at com.facebook.react.uimanager.UIManagerModule.dispatchCommand(UIManagerModule.java:739)
        at com.facebook.react.uimanager.UIManagerModule.dispatchViewManagerCommand(UIManagerModule.java:726)
        at com.facebook.react.uimanager.ReanimatedUIManager.dispatchViewManagerCommand(ReanimatedUIManager.java:233)
        at java.lang.reflect.Method.invoke(Method.java:-2)
        at com.facebook.react.bridge.JavaMethodWrapper.invoke(JavaMethodWrapper.java:372)
        at com.facebook.react.bridge.JavaModuleWrapper.invoke(JavaModuleWrapper.java:151)
        at com.facebook.react.bridge.queue.NativeRunnable.run(NativeRunnable.java:-2)
```

There are many other places in UIImplementation that instead of throwing a fatal exception, just log a message and continues execution.

Logging a message and discarding the command should be safe for commands like "blur this view". We keep the previous behavior in debug mode, but just log and discard commands in production.

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Android] [Fixed] - Avoid crash by handling missing views in dispatchViewManagerCommand

Pull Request resolved: #33795

Test Plan: We had a tough time reproducing specific crashes, but we ran with this binary in the latest release of our app and it has eliminated this class of crashes from Google Play and Bugsnag instrumentation.

Reviewed By: cortinico

Differential Revision: D36285603

Pulled By: JoshuaGross

fbshipit-source-id: db0be74926c4cb83f07686398579236da8e586bf
  • Loading branch information
hsource authored and facebook-github-bot committed May 10, 2022
1 parent c663c0e commit ee1a191
Showing 1 changed file with 42 additions and 12 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.facebook.react.bridge.UiThreadUtil;
import com.facebook.react.bridge.WritableArray;
import com.facebook.react.common.ReactConstants;
import com.facebook.react.common.build.ReactBuildConfig;
import com.facebook.react.modules.i18nmanager.I18nUtil;
import com.facebook.react.uimanager.debug.NotThreadSafeViewHierarchyUpdateDebugListener;
import com.facebook.react.uimanager.events.EventDispatcher;
Expand Down Expand Up @@ -742,13 +743,23 @@ public void clearJSResponder() {
@Deprecated
public void dispatchViewManagerCommand(
int reactTag, int commandId, @Nullable ReadableArray commandArgs) {
assertViewExists(reactTag, "dispatchViewManagerCommand: " + commandId);
boolean viewExists =
checkOrAssertViewExists(reactTag, "dispatchViewManagerCommand: " + commandId);
if (!viewExists) {
return;
}

mOperationsQueue.enqueueDispatchCommand(reactTag, commandId, commandArgs);
}

public void dispatchViewManagerCommand(
int reactTag, String commandId, @Nullable ReadableArray commandArgs) {
assertViewExists(reactTag, "dispatchViewManagerCommand: " + commandId);
boolean viewExists =
checkOrAssertViewExists(reactTag, "dispatchViewManagerCommand: " + commandId);
if (!viewExists) {
return;
}

mOperationsQueue.enqueueDispatchCommand(reactTag, commandId, commandArgs);
}

Expand All @@ -763,7 +774,11 @@ public void dispatchViewManagerCommand(
* no arguments if the menu is dismissed
*/
public void showPopupMenu(int reactTag, ReadableArray items, Callback error, Callback success) {
assertViewExists(reactTag, "showPopupMenu");
boolean viewExists = checkOrAssertViewExists(reactTag, "showPopupMenu");
if (!viewExists) {
return;
}

mOperationsQueue.enqueueShowPopupMenu(reactTag, items, error, success);
}

Expand Down Expand Up @@ -867,15 +882,30 @@ private void measureLayoutRelativeToVerifiedAncestor(
outputBuffer[3] = node.getScreenHeight();
}

private void assertViewExists(int reactTag, String operationNameForExceptionMessage) {
if (mShadowNodeRegistry.getNode(reactTag) == null) {
throw new IllegalViewOperationException(
"Unable to execute operation "
+ operationNameForExceptionMessage
+ " on view with "
+ "tag: "
+ reactTag
+ ", since the view does not exists");
/**
* Returns whether a view identified by the tag exists. In debug mode, this will throw whenever
* the view doesn't exist. In production, it'll log a warning. Callers should use this and just
* return if the view doesn't exist to avoid crashing.
*/
private boolean checkOrAssertViewExists(int reactTag, String operationNameForExceptionMessage) {
boolean viewExists = mShadowNodeRegistry.getNode(reactTag) != null;
if (viewExists) {
return true;
}

String message =
"Unable to execute operation "
+ operationNameForExceptionMessage
+ " on view with "
+ "tag: "
+ reactTag
+ ", since the view does not exist";

if (ReactBuildConfig.DEBUG) {
throw new IllegalViewOperationException(message);
} else {
FLog.w(ReactConstants.TAG, message);
return false;
}
}

Expand Down

0 comments on commit ee1a191

Please sign in to comment.