From 9bc7a07de140973f9585e90948ed4ed80e19c670 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Tue, 14 Apr 2020 14:26:01 -0700 Subject: [PATCH] Retryable ViewCommand exceptions shouldn't crash Summary: Early ViewCommand Dispatch will solve this category of crashes by going through an entirely different codepath. For users not in that experiment, it might be good to have a mitigation that prevents non-critical issues from crashing (like "blur" failing). Currently, "blur" failures cause lots of screens to crash. There's no useful signal and those crashes aren't super actionable, so seems better to swallow. If/when early viewcommand dispatch ships as the default/only mode, we can remove this try/catch entirely. The only concern I have with landing this is the perf implications of putting a try/catch inside this loop. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D21023213 fbshipit-source-id: 310fe2d55a44bc424692a2365ccd5882f35f9d82 --- .../react/fabric/FabricUIManager.java | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 5bc9ab4627d..d63206e3f22 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -758,7 +758,26 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { FLog.d(TAG, "dispatchMountItems: Executing mountItem: " + m); } } - mountItem.execute(mMountingManager); + + // TODO: if early ViewCommand dispatch ships 100% as a feature, this can be removed. + // This try/catch catches Retryable errors that can only be thrown by ViewCommands, which + // won't be executed here in Early Dispatch mode. + try { + mountItem.execute(mMountingManager); + } catch (RetryableMountingLayerException e) { + // It's very common for commands to be executed on views that no longer exist - for + // example, a blur event on TextInput being fired because of a navigation event away + // from the current screen. If the exception is marked as Retryable, we log a soft + // exception but never crash in debug. + // It's not clear that logging this is even useful, because these events are very + // common, mundane, and there's not much we can do about them currently. + ReactSoftException.logSoftException( + TAG, + new ReactNoCrashSoftException( + "Caught exception executing retryable mounting layer instruction: " + + mountItem.toString(), + e)); + } } mBatchedExecutionTime += SystemClock.uptimeMillis() - batchedExecutionStartTime; }