Skip to content

Commit

Permalink
Destroy React Native instance after catching a fatal js error
Browse files Browse the repository at this point in the history
Summary:
As title, destroy React Native instance after catching a fatal js error to avoid incoming calls into JS.

Changelog:
[Android][Changed] - Rename NativeModuleCallExceptionHandler to JSExceptionHandler for broader usage

Reviewed By: fkgozali

Differential Revision: D37379340

fbshipit-source-id: 465a30bc824a264b45df3e8b0b24edd61c4b571d
  • Loading branch information
luluwu2032 authored and facebook-github-bot committed Jun 28, 2022
1 parent ca8481b commit b6f7689
Show file tree
Hide file tree
Showing 19 changed files with 76 additions and 85 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -218,8 +218,8 @@ public void loadBundle(
builder.setReactPackageTurboModuleManagerDelegateBuilder(
spec.getReactPackageTurboModuleManagerDelegateBuilder());
}
if (spec.getNativeModuleCallExceptionHandler() != null) {
builder.setNativeModuleCallExceptionHandler(spec.getNativeModuleCallExceptionHandler());
if (spec.getJSExceptionHandler() != null) {
builder.setJSExceptionHandler(spec.getJSExceptionHandler());
}

if (!spec.getAlternativeReactPackagesForTest().isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,11 @@
import androidx.annotation.Nullable;
import com.facebook.react.ReactPackage;
import com.facebook.react.ReactPackageTurboModuleManagerDelegate;
import com.facebook.react.bridge.JSExceptionHandler;
import com.facebook.react.bridge.JSIModuleSpec;
import com.facebook.react.bridge.JavaScriptExecutorFactory;
import com.facebook.react.bridge.JavaScriptModule;
import com.facebook.react.bridge.NativeModule;
import com.facebook.react.bridge.NativeModuleCallExceptionHandler;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.uimanager.ViewManager;
import java.util.ArrayList;
Expand All @@ -40,7 +40,7 @@ public abstract static class JSIModuleBuilder {
private final List<Class<? extends JavaScriptModule>> mJSModuleSpecs = new ArrayList<>();
private final List<ViewManager> mViewManagers = new ArrayList<>();
private final ArrayList<ReactPackage> mReactPackages = new ArrayList<>();
@Nullable private NativeModuleCallExceptionHandler mNativeModuleCallExceptionHandler = null;
@Nullable private JSExceptionHandler mJSExceptionHandler = null;
@Nullable private FabricUIManagerFactory mFabricUIManagerFactory = null;
@Nullable private JavaScriptExecutorFactory mJavaScriptExecutorFactory = null;
@Nullable private ReactPackageTurboModuleManagerDelegate.Builder mTMMDelegateBuilder = null;
Expand All @@ -51,14 +51,13 @@ public ReactInstanceSpecForTest addNativeModule(NativeModule module) {
return this;
}

public ReactInstanceSpecForTest setNativeModuleCallExceptionHandler(
NativeModuleCallExceptionHandler nativeModuleCallExceptionHandler) {
mNativeModuleCallExceptionHandler = nativeModuleCallExceptionHandler;
public ReactInstanceSpecForTest setJSExceptionHandler(JSExceptionHandler jSExceptionHandler) {
mJSExceptionHandler = jSExceptionHandler;
return this;
}

public NativeModuleCallExceptionHandler getNativeModuleCallExceptionHandler() {
return mNativeModuleCallExceptionHandler;
public JSExceptionHandler getJSExceptionHandler() {
return mJSExceptionHandler;
}

public ReactInstanceSpecForTest setJavaScriptExecutorFactory(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@
import com.facebook.react.bridge.CatalystInstance;
import com.facebook.react.bridge.CatalystInstanceImpl;
import com.facebook.react.bridge.JSBundleLoader;
import com.facebook.react.bridge.JSExceptionHandler;
import com.facebook.react.bridge.JavaScriptExecutor;
import com.facebook.react.bridge.NativeModule;
import com.facebook.react.bridge.NativeModuleCallExceptionHandler;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.queue.ReactQueueConfigurationSpec;
import com.facebook.react.modules.core.ReactChoreographer;
Expand Down Expand Up @@ -90,8 +90,8 @@ public CatalystInstance build() {
.setJSBundleLoader(
JSBundleLoader.createAssetLoader(
mContext, "assets://AndroidTestBundle.js", false /* Asynchronous */))
.setNativeModuleCallExceptionHandler(
new NativeModuleCallExceptionHandler() {
.setJSExceptionHandler(
new JSExceptionHandler() {
@Override
public void handleException(Exception e) {
throw new RuntimeException(e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@
import com.facebook.react.bridge.CatalystInstance;
import com.facebook.react.bridge.CatalystInstanceImpl;
import com.facebook.react.bridge.JSBundleLoader;
import com.facebook.react.bridge.JSExceptionHandler;
import com.facebook.react.bridge.JSIModulePackage;
import com.facebook.react.bridge.JSIModuleType;
import com.facebook.react.bridge.JavaJSExecutor;
import com.facebook.react.bridge.JavaScriptExecutor;
import com.facebook.react.bridge.JavaScriptExecutorFactory;
import com.facebook.react.bridge.NativeModuleCallExceptionHandler;
import com.facebook.react.bridge.NativeModuleRegistry;
import com.facebook.react.bridge.NotThreadSafeBridgeIdleDebugListener;
import com.facebook.react.bridge.ProxyJavaScriptExecutor;
Expand Down Expand Up @@ -182,7 +182,7 @@ public interface ReactInstanceEventListener
// while true any spawned create thread should wait for proper clean up before initializing
private volatile Boolean mHasStartedDestroying = false;
private final MemoryPressureRouter mMemoryPressureRouter;
private final @Nullable NativeModuleCallExceptionHandler mNativeModuleCallExceptionHandler;
private final @Nullable JSExceptionHandler mJSExceptionHandler;
private final @Nullable JSIModulePackage mJSIModulePackage;
private final @Nullable ReactPackageTurboModuleManagerDelegate.Builder mTMMDelegateBuilder;
private List<ViewManager> mViewManagers;
Expand Down Expand Up @@ -226,7 +226,7 @@ public static ReactInstanceManagerBuilder builder() {
@Nullable NotThreadSafeBridgeIdleDebugListener bridgeIdleDebugListener,
LifecycleState initialLifecycleState,
@Nullable UIImplementationProvider mUIImplementationProvider,
NativeModuleCallExceptionHandler nativeModuleCallExceptionHandler,
JSExceptionHandler jSExceptionHandler,
@Nullable RedBoxHandler redBoxHandler,
boolean lazyViewManagersEnabled,
@Nullable DevBundleDownloadListener devBundleDownloadListener,
Expand Down Expand Up @@ -268,7 +268,7 @@ public static ReactInstanceManagerBuilder builder() {
mBridgeIdleDebugListener = bridgeIdleDebugListener;
mLifecycleState = initialLifecycleState;
mMemoryPressureRouter = new MemoryPressureRouter(applicationContext);
mNativeModuleCallExceptionHandler = nativeModuleCallExceptionHandler;
mJSExceptionHandler = jSExceptionHandler;
mTMMDelegateBuilder = tmmDelegateBuilder;
synchronized (mPackages) {
PrinterHolder.getPrinter()
Expand Down Expand Up @@ -1331,11 +1331,9 @@ private ReactApplicationContext createReactContext(
ReactMarker.logMarker(CREATE_REACT_CONTEXT_START, jsExecutor.getName());
final ReactApplicationContext reactContext = new ReactApplicationContext(mApplicationContext);

NativeModuleCallExceptionHandler exceptionHandler =
mNativeModuleCallExceptionHandler != null
? mNativeModuleCallExceptionHandler
: mDevSupportManager;
reactContext.setNativeModuleCallExceptionHandler(exceptionHandler);
JSExceptionHandler exceptionHandler =
mJSExceptionHandler != null ? mJSExceptionHandler : mDevSupportManager;
reactContext.setJSExceptionHandler(exceptionHandler);

NativeModuleRegistry nativeModuleRegistry = processPackages(reactContext, mPackages, false);

Expand All @@ -1345,7 +1343,7 @@ private ReactApplicationContext createReactContext(
.setJSExecutor(jsExecutor)
.setRegistry(nativeModuleRegistry)
.setJSBundleLoader(jsBundleLoader)
.setNativeModuleCallExceptionHandler(exceptionHandler);
.setJSExceptionHandler(exceptionHandler);

ReactMarker.logMarker(CREATE_CATALYST_INSTANCE_START);
// CREATE_CATALYST_INSTANCE_END is in JSCExecutor.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@
import com.facebook.hermes.reactexecutor.HermesExecutorFactory;
import com.facebook.infer.annotation.Assertions;
import com.facebook.react.bridge.JSBundleLoader;
import com.facebook.react.bridge.JSExceptionHandler;
import com.facebook.react.bridge.JSIModulePackage;
import com.facebook.react.bridge.JavaScriptExecutorFactory;
import com.facebook.react.bridge.NativeModuleCallExceptionHandler;
import com.facebook.react.bridge.NotThreadSafeBridgeIdleDebugListener;
import com.facebook.react.common.LifecycleState;
import com.facebook.react.common.SurfaceDelegateFactory;
Expand Down Expand Up @@ -53,7 +53,7 @@ public class ReactInstanceManagerBuilder {
private boolean mRequireActivity;
private @Nullable LifecycleState mInitialLifecycleState;
private @Nullable UIImplementationProvider mUIImplementationProvider;
private @Nullable NativeModuleCallExceptionHandler mNativeModuleCallExceptionHandler;
private @Nullable JSExceptionHandler mJSExceptionHandler;
private @Nullable Activity mCurrentActivity;
private @Nullable DefaultHardwareBackBtnHandler mDefaultHardwareBackBtnHandler;
private @Nullable RedBoxHandler mRedBoxHandler;
Expand Down Expand Up @@ -253,9 +253,8 @@ public ReactInstanceManagerBuilder setInitialLifecycleState(
* DevSupportManager} will be used, which shows a redbox in dev mode and rethrows (crashes the
* app) in prod mode.
*/
public ReactInstanceManagerBuilder setNativeModuleCallExceptionHandler(
NativeModuleCallExceptionHandler handler) {
mNativeModuleCallExceptionHandler = handler;
public ReactInstanceManagerBuilder setJSExceptionHandler(JSExceptionHandler handler) {
mJSExceptionHandler = handler;
return this;
}

Expand Down Expand Up @@ -357,7 +356,7 @@ public ReactInstanceManager build() {
mBridgeIdleDebugListener,
Assertions.assertNotNull(mInitialLifecycleState, "Initial lifecycle state was not set"),
mUIImplementationProvider,
mNativeModuleCallExceptionHandler,
mJSExceptionHandler,
mRedBoxHandler,
mLazyViewManagersEnabled,
mDevBundleDownloadListener,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@

/**
* Like {@link AssertionError} but extends RuntimeException so that it may be caught by a {@link
* NativeModuleCallExceptionHandler}. See that class for more details. Used in conjunction with
* {@link SoftAssertions}.
* JSExceptionHandler}. See that class for more details. Used in conjunction with {@link
* SoftAssertions}.
*/
public class AssertionException extends RuntimeException {

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public String toString() {

private final NativeModuleRegistry mNativeModuleRegistry;
private final JSIModuleRegistry mJSIModuleRegistry = new JSIModuleRegistry();
private final NativeModuleCallExceptionHandler mNativeModuleCallExceptionHandler;
private final JSExceptionHandler mJSExceptionHandler;
private final MessageQueueThread mNativeModulesQueueThread;
private boolean mInitialized = false;
private volatile boolean mAcceptCalls = false;
Expand All @@ -120,7 +120,7 @@ private CatalystInstanceImpl(
final JavaScriptExecutor jsExecutor,
final NativeModuleRegistry nativeModuleRegistry,
final JSBundleLoader jsBundleLoader,
NativeModuleCallExceptionHandler nativeModuleCallExceptionHandler) {
JSExceptionHandler jSExceptionHandler) {
FLog.d(ReactConstants.TAG, "Initializing React Xplat Bridge.");
Systrace.beginSection(TRACE_TAG_REACT_JAVA_BRIDGE, "createCatalystInstanceImpl");

Expand All @@ -141,7 +141,7 @@ private CatalystInstanceImpl(
mNativeModuleRegistry = nativeModuleRegistry;
mJSModuleRegistry = new JavaScriptModuleRegistry();
mJSBundleLoader = jsBundleLoader;
mNativeModuleCallExceptionHandler = nativeModuleCallExceptionHandler;
mJSExceptionHandler = jSExceptionHandler;
mNativeModulesQueueThread = mReactQueueConfiguration.getNativeModulesQueueThread();
mTraceListener = new JSProfilerTraceListener(this);
Systrace.endSection(TRACE_TAG_REACT_JAVA_BRIDGE);
Expand Down Expand Up @@ -626,7 +626,7 @@ public void run() {
}

private void onNativeException(Exception e) {
mNativeModuleCallExceptionHandler.handleException(e);
mJSExceptionHandler.handleException(e);
mReactQueueConfiguration
.getUIQueueThread()
.runOnQueue(
Expand Down Expand Up @@ -682,7 +682,7 @@ public static class Builder {
private @Nullable JSBundleLoader mJSBundleLoader;
private @Nullable NativeModuleRegistry mRegistry;
private @Nullable JavaScriptExecutor mJSExecutor;
private @Nullable NativeModuleCallExceptionHandler mNativeModuleCallExceptionHandler;
private @Nullable JSExceptionHandler mJSExceptionHandler;

public Builder setReactQueueConfigurationSpec(
ReactQueueConfigurationSpec ReactQueueConfigurationSpec) {
Expand All @@ -705,8 +705,8 @@ public Builder setJSExecutor(JavaScriptExecutor jsExecutor) {
return this;
}

public Builder setNativeModuleCallExceptionHandler(NativeModuleCallExceptionHandler handler) {
mNativeModuleCallExceptionHandler = handler;
public Builder setJSExceptionHandler(JSExceptionHandler handler) {
mJSExceptionHandler = handler;
return this;
}

Expand All @@ -716,7 +716,7 @@ public CatalystInstanceImpl build() {
Assertions.assertNotNull(mJSExecutor),
Assertions.assertNotNull(mRegistry),
Assertions.assertNotNull(mJSBundleLoader),
Assertions.assertNotNull(mNativeModuleCallExceptionHandler));
Assertions.assertNotNull(mJSExceptionHandler));
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
package com.facebook.react.bridge;

/** Crashy crashy exception handler. */
public class DefaultNativeModuleCallExceptionHandler implements NativeModuleCallExceptionHandler {
public class DefaultJSExceptionHandler implements JSExceptionHandler {

@Override
public void handleException(Exception e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,21 @@

/**
* Abstract base for a AsyncTask that should have any RuntimeExceptions it throws handled by the
* {@link com.facebook.react.bridge.NativeModuleCallExceptionHandler} registered if the app is in
* dev mode.
* {@link JSExceptionHandler} registered if the app is in dev mode.
*
* <p>This class doesn't allow doInBackground to return a results. If you need this use
* GuardedResultAsyncTask instead.
*/
public abstract class GuardedAsyncTask<Params, Progress> extends AsyncTask<Params, Progress, Void> {

private final NativeModuleCallExceptionHandler mExceptionHandler;
private final JSExceptionHandler mExceptionHandler;

@Deprecated
protected GuardedAsyncTask(ReactContext reactContext) {
this(reactContext.getExceptionHandler());
}

protected GuardedAsyncTask(NativeModuleCallExceptionHandler exceptionHandler) {
protected GuardedAsyncTask(JSExceptionHandler exceptionHandler) {
mExceptionHandler = exceptionHandler;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,19 +11,18 @@

/**
* Abstract base for a AsyncTask with result support that should have any RuntimeExceptions it
* throws handled by the {@link com.facebook.react.bridge.NativeModuleCallExceptionHandler}
* registered if the app is in dev mode.
* throws handled by the {@link JSExceptionHandler} registered if the app is in dev mode.
*/
public abstract class GuardedResultAsyncTask<Result> extends AsyncTask<Void, Void, Result> {

private final NativeModuleCallExceptionHandler mExceptionHandler;
private final JSExceptionHandler mExceptionHandler;

@Deprecated
protected GuardedResultAsyncTask(ReactContext reactContext) {
this(reactContext.getExceptionHandler());
}

protected GuardedResultAsyncTask(NativeModuleCallExceptionHandler exceptionHandler) {
protected GuardedResultAsyncTask(JSExceptionHandler exceptionHandler) {
mExceptionHandler = exceptionHandler;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,18 @@

/**
* Abstract base for a Runnable that should have any RuntimeExceptions it throws handled by the
* {@link com.facebook.react.bridge.NativeModuleCallExceptionHandler} registered if the app is in
* dev mode.
* {@link JSExceptionHandler} registered if the app is in dev mode.
*/
public abstract class GuardedRunnable implements Runnable {

private final NativeModuleCallExceptionHandler mExceptionHandler;
private final JSExceptionHandler mExceptionHandler;

@Deprecated
public GuardedRunnable(ReactContext reactContext) {
this(reactContext.getExceptionHandler());
}

public GuardedRunnable(NativeModuleCallExceptionHandler exceptionHandler) {
public GuardedRunnable(JSExceptionHandler exceptionHandler) {
mExceptionHandler = exceptionHandler;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,15 @@
package com.facebook.react.bridge;

/**
* Interface for a class that knows how to handle an Exception thrown by a native module invoked
* from JS. Since these Exceptions are triggered by JS calls (and can be fixed in JS), a common way
* to handle one is to show a error dialog and allow the developer to change and reload JS.
* Interface for a class that knows how to handle an Exception invoked from JS. Since these
* Exceptions are triggered by JS calls (and can be fixed in JS), a common way to handle one is to
* show a error dialog and allow the developer to change and reload JS.
*
* <p>We should also note that we have a unique stance on what 'caused' means: even if there's a bug
* in the framework/native code, it was triggered by JS and theoretically since we were able to set
* up the bridge, JS could change its logic, reload, and not trigger that crash.
* up the React Instance, JS could change its logic, reload, and not trigger that crash.
*/
public interface NativeModuleCallExceptionHandler {
public interface JSExceptionHandler {

/** Do something to display or log the exception. */
void handleException(Exception e);
Expand Down
Loading

0 comments on commit b6f7689

Please sign in to comment.