Skip to content

Commit 41b375e

Browse files
generatedunixname1608173377072046meta-codesync[bot]
authored andcommitted
Report a fatal JS error instead of aborting the process when reading its extra data fails (#57636)
Summary: Pull Request resolved: #57636 Changelog: [Android][Fixed] - Report a fatal JS error instead of aborting the process when reading the error's extra data fails Reviewed By: andrewdacenko, javache Differential Revision: D112476648 fbshipit-source-id: d214ad35f3a9b377f3a8c216bae43196078cd0dd
1 parent 9efcdfc commit 41b375e

2 files changed

Lines changed: 34 additions & 6 deletions

File tree

packages/react-native/ReactAndroid/src/main/java/com/facebook/react/interfaces/exceptionmanager/ReactJsExceptionHandler.kt

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ internal fun interface ReactJsExceptionHandler {
3333
val stack: List<StackFrame>
3434
val id: Int
3535
val isFatal: Boolean
36-
val extraData: ReadableMap
36+
// Nullable: native marshalling may fail or the error may carry none (see
37+
// JReactExceptionManager.cpp).
38+
val extraData: ReadableMap?
3739
}
3840

3941
@DoNotStripAny
@@ -53,7 +55,7 @@ internal fun interface ReactJsExceptionHandler {
5355
override val stack: ArrayList<ProcessedErrorStackFrameImpl>,
5456
override val id: Int,
5557
override val isFatal: Boolean,
56-
override val extraData: ReadableNativeMap,
58+
override val extraData: ReadableNativeMap?,
5759
) : ProcessedError
5860

5961
fun reportJsException(errorMap: ProcessedError)

packages/react-native/ReactAndroid/src/main/jni/react/runtime/jni/JReactExceptionManager.cpp

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,37 @@ class ProcessedErrorImpl
5353
stack->add(ProcessedErrorStackFrameImpl::create(frame));
5454
}
5555

56-
auto extraDataDynamic =
57-
jsi::dynamicFromValue(runtime, jsi::Value(runtime, error.extraData));
56+
// Marshalling `extraData` out of the runtime can throw (e.g. a
57+
// jsi::JSIException when the runtime is in a bad state during a fatal
58+
// error). This runs inside the `noexcept` onJsError callback, so an
59+
// escaping exception aborts the whole process; contain it and report the
60+
// error without its extra data instead. `extraData` may end up null (here,
61+
// or when the JS value is null/undefined); the Java field is nullable.
62+
jni::local_ref<ReadableNativeMap::jhybridobject> extraData;
5863

59-
auto extraData =
60-
ReadableNativeMap::createWithContents(std::move(extraDataDynamic));
64+
auto reportWithoutExtraData = [&](const char* reason) {
65+
LOG(ERROR)
66+
<< "JReactExceptionManager: failed to marshal JS error extraData; reporting the error without it: "
67+
<< reason;
68+
try {
69+
extraData = ReadableNativeMap::createWithContents(
70+
folly::dynamic::object("extraDataMarshallingError", reason));
71+
} catch (...) {
72+
// The fallback allocation can itself throw; leave extraData null.
73+
extraData = nullptr;
74+
}
75+
};
76+
77+
try {
78+
auto extraDataDynamic =
79+
jsi::dynamicFromValue(runtime, jsi::Value(runtime, error.extraData));
80+
extraData =
81+
ReadableNativeMap::createWithContents(std::move(extraDataDynamic));
82+
} catch (const std::exception& e) {
83+
reportWithoutExtraData(e.what());
84+
} catch (...) {
85+
reportWithoutExtraData("unknown exception");
86+
}
6187

6288
return newInstance(
6389
error.message,

0 commit comments

Comments
 (0)