From 5323221d1442d1573bc65daff618478cb6f056f0 Mon Sep 17 00:00:00 2001 From: Peter Abbondanzo Date: Tue, 19 Sep 2023 06:45:19 -0700 Subject: [PATCH] Log a soft exception from accessibility event exceptions (#37002) Summary: This PR attempts to solve [this issue](https://github.com/facebook/react-native/issues/33021) I created last year, where accessibility events can fire after a view has been unmounted, causing the entire application to crash. This newly added try/catch block is modeled and copies verbiage exactly from the Fabric implementation of accessibility events [here](https://github.com/facebook/react-native/blob/2d9c81780c2bb3c2c2dc8875147261c9a1843eb4/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/SendAccessibilityEventMountItem.java#L30-L43) and [here](https://github.com/facebook/react-native/blob/2d9c81780c2bb3c2c2dc8875147261c9a1843eb4/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/SendAccessibilityEventMountItem.java#L30-L43). ## Changelog: [ANDROID] [FIXED] - Fixed an issue where calling `Accessibility.setAccessibilityFocus` on an unmounted component would crash Pull Request resolved: https://github.com/facebook/react-native/pull/37002 Test Plan: 1. I reimplemented the crash demo from the Snack showcased in [this issue](https://github.com/facebook/react-native/issues/33021). I created the branch [here](https://github.com/Abbondanzo/react-native/pull/1/files), although I may archive or delete this repo in the future so the link is not permanent 1. I disabled the new Fabric architecture. This crash only occurs in the old architecture 1. I pressed the button that would set accessibility focus on an unmounted component, and it logs a soft exception as seen in the screenshot below. I've also attached a screenshot from an identical soft exception that already gets logged by Fabric 1. In both cases, the app continues to function as expected |Example SoftException from this change|Example SoftException from Fabric (no changes to this code)| |-|-| |![image](https://user-images.githubusercontent.com/10366495/233423632-9eaa7c06-90a0-4e5c-ad65-5cc98ea52dc6.png)|![image](https://user-images.githubusercontent.com/10366495/233423233-f8f0bdb4-7f90-44f8-92aa-27992f60ccf3.png)| Reviewed By: cipolleschi Differential Revision: D49270491 Pulled By: cortinico fbshipit-source-id: a489f160be44d78afb751527f70aa88e8a463ee0 --- .../react/uimanager/NativeViewHierarchyManager.java | 2 +- .../react/uimanager/UIViewOperationQueue.java | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java index 9dca8b0a37361f..53f31f3cd570a3 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/NativeViewHierarchyManager.java @@ -945,7 +945,7 @@ private ThemedReactContext getReactContextForView(int reactTag) { public void sendAccessibilityEvent(int tag, int eventType) { View view = mTagsToViews.get(tag); if (view == null) { - throw new JSApplicationIllegalArgumentException("Could not find view with tag " + tag); + throw new RetryableMountingLayerException("Could not find view with tag " + tag); } view.sendAccessibilityEvent(eventType); } diff --git a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java index 93ccb0a9fbf98b..de3ca5db4f1899 100644 --- a/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java +++ b/packages/react-native/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIViewOperationQueue.java @@ -591,7 +591,18 @@ private SendAccessibilityEvent(int tag, int eventType) { @Override public void execute() { - mNativeViewHierarchyManager.sendAccessibilityEvent(mTag, mEventType); + try { + mNativeViewHierarchyManager.sendAccessibilityEvent(mTag, mEventType); + } catch (RetryableMountingLayerException e) { + // Accessibility events are similar to commands in that they're imperative + // calls from JS, disconnected from the commit lifecycle, and therefore + // inherently unpredictable and dangerous. If we encounter a "retryable" + // error, that is, a known category of errors that this is likely to hit + // due to race conditions (like the view disappearing after the event is + // queued and before it executes), we log a soft exception and continue along. + // Other categories of errors will still cause a hard crash. + ReactSoftExceptionLogger.logSoftException(TAG, e); + } } }