From 69ecaef068f3e6e2be192246594f782bb403e4d6 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Mon, 2 Dec 2024 14:18:55 -0800 Subject: [PATCH] Remove native -> js call noop-ing after early js error (#47915) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/47915 ## Purpose of this noop-ing If an fatal js error happens during js runtime init, the js thread continues executing and raises **yet another** fatal error. This noop-ing was an **attempt** to prevent that second inactionable fatal from happening: That fatal is usually inactionable. ## Problems with this noop-ing I don't think this is the right approach: There could be legitimate reasons to continue executing native -> js calls post first js fatal. I don't think it does *much*: it doesn't noop native -> js calls executed on the runtime scheduler, which should be most of them. ## Changes Instead of trying to prevent that fatal, just let it happen. Then, don't report the second fatal: D66193194 and D66392706. ## Safetly The production impact is negligible: This codepath is executed only after early js errors. There shouldn't be any in production right now. We've spent a lot of time making our javascript error handling pipeline's coverage compresive. So, after an early js fatal error happens, subsequent js fatals should get handled properly. Changelog: [Internal] Reviewed By: javache Differential Revision: D66394278 fbshipit-source-id: ef342fc2eba9ae9f27b15a0f412fb69bd92aed43 --- .../react/runtime/ReactInstance.cpp | 88 ++++++++----------- 1 file changed, 37 insertions(+), 51 deletions(-) diff --git a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp index 2a695fc28b2..db69c4a3dd7 100644 --- a/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp +++ b/packages/react-native/ReactCommon/react/runtime/ReactInstance.cpp @@ -57,62 +57,48 @@ ReactInstance::ReactInstance( timerManager_(std::move(timerManager)), jsErrorHandler_(std::make_shared(std::move(onJsError))), parentInspectorTarget_(parentInspectorTarget) { - RuntimeExecutor runtimeExecutor = [weakRuntime = std::weak_ptr(runtime_), - weakTimerManager = - std::weak_ptr(timerManager_), - weakJsThread = - std::weak_ptr(jsMessageQueueThread_), - jsErrorHandler = - jsErrorHandler_](auto callback) { - if (weakRuntime.expired()) { - return; - } - - /** - * If a fatal error was caught while executing the main bundle, assume the - * js runtime is invalid. And stop executing any more js. - */ - if (!jsErrorHandler->isRuntimeReady() && - jsErrorHandler->hasHandledFatalError()) { - LOG(INFO) - << "RuntimeExecutor: Detected fatal error. Dropping work on non-js thread." - << std::endl; - return; - } - - if (auto jsThread = weakJsThread.lock()) { - jsThread->runOnQueue([jsErrorHandler, - weakRuntime, - weakTimerManager, - callback = std::move(callback)]() { - auto runtime = weakRuntime.lock(); - if (!runtime) { + RuntimeExecutor runtimeExecutor = + [weakRuntime = std::weak_ptr(runtime_), + weakTimerManager = std::weak_ptr(timerManager_), + weakJsThread = std::weak_ptr(jsMessageQueueThread_), + jsErrorHandler = jsErrorHandler_](auto callback) { + if (weakRuntime.expired()) { return; } - jsi::Runtime& jsiRuntime = runtime->getRuntime(); - SystraceSection s("ReactInstance::_runtimeExecutor[Callback]"); - try { - ShadowNode::setUseRuntimeShadowNodeReferenceUpdateOnThread(true); - callback(jsiRuntime); - - // If we have first-class support for microtasks, - // they would've been called as part of the previous callback. - if (ReactNativeFeatureFlags::disableEventLoopOnBridgeless()) { - if (auto timerManager = weakTimerManager.lock()) { - timerManager->callReactNativeMicrotasks(jsiRuntime); + if (auto jsThread = weakJsThread.lock()) { + jsThread->runOnQueue([jsErrorHandler, + weakRuntime, + weakTimerManager, + callback = std::move(callback)]() { + auto runtime = weakRuntime.lock(); + if (!runtime) { + return; } - } - } catch (jsi::JSError& originalError) { - jsErrorHandler->handleError(jsiRuntime, originalError, true); - } catch (std::exception& ex) { - jsi::JSError error( - jsiRuntime, std::string("Non-js exception: ") + ex.what()); - jsErrorHandler->handleError(jsiRuntime, error, true); + + jsi::Runtime& jsiRuntime = runtime->getRuntime(); + SystraceSection s("ReactInstance::_runtimeExecutor[Callback]"); + try { + ShadowNode::setUseRuntimeShadowNodeReferenceUpdateOnThread(true); + callback(jsiRuntime); + + // If we have first-class support for microtasks, + // they would've been called as part of the previous callback. + if (ReactNativeFeatureFlags::disableEventLoopOnBridgeless()) { + if (auto timerManager = weakTimerManager.lock()) { + timerManager->callReactNativeMicrotasks(jsiRuntime); + } + } + } catch (jsi::JSError& originalError) { + jsErrorHandler->handleError(jsiRuntime, originalError, true); + } catch (std::exception& ex) { + jsi::JSError error( + jsiRuntime, std::string("Non-js exception: ") + ex.what()); + jsErrorHandler->handleError(jsiRuntime, error, true); + } + }); } - }); - } - }; + }; if (parentInspectorTarget_) { auto executor = parentInspectorTarget_->executorFromThis();