Skip to content

Fix LogBox parsing for cross-runtime jsi::JSErrors #55994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Naim08 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix LogBox parsing for cross-runtime jsi::JSErrors #55994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 29 additions & 2 deletions ...ages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ void RuntimeScheduler_Legacy::callExpiredTasks(jsi::Runtime& runtime) {
executeTask(runtime, topPriorityTask, didUserCallbackTimeout);
}
} catch (jsi::JSError& error) {
onTaskError_(runtime, error);
reportError(runtime, error);
} catch (std::exception& ex) {
jsi::JSError error(runtime, std::string("Non-js exception: ") + ex.what());
onTaskError_(runtime, error);
Expand Down Expand Up @@ -242,7 +242,7 @@ void RuntimeScheduler_Legacy::startWorkLoop(jsi::Runtime& runtime) {
executeTask(runtime, topPriorityTask, didUserCallbackTimeout);
}
} catch (jsi::JSError& error) {
onTaskError_(runtime, error);
reportError(runtime, error);
} catch (std::exception& ex) {
jsi::JSError error(runtime, std::string("Non-js exception: ") + ex.what());
onTaskError_(runtime, error);
Expand All @@ -252,6 +252,33 @@ void RuntimeScheduler_Legacy::startWorkLoop(jsi::Runtime& runtime) {
isPerformingWork_ = false;
}

void RuntimeScheduler_Legacy::reportError(
jsi::Runtime& runtime,
jsi::JSError& error) const {
try {
auto errorCtor =
runtime.global().getPropertyAsFunction(runtime, "Error");
auto errorObj =
errorCtor
.callAsConstructor(
runtime,
jsi::String::createFromUtf8(runtime, error.getMessage()))
.getObject(runtime);
errorObj.setProperty(
runtime,
"stack",
jsi::String::createFromUtf8(runtime, error.getStack()));
jsi::JSError localError(
jsi::Value(std::move(errorObj)),
error.getMessage(),
error.getStack());
onTaskError_(runtime, localError);
} catch (...) {
jsi::JSError fallbackError(runtime, error.getMessage());

Copilot AI Mar 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fallback path reconstructs a jsi::JSError with only the message, which discards error.getStack() even though it is already available and safe. Consider using the JSError(Runtime&, message, stack) constructor in the fallback to preserve the original stack trace when Error reconstruction fails.

Suggested change
jsi::JSError fallbackError(runtime, error.getMessage());
jsi::JSError fallbackError(
runtime,
error.getMessage(),
error.getStack());

Copilot uses AI. Check for mistakes.
onTaskError_(runtime, fallbackError);
}
}

void RuntimeScheduler_Legacy::executeTask(
jsi::Runtime& runtime,
const std::shared_ptr<Task>& task,
Expand Down
2 changes: 2 additions & 0 deletions packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Legacy.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,8 @@ class RuntimeScheduler_Legacy final : public RuntimeSchedulerBase {

void executeTask(jsi::Runtime &runtime, const std::shared_ptr<Task> &task, bool didUserCallbackTimeout);

void reportError(jsi::Runtime &runtime, jsi::JSError &error) const;

/*
* Returns a time point representing the current point in time. May be called
* from multiple threads.
Expand Down
31 changes: 29 additions & 2 deletions ...ages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -380,13 +380,40 @@ void RuntimeScheduler_Modern::executeTask(
task.callback = result.getObject(runtime).getFunction(runtime);
}
} catch (jsi::JSError& error) {
onTaskError_(runtime, error);
reportError(runtime, error);
} catch (std::exception& ex) {
jsi::JSError error(runtime, std::string("Non-js exception: ") + ex.what());
onTaskError_(runtime, error);
}
}

void RuntimeScheduler_Modern::reportError(
jsi::Runtime& runtime,
jsi::JSError& error) const {
try {
auto errorCtor =
runtime.global().getPropertyAsFunction(runtime, "Error");
auto errorObj =
errorCtor
.callAsConstructor(
runtime,
jsi::String::createFromUtf8(runtime, error.getMessage()))
.getObject(runtime);
errorObj.setProperty(
runtime,
"stack",
jsi::String::createFromUtf8(runtime, error.getStack()));
jsi::JSError localError(
jsi::Value(std::move(errorObj)),
error.getMessage(),
error.getStack());
onTaskError_(runtime, localError);
} catch (...) {
jsi::JSError fallbackError(runtime, error.getMessage());

Copilot AI Mar 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the fallback path you construct jsi::JSError with only error.getMessage(), which drops the original stack trace even though error.getStack() is available as a safe C++ string. Consider using the JSError(Runtime&, message, stack) constructor here so LogBox still gets a stack when reconstruction fails.

Suggested change
jsi::JSError fallbackError(runtime, error.getMessage());
jsi::JSError fallbackError(
runtime,
error.getMessage(),
error.getStack());

Copilot uses AI. Check for mistakes.
onTaskError_(runtime, fallbackError);
}
}

/**
* This is partially equivalent to the "Perform a microtask checkpoint" step in
* the Web event loop. See
Expand Down Expand Up @@ -419,7 +446,7 @@ void RuntimeScheduler_Modern::performMicrotaskCheckpoint(
break;
}
} catch (jsi::JSError& error) {
onTaskError_(runtime, error);
reportError(runtime, error);
} catch (std::exception& ex) {
jsi::JSError error(
runtime, std::string("Non-js exception: ") + ex.what());
Expand Down
10 changes: 10 additions & 0 deletions packages/react-native/ReactCommon/react/renderer/runtimescheduler/RuntimeScheduler_Modern.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,16 @@ class RuntimeScheduler_Modern final : public RuntimeSchedulerBase {

void executeTask(jsi::Runtime &runtime, Task &task, bool didUserCallbackTimeout) const;

/*
* Re-creates a jsi::JSError as a proper Error instance in the given runtime
* and forwards it to onTaskError_. This is necessary because the caught
* error's jsi::Value may belong to a different jsi::Runtime (e.g. when the
* error originates from a background Hermes instance). Using a cross-runtime
* Value is undefined behavior and causes LogBox to show "Unknown".
* getMessage() and getStack() are plain C++ strings and always safe to use.
*/
void reportError(jsi::Runtime &runtime, jsi::JSError &error) const;

void updateRendering(HighResTimeStamp taskEndTime);

bool performingMicrotaskCheckpoint_{false};
Expand Down
49 changes: 49 additions & 0 deletions ...s/react-native/ReactCommon/react/renderer/runtimescheduler/tests/RuntimeSchedulerTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,55 @@ TEST_P(RuntimeSchedulerTest, errorInTaskShouldNotStopMicrotasks) {
EXPECT_EQ(stubErrorUtils_->getReportFatalCallCount(), 1);
}

TEST_P(RuntimeSchedulerTest, handlingCrossRuntimeError) {
auto secondRuntime = facebook::hermes::makeHermesRuntime();

bool didRunTask = false;
auto callback = createHostFunctionFromLambda(
[&didRunTask, &secondRuntime](bool /*unused*/) {
didRunTask = true;
throw jsi::JSError(*secondRuntime, "Cross-runtime error");
return jsi::Value::undefined();
});

runtimeScheduler_->scheduleTask(
SchedulerPriority::NormalPriority, std::move(callback));

EXPECT_FALSE(didRunTask);
EXPECT_EQ(stubQueue_->size(), 1);

stubQueue_->tick();

EXPECT_TRUE(didRunTask);
EXPECT_EQ(stubQueue_->size(), 0);
EXPECT_EQ(stubErrorUtils_->getReportFatalCallCount(), 1);
EXPECT_EQ(stubErrorUtils_->getLastReportedMessage(), "Cross-runtime error");
}

TEST_P(RuntimeSchedulerTest, handlingErrorPreservesMessage) {
bool didRunTask = false;
auto callback =
createHostFunctionFromLambda([this, &didRunTask](bool /*unused*/) {
didRunTask = true;
throw jsi::JSError(*runtime_, "Same-runtime error");
return jsi::Value::undefined();
});

runtimeScheduler_->scheduleTask(
SchedulerPriority::NormalPriority, std::move(callback));

EXPECT_FALSE(didRunTask);
EXPECT_EQ(stubQueue_->size(), 1);

stubQueue_->tick();

EXPECT_TRUE(didRunTask);
EXPECT_EQ(stubQueue_->size(), 0);
EXPECT_EQ(stubErrorUtils_->getReportFatalCallCount(), 1);
EXPECT_EQ(
stubErrorUtils_->getLastReportedMessage(), "Same-runtime error");
}

TEST_P(RuntimeSchedulerTest, reportsLongTasks) {
// Only for event loop
if (!GetParam()) {
Expand Down
15 changes: 14 additions & 1 deletion packages/react-native/ReactCommon/react/renderer/runtimescheduler/tests/StubErrorUtils.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,15 @@ class StubErrorUtils : public jsi::HostObject {
name,
1,
[this](
jsi::Runtime &runtime, const jsi::Value &, const jsi::Value *arguments, size_t) noexcept -> jsi::Value {
jsi::Runtime &runtime, const jsi::Value &, const jsi::Value *arguments, size_t count) noexcept -> jsi::Value {
reportFatalCallCount_++;
if (count > 0 && arguments[0].isObject()) {
auto obj = arguments[0].getObject(runtime);
auto msgVal = obj.getProperty(runtime, "message");
if (msgVal.isString()) {
lastReportedMessage_ = msgVal.getString(runtime).utf8(runtime);
Comment on lines 48 to +55

Copilot AI Mar 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The host function lambda for reportFatalError is declared noexcept, but the newly added JSI operations (getObject, getProperty, getString) can throw jsi::JSIException. If any of those throw, the process will std::terminate(). Remove noexcept here or wrap the property access in a try/catch to keep the stub safe.

Copilot uses AI. Check for mistakes.
}
}
Comment on lines 50 to +57

Copilot AI Mar 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lastReportedMessage_ is only updated when a valid message string is found; otherwise it retains its previous value. Consider clearing it at the start of each reportFatalError call (or explicitly setting it to an empty string when parsing fails) to avoid stale assertions if multiple errors are reported.

Copilot uses AI. Check for mistakes.
return jsi::Value::undefined();
});
}
Expand All @@ -60,8 +67,14 @@ class StubErrorUtils : public jsi::HostObject {
return reportFatalCallCount_;
}

const std::string &getLastReportedMessage() const
{
return lastReportedMessage_;
}

private:
int reportFatalCallCount_;
std::string lastReportedMessage_;
Comment on lines 76 to +77

Copilot AI Mar 8, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

StubErrorUtils leaves reportFatalCallCount_ uninitialized. Because the object is created with the implicit default constructor, the first reportFatalCallCount_++ is undefined behavior and can make the tests flaky. Initialize this member (and lastReportedMessage_) with in-class initializers or an explicit constructor.

Suggested change
int reportFatalCallCount_;
std::string lastReportedMessage_;
int reportFatalCallCount_{0};
std::string lastReportedMessage_{};

Copilot uses AI. Check for mistakes.
};

} // namespace facebook::react
Loading