From 4866ec27675c44818c0e892b310965eaeb1d09f9 Mon Sep 17 00:00:00 2001 From: Felix Oghina Date: Mon, 4 Jan 2016 13:16:58 -0800 Subject: [PATCH] crash gracefully on fatal js errors (take two) Reviewed By: AaaChiuuu Differential Revision: D2773513 fb-gh-sync-id: f8e50ad12e808caf1d85c6c7859c04fcabdaaeae --- .../facebook/react/ReactInstanceManager.java | 15 ++++++++++++- .../react/ReactInstanceManagerImpl.java | 11 ++++++++-- .../react/devsupport/DevSupportManager.java | 21 +++++++++---------- 3 files changed, 33 insertions(+), 14 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java index f3544cb81..17fb538b8 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManager.java @@ -19,6 +19,7 @@ import android.app.Application; import android.content.Intent; import com.facebook.infer.annotation.Assertions; +import com.facebook.react.bridge.NativeModuleCallExceptionHandler; import com.facebook.react.bridge.NotThreadSafeBridgeIdleDebugListener; import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReactContext; @@ -159,6 +160,7 @@ public abstract class ReactInstanceManager { protected boolean mUseDeveloperSupport; protected @Nullable LifecycleState mInitialLifecycleState; protected @Nullable UIImplementationProvider mUIImplementationProvider; + protected @Nullable NativeModuleCallExceptionHandler mNativeModuleCallExceptionHandler; protected Builder() { } @@ -242,6 +244,16 @@ public abstract class ReactInstanceManager { return this; } + /** + * Set the exception handler for all native module calls. If not set, the default + * {@link DevSupportManager} will be used, which shows a redbox in dev mode and rethrows + * (crashes the app) in prod mode. + */ + public Builder setNativeModuleCallExceptionHandler(NativeModuleCallExceptionHandler handler) { + mNativeModuleCallExceptionHandler = handler; + return this; + } + /** * Instantiates a new {@link ReactInstanceManagerImpl}. * Before calling {@code build}, the following must be called: @@ -274,7 +286,8 @@ public abstract class ReactInstanceManager { mUseDeveloperSupport, mBridgeIdleDebugListener, Assertions.assertNotNull(mInitialLifecycleState, "Initial lifecycle state was not set"), - mUIImplementationProvider); + mUIImplementationProvider, + mNativeModuleCallExceptionHandler); } } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java index 66ce1a2e8..ad10a6e7c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java +++ b/ReactAndroid/src/main/java/com/facebook/react/ReactInstanceManagerImpl.java @@ -36,6 +36,7 @@ import com.facebook.react.bridge.JavaScriptExecutor; import com.facebook.react.bridge.JavaScriptModule; import com.facebook.react.bridge.JavaScriptModulesConfig; import com.facebook.react.bridge.NativeModule; +import com.facebook.react.bridge.NativeModuleCallExceptionHandler; import com.facebook.react.bridge.NativeModuleRegistry; import com.facebook.react.bridge.NotThreadSafeBridgeIdleDebugListener; import com.facebook.react.bridge.ProxyJavaScriptExecutor; @@ -103,6 +104,7 @@ import com.facebook.systrace.Systrace; private volatile boolean mHasStartedCreatingInitialContext = false; private final UIImplementationProvider mUIImplementationProvider; private final MemoryPressureRouter mMemoryPressureRouter; + private final @Nullable NativeModuleCallExceptionHandler mNativeModuleCallExceptionHandler; private final ReactInstanceDevCommandsHandler mDevInterface = new ReactInstanceDevCommandsHandler() { @@ -198,7 +200,8 @@ import com.facebook.systrace.Systrace; boolean useDeveloperSupport, @Nullable NotThreadSafeBridgeIdleDebugListener bridgeIdleDebugListener, LifecycleState initialLifecycleState, - UIImplementationProvider uiImplementationProvider) { + UIImplementationProvider uiImplementationProvider, + NativeModuleCallExceptionHandler nativeModuleCallExceptionHandler) { initializeSoLoaderIfNecessary(applicationContext); // TODO(9577825): remove this @@ -222,6 +225,7 @@ import com.facebook.systrace.Systrace; mLifecycleState = initialLifecycleState; mUIImplementationProvider = uiImplementationProvider; mMemoryPressureRouter = new MemoryPressureRouter(applicationContext); + mNativeModuleCallExceptionHandler = nativeModuleCallExceptionHandler; } @Override @@ -660,13 +664,16 @@ import com.facebook.systrace.Systrace; Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE); } + NativeModuleCallExceptionHandler exceptionHandler = mNativeModuleCallExceptionHandler != null + ? mNativeModuleCallExceptionHandler + : mDevSupportManager; CatalystInstanceImpl.Builder catalystInstanceBuilder = new CatalystInstanceImpl.Builder() .setCatalystQueueConfigurationSpec(CatalystQueueConfigurationSpec.createDefault()) .setJSExecutor(jsExecutor) .setRegistry(nativeModuleRegistry) .setJSModulesConfig(javaScriptModulesConfig) .setJSBundleLoader(jsBundleLoader) - .setNativeModuleCallExceptionHandler(mDevSupportManager); + .setNativeModuleCallExceptionHandler(exceptionHandler); Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "createCatalystInstance"); CatalystInstance catalystInstance; diff --git a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManager.java b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManager.java index 3e38a19fb..bcc094062 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/devsupport/DevSupportManager.java @@ -155,7 +155,7 @@ public class DevSupportManager implements NativeModuleCallExceptionHandler { public void handleException(Exception e) { if (mIsDevSupportEnabled) { FLog.e(ReactConstants.TAG, "Exception in native call from JS", e); - showNewError(e.getMessage(), StackTraceHelper.convertJavaStackTrace(e), JAVA_ERROR_COOKIE); + showNewJavaError(e.getMessage(), e); } else { if (e instanceof RuntimeException) { // Because we are rethrowing the original exception, the original stacktrace will be @@ -167,6 +167,10 @@ public class DevSupportManager implements NativeModuleCallExceptionHandler { } } + public void showNewJavaError(String message, Throwable e) { + showNewError(message, StackTraceHelper.convertJavaStackTrace(e), JAVA_ERROR_COOKIE); + } + /** * Add option item to dev settings dialog displayed by this manager. In the case user select given * option from that dialog, the appropriate handler passed as {@param optionHandler} will be @@ -555,10 +559,9 @@ public class DevSupportManager implements NativeModuleCallExceptionHandler { new Runnable() { @Override public void run() { - showNewError( + showNewJavaError( mApplicationContext.getString(R.string.catalyst_remotedbg_error), - StackTraceHelper.convertJavaStackTrace(cause), - JAVA_ERROR_COOKIE); + cause); } }); } @@ -590,15 +593,11 @@ public class DevSupportManager implements NativeModuleCallExceptionHandler { public void run() { if (cause instanceof DebugServerException) { DebugServerException debugServerException = (DebugServerException) cause; - showNewError( - debugServerException.description, - StackTraceHelper.convertJavaStackTrace(cause), - JAVA_ERROR_COOKIE); + showNewJavaError(debugServerException.description, cause); } else { - showNewError( + showNewJavaError( mApplicationContext.getString(R.string.catalyst_jsload_error), - StackTraceHelper.convertJavaStackTrace(cause), - JAVA_ERROR_COOKIE); + cause); } } });