Skip to content

Commit 7558464

Browse files
alanleedevcortinico
alanleedev
authored and
cortinico
committed
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 Reviewed By: javache, cortinico Differential Revision: D58385767 fbshipit-source-id: 46548677df936b7c2f584084a2c9769c27e6a963
1 parent f86bc95 commit 7558464

File tree

5 files changed

+53
-21
lines changed

5 files changed

+53
-21
lines changed

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

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

37813781
public class com/facebook/react/runtime/ReactHostImpl : com/facebook/react/ReactHost {
3782-
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
3783-
public fun <init> (Landroid/content/Context;Lcom/facebook/react/runtime/ReactHostDelegate;Lcom/facebook/react/fabric/ComponentFactory;ZLcom/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler;Z)V
3782+
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
3783+
public fun <init> (Landroid/content/Context;Lcom/facebook/react/runtime/ReactHostDelegate;Lcom/facebook/react/fabric/ComponentFactory;ZZ)V
37843784
public fun addBeforeDestroyListener (Lkotlin/jvm/functions/Function0;)V
37853785
public fun addReactInstanceEventListener (Lcom/facebook/react/ReactInstanceEventListener;)V
37863786
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;
@@ -145,15 +143,13 @@ public ReactHostImpl(
145143
ReactHostDelegate delegate,
146144
ComponentFactory componentFactory,
147145
boolean allowPackagerServerAccess,
148-
ReactJsExceptionHandler reactJsExceptionHandler,
149146
boolean useDevSupport) {
150147
this(
151148
context,
152149
delegate,
153150
componentFactory,
154151
Executors.newSingleThreadExecutor(),
155152
Task.UI_THREAD_EXECUTOR,
156-
reactJsExceptionHandler,
157153
allowPackagerServerAccess,
158154
useDevSupport);
159155
}
@@ -164,15 +160,13 @@ public ReactHostImpl(
164160
ComponentFactory componentFactory,
165161
Executor bgExecutor,
166162
Executor uiExecutor,
167-
ReactJsExceptionHandler reactJsExceptionHandler,
168163
boolean allowPackagerServerAccess,
169164
boolean useDevSupport) {
170165
mContext = context;
171166
mReactHostDelegate = delegate;
172167
mComponentFactory = componentFactory;
173168
mBGExecutor = bgExecutor;
174169
mUIExecutor = uiExecutor;
175-
mReactJsExceptionHandler = reactJsExceptionHandler;
176170
mQueueThreadExceptionHandler = ReactHostImpl.this::handleHostException;
177171
mMemoryPressureRouter = new MemoryPressureRouter(context);
178172
mAllowPackagerServerAccess = allowPackagerServerAccess;
@@ -1074,7 +1068,6 @@ private Task<ReactInstance> getOrCreateReactInstanceTask() {
10741068
mComponentFactory,
10751069
devSupportManager,
10761070
mQueueThreadExceptionHandler,
1077-
mReactJsExceptionHandler,
10781071
mUseDevSupport,
10791072
getOrCreateReactHostInspectorTarget());
10801073
mReactInstance = instance;

‎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);
@@ -313,6 +323,44 @@ public ReactQueueConfiguration getReactQueueConfiguration() {
313323
return mQueueConfiguration;
314324
}
315325

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