Skip to content

Commit c365360

Browse files
alanleedevfacebook-github-bot
authored andcommitted
add default early JS error handler (#44884)
Summary: Pull Request resolved: #44884 Removing JavaScript error handler supplied to ReactHostImpl.java which is just a stub and creating a default handler in ReactInstance.java which uses NativeExceptionHandler TurboModule to handle error. Changelog: [Android][BREAKING] Removing `ReactJsExceptionHandler` param from ReactHostImpl() constructor and providing a default private implementation Differential Revision: D58385767
1 parent b19bf2b commit c365360

5 files changed

Lines changed: 53 additions & 21 deletions

File tree

packages/react-native/ReactAndroid/api/ReactAndroid.api

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3778,8 +3778,8 @@ public abstract class com/facebook/react/runtime/JSRuntimeFactory {
37783778
}
37793779

37803780
public class com/facebook/react/runtime/ReactHostImpl : com/facebook/react/ReactHost {
3781-
public fun <init> (Landroid/content/Context;Lcom/facebook/react/runtime/ReactHostDelegate;Lcom/facebook/react/fabric/ComponentFactory;Ljava/util/concurrent/Executor;Ljava/util/concurrent/Executor;Lcom/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler;ZZ)V
3782-
public fun <init> (Landroid/content/Context;Lcom/facebook/react/runtime/ReactHostDelegate;Lcom/facebook/react/fabric/ComponentFactory;ZLcom/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler;Z)V
3781+
public fun <init> (Landroid/content/Context;Lcom/facebook/react/runtime/ReactHostDelegate;Lcom/facebook/react/fabric/ComponentFactory;Ljava/util/concurrent/Executor;Ljava/util/concurrent/Executor;ZZ)V
3782+
public fun <init> (Landroid/content/Context;Lcom/facebook/react/runtime/ReactHostDelegate;Lcom/facebook/react/fabric/ComponentFactory;ZZ)V
37833783
public fun addBeforeDestroyListener (Lkotlin/jvm/functions/Function0;)V
37843784
public fun addReactInstanceEventListener (Lcom/facebook/react/ReactInstanceEventListener;)V
37853785
public fun createSurface (Landroid/content/Context;Ljava/lang/String;Landroid/os/Bundle;)Lcom/facebook/react/interfaces/fabric/ReactSurface;

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/defaults/DefaultReactHost.kt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ import com.facebook.react.bridge.ReactContext
1717
import com.facebook.react.common.annotations.UnstableReactNativeAPI
1818
import com.facebook.react.common.build.ReactBuildConfig
1919
import com.facebook.react.fabric.ComponentFactory
20-
import com.facebook.react.interfaces.exceptionmanager.ReactJsExceptionHandler
2120
import com.facebook.react.runtime.JSCInstance
2221
import com.facebook.react.runtime.ReactHostImpl
2322
import com.facebook.react.runtime.cxxreactpackage.CxxReactPackage
@@ -74,8 +73,6 @@ public object DefaultReactHost {
7473
reactPackages = packageList,
7574
jsRuntimeFactory = jsRuntimeFactory,
7675
turboModuleManagerDelegateBuilder = defaultTmmDelegateBuilder)
77-
// TODO: T180971255 Improve default exception handler
78-
val reactJsExceptionHandler = ReactJsExceptionHandler { _ -> }
7976
val componentFactory = ComponentFactory()
8077
DefaultComponentsRegistry.register(componentFactory)
8178
// TODO: T164788699 find alternative of accessing ReactHostImpl for initialising reactHost
@@ -85,7 +82,6 @@ public object DefaultReactHost {
8582
defaultReactHostDelegate,
8683
componentFactory,
8784
true /* allowPackagerServerAccess */,
88-
reactJsExceptionHandler,
8985
useDevSupport,
9086
)
9187
.apply {

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactHostImpl.java

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@
5757
import com.facebook.react.fabric.ComponentFactory;
5858
import com.facebook.react.fabric.FabricUIManager;
5959
import com.facebook.react.interfaces.TaskInterface;
60-
import com.facebook.react.interfaces.exceptionmanager.ReactJsExceptionHandler;
6160
import com.facebook.react.interfaces.fabric.ReactSurface;
6261
import com.facebook.react.internal.featureflags.ReactNativeFeatureFlags;
6362
import com.facebook.react.modules.appearance.AppearanceModule;
@@ -102,7 +101,6 @@ public class ReactHostImpl implements ReactHost {
102101
private final Context mContext;
103102
private final ReactHostDelegate mReactHostDelegate;
104103
private final ComponentFactory mComponentFactory;
105-
private final ReactJsExceptionHandler mReactJsExceptionHandler;
106104
private final DevSupportManager mDevSupportManager;
107105
private final Executor mBGExecutor;
108106
private final Executor mUIExecutor;
@@ -143,15 +141,13 @@ public ReactHostImpl(
143141
ReactHostDelegate delegate,
144142
ComponentFactory componentFactory,
145143
boolean allowPackagerServerAccess,
146-
ReactJsExceptionHandler reactJsExceptionHandler,
147144
boolean useDevSupport) {
148145
this(
149146
context,
150147
delegate,
151148
componentFactory,
152149
Executors.newSingleThreadExecutor(),
153150
Task.UI_THREAD_EXECUTOR,
154-
reactJsExceptionHandler,
155151
allowPackagerServerAccess,
156152
useDevSupport);
157153
}
@@ -162,15 +158,13 @@ public ReactHostImpl(
162158
ComponentFactory componentFactory,
163159
Executor bgExecutor,
164160
Executor uiExecutor,
165-
ReactJsExceptionHandler reactJsExceptionHandler,
166161
boolean allowPackagerServerAccess,
167162
boolean useDevSupport) {
168163
mContext = context;
169164
mReactHostDelegate = delegate;
170165
mComponentFactory = componentFactory;
171166
mBGExecutor = bgExecutor;
172167
mUIExecutor = uiExecutor;
173-
mReactJsExceptionHandler = reactJsExceptionHandler;
174168
mQueueThreadExceptionHandler = ReactHostImpl.this::handleHostException;
175169
mMemoryPressureRouter = new MemoryPressureRouter(context);
176170
mAllowPackagerServerAccess = allowPackagerServerAccess;
@@ -1067,7 +1061,6 @@ private Task<ReactInstance> getOrCreateReactInstanceTask() {
10671061
mComponentFactory,
10681062
devSupportManager,
10691063
mQueueThreadExceptionHandler,
1070-
mReactJsExceptionHandler,
10711064
mUseDevSupport,
10721065
getOrCreateReactHostInspectorTarget());
10731066

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/runtime/ReactInstance.java

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,16 @@
77

88
package com.facebook.react.runtime;
99

10+
import static com.facebook.react.util.JSStackTrace.COLUMN_KEY;
11+
import static com.facebook.react.util.JSStackTrace.FILE_KEY;
12+
import static com.facebook.react.util.JSStackTrace.LINE_NUMBER_KEY;
13+
import static com.facebook.react.util.JSStackTrace.METHOD_NAME_KEY;
14+
1015
import android.content.res.AssetManager;
1116
import android.view.View;
1217
import com.facebook.common.logging.FLog;
18+
import com.facebook.fbreact.specs.NativeExceptionsManagerSpec;
19+
import com.facebook.infer.annotation.Assertions;
1320
import com.facebook.infer.annotation.Nullsafe;
1421
import com.facebook.infer.annotation.ThreadConfined;
1522
import com.facebook.infer.annotation.ThreadSafe;
@@ -21,12 +28,15 @@
2128
import com.facebook.react.bridge.Arguments;
2229
import com.facebook.react.bridge.JSBundleLoader;
2330
import com.facebook.react.bridge.JSBundleLoaderDelegate;
31+
import com.facebook.react.bridge.JavaOnlyArray;
32+
import com.facebook.react.bridge.JavaOnlyMap;
2433
import com.facebook.react.bridge.JavaScriptContextHolder;
2534
import com.facebook.react.bridge.NativeArray;
2635
import com.facebook.react.bridge.NativeMap;
2736
import com.facebook.react.bridge.NativeModule;
2837
import com.facebook.react.bridge.ReactNoCrashSoftException;
2938
import com.facebook.react.bridge.ReactSoftExceptionLogger;
39+
import com.facebook.react.bridge.ReadableMap;
3040
import com.facebook.react.bridge.RuntimeExecutor;
3141
import com.facebook.react.bridge.RuntimeScheduler;
3242
import com.facebook.react.bridge.queue.MessageQueueThread;
@@ -110,7 +120,6 @@ final class ReactInstance {
110120
ComponentFactory componentFactory,
111121
DevSupportManager devSupportManager,
112122
QueueThreadExceptionHandler exceptionHandler,
113-
ReactJsExceptionHandler reactExceptionManager,
114123
boolean useDevSupport,
115124
@Nullable ReactHostInspectorTarget reactHostInspectorTarget) {
116125
mBridgelessReactContext = bridgelessReactContext;
@@ -154,14 +163,15 @@ final class ReactInstance {
154163
// Notify JS if profiling is enabled
155164
boolean isProfiling =
156165
Systrace.isTracing(Systrace.TRACE_TAG_REACT_APPS | Systrace.TRACE_TAG_REACT_JS_VM_CALLS);
166+
157167
mHybridData =
158168
initHybrid(
159169
jsRuntimeFactory,
160170
jsMessageQueueThread,
161171
nativeModulesMessageQueueThread,
162172
mJavaTimerManager,
163173
jsTimerExecutor,
164-
reactExceptionManager,
174+
new ReactJsExceptionHandlerImpl(nativeModulesMessageQueueThread),
165175
bindingsInstaller,
166176
isProfiling,
167177
reactHostInspectorTarget);
@@ -311,6 +321,44 @@ public ReactQueueConfiguration getReactQueueConfiguration() {
311321
return mQueueConfiguration;
312322
}
313323

324+
private class ReactJsExceptionHandlerImpl implements ReactJsExceptionHandler {
325+
private final MessageQueueThread mNativemodulesmessagequeuethread;
326+
327+
ReactJsExceptionHandlerImpl(MessageQueueThread nativeModulesMessageQueueThread) {
328+
this.mNativemodulesmessagequeuethread = nativeModulesMessageQueueThread;
329+
}
330+
331+
@Override
332+
public void reportJsException(ParsedError error) {
333+
List<ReactJsExceptionHandler.ParsedError.StackFrame> frames = error.getFrames();
334+
List<ReadableMap> readableMapList = new ArrayList<>();
335+
for (ReactJsExceptionHandler.ParsedError.StackFrame frame : frames) {
336+
JavaOnlyMap map = new JavaOnlyMap();
337+
map.putDouble(COLUMN_KEY, frame.getColumnNumber());
338+
map.putDouble(LINE_NUMBER_KEY, frame.getLineNumber());
339+
map.putString(FILE_KEY, (String) frame.getFileName());
340+
map.putString(METHOD_NAME_KEY, (String) frame.getMethodName());
341+
readableMapList.add(map);
342+
}
343+
344+
JavaOnlyMap data = new JavaOnlyMap();
345+
data.putString("message", error.getMessage());
346+
data.putArray("stack", JavaOnlyArray.from(readableMapList));
347+
data.putInt("id", error.getExceptionId());
348+
data.putBoolean("isFatal", error.isFatal());
349+
350+
// Simulate async native module method call
351+
mNativemodulesmessagequeuethread.runOnQueue(
352+
() -> {
353+
NativeExceptionsManagerSpec exceptionsManager =
354+
(NativeExceptionsManagerSpec)
355+
Assertions.assertNotNull(
356+
mTurboModuleManager.getModule(NativeExceptionsManagerSpec.NAME));
357+
exceptionsManager.reportException(data);
358+
});
359+
}
360+
}
361+
314362
public void loadJSBundle(JSBundleLoader bundleLoader) {
315363
// Load the JS bundle
316364
Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "ReactInstance.loadJSBundle");

packages/react-native/ReactAndroid/src/test/java/com/facebook/react/runtime/ReactHostTest.kt

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,12 +84,7 @@ class ReactHostTest {
8484
Mockito.doReturn(jSBundleLoader).`when`(reactHostDelegate).jsBundleLoader
8585
reactHost =
8686
ReactHostImpl(
87-
activityController.get().application,
88-
reactHostDelegate,
89-
componentFactory,
90-
false,
91-
{},
92-
false)
87+
activityController.get().application, reactHostDelegate, componentFactory, false, false)
9388
val taskCompletionSource = TaskCompletionSource<Boolean>().apply { setResult(true) }
9489
mockedTaskCompletionSourceCtor =
9590
Mockito.mockConstruction(

0 commit comments

Comments
 (0)