Skip to content

Commit

Permalink
Retryable ViewCommand exceptions shouldn't crash
Browse files Browse the repository at this point in the history
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
  • Loading branch information
JoshuaGross authored and facebook-github-bot committed Apr 14, 2020
1 parent e0da72a commit 9bc7a07
Showing 1 changed file with 20 additions and 1 deletion.
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,26 @@ private boolean dispatchMountItems() {
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;
}
Expand Down

0 comments on commit 9bc7a07

Please sign in to comment.