Skip to content

Commit 1d1ca7e

Browse files
mrousavyfacebook-github-bot
mrousavy
authored and
facebook-github-bot
committed
Use createError in JSError and forward it in RuntimeDecorator
Summary: `JSError(rt, message)` still creates its value by looking up `globalThis.Error` and calling it, which predates `IRuntime::createError`. It now uses `rt.createError(...)` like the `JSError::createXError` factories, with the same try/catch fallback. The default `Runtime::createError` makes the same global call, so runtimes without a native implementation behave as before. On Hermes every `JSError` thrown from native code skips a global lookup and a JS call, and `message`, `stack` and `what()` come out the same. `RuntimeDecorator` and `WithRuntimeDecorator` didn't forward the `createError` family, so decorated runtimes (the thread-safe runtime, `TracingRuntime`) quietly fell back to the global constructor calls. They forward to `plain()` now, through `Around` for `WithRuntimeDecorator`, and they're public like in `jsi::Runtime`. One visible difference on Hermes, same as the other `createXError` APIs: if user code replaced or deleted `globalThis.Error`, `JSError(rt, message)` no longer calls the replacement and creates a real `Error` anyway. X-link: facebook/hermes#2203 Differential Revision: D124178378 Pulled By: lavenzg
1 parent cf1fc01 commit 1d1ca7e

15 files changed

Lines changed: 391 additions & 11 deletions

‎packages/react-native/ReactCommon/jsi/jsi/decorator.h‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,28 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
149149
return *this;
150150
}
151151

152+
Value createError(const String& msg) override {
153+
return plain().createError(msg);
154+
}
155+
Value createEvalError(const String& msg) override {
156+
return plain().createEvalError(msg);
157+
}
158+
Value createRangeError(const String& msg) override {
159+
return plain().createRangeError(msg);
160+
}
161+
Value createReferenceError(const String& msg) override {
162+
return plain().createReferenceError(msg);
163+
}
164+
Value createSyntaxError(const String& msg) override {
165+
return plain().createSyntaxError(msg);
166+
}
167+
Value createTypeError(const String& msg) override {
168+
return plain().createTypeError(msg);
169+
}
170+
Value createURIError(const String& msg) override {
171+
return plain().createURIError(msg);
172+
}
173+
152174
protected:
153175
// plain is generally going to be a reference to an object managed
154176
// by a derived class. We cache it here so this class can be
@@ -706,6 +728,35 @@ class WithRuntimeDecorator : public RuntimeDecorator<Plain, Base> {
706728
return RD::isInspectable();
707729
}
708730

731+
Value createError(const String& msg) override {
732+
Around around{with_};
733+
return RD::createError(msg);
734+
}
735+
Value createEvalError(const String& msg) override {
736+
Around around{with_};
737+
return RD::createEvalError(msg);
738+
}
739+
Value createRangeError(const String& msg) override {
740+
Around around{with_};
741+
return RD::createRangeError(msg);
742+
}
743+
Value createReferenceError(const String& msg) override {
744+
Around around{with_};
745+
return RD::createReferenceError(msg);
746+
}
747+
Value createSyntaxError(const String& msg) override {
748+
Around around{with_};
749+
return RD::createSyntaxError(msg);
750+
}
751+
Value createTypeError(const String& msg) override {
752+
Around around{with_};
753+
return RD::createTypeError(msg);
754+
}
755+
Value createURIError(const String& msg) override {
756+
Around around{with_};
757+
return RD::createURIError(msg);
758+
}
759+
709760
// The jsi:: prefix is necessary because MSVC compiler complains C2247:
710761
// Instrumentation is not accessible because RuntimeDecorator uses private
711762
// to inherit from Instrumentation.

‎packages/react-native/ReactCommon/jsi/jsi/jsi.cpp‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -967,9 +967,7 @@ JSError::JSError(IRuntime& rt, Value&& value) {
967967

968968
JSError::JSError(IRuntime& rt, std::string msg) : message_(std::move(msg)) {
969969
try {
970-
setValue(
971-
rt,
972-
callGlobalFunction(rt, "Error", String::createFromUtf8(rt, message_)));
970+
setValue(rt, rt.createError(String::createFromUtf8(rt, message_)));
973971
} catch (const JSIException& ex) {
974972
message_ = std::string(ex.what()) + " (while raising " + message_ + ")";
975973
setValue(rt, String::createFromUtf8(rt, message_));

‎packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp‎

Lines changed: 171 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1209,10 +1209,24 @@ TEST_P(JSITest, JSErrorsCanBeConstructedWithStack) {
12091209
}
12101210

12111211
TEST_P(JSITest, JSErrorDoesNotInfinitelyRecurse) {
1212-
Value globalError = rt.global().getProperty(rt, "Error");
1213-
rt.global().setProperty(rt, "Error", Value::undefined());
1212+
// JSError(rt, message) creates its value with createError. Use the default
1213+
// implementation, which calls the global Error constructor, so that removing
1214+
// it below makes creating the Error fail even if the runtime has a native
1215+
// createError.
1216+
class RD : public RuntimeDecorator<Runtime, Runtime> {
1217+
public:
1218+
explicit RD(Runtime& rt) : RuntimeDecorator(rt) {}
1219+
1220+
Value createError(const String& msg) override {
1221+
return Runtime::createError(msg);
1222+
}
1223+
};
1224+
RD drt(rt);
1225+
1226+
Value globalError = drt.global().getProperty(drt, "Error");
1227+
drt.global().setProperty(drt, "Error", Value::undefined());
12141228
try {
1215-
rt.global().getPropertyAsFunction(rt, "NotAFunction");
1229+
drt.global().getPropertyAsFunction(drt, "NotAFunction");
12161230
FAIL() << "expected exception";
12171231
} catch (const JSError& ex) {
12181232
EXPECT_EQ(
@@ -1235,14 +1249,14 @@ TEST_P(JSITest, JSErrorDoesNotInfinitelyRecurse) {
12351249
"function (f) { try { f(); return 'undefined'; }"
12361250
"catch (e) { return typeof e; } }")
12371251
.call(
1238-
rt,
1252+
drt,
12391253
Function::createFromHostFunction(
1240-
rt, PropNameID::forAscii(rt, "fails"), 0, fails))
1241-
.getString(rt)
1242-
.utf8(rt),
1254+
drt, PropNameID::forAscii(drt, "fails"), 0, fails))
1255+
.getString(drt)
1256+
.utf8(drt),
12431257
"string");
12441258

1245-
rt.global().setProperty(rt, "Error", globalError);
1259+
drt.global().setProperty(drt, "Error", globalError);
12461260
}
12471261

12481262
TEST_P(JSITest, JSErrorStackOverflowHandling) {
@@ -1655,6 +1669,155 @@ TEST_P(JSITest, CreateErrorTest) {
16551669
}
16561670
}
16571671

1672+
TEST_P(JSITest, JSErrorMessageTest) {
1673+
// JSError(rt, message) creates a new Error instance.
1674+
{
1675+
JSError e(rt, "native error");
1676+
EXPECT_EQ(e.getMessage(), "native error");
1677+
Object obj = e.value().getObject(rt);
1678+
EXPECT_TRUE(
1679+
rt.instanceOf(obj, rt.global().getPropertyAsFunction(rt, "Error")));
1680+
EXPECT_EQ(
1681+
obj.getProperty(rt, "message").getString(rt).utf8(rt), "native error");
1682+
}
1683+
1684+
// JSError(rt, message) creates its value with Runtime::createError, so a
1685+
// replaced global Error constructor is invoked exactly when createError
1686+
// invokes it (e.g. the default jsi::Runtime implementation does, runtimes
1687+
// with a native implementation do not).
1688+
eval(
1689+
"var OriginalError = Error;"
1690+
"var replacedErrorCalls = 0;"
1691+
"Error = function(message) {"
1692+
" ++replacedErrorCalls;"
1693+
" return new OriginalError(message);"
1694+
"};");
1695+
auto replacedErrorCalls = [&] {
1696+
return rt.global().getProperty(rt, "replacedErrorCalls").getNumber();
1697+
};
1698+
1699+
rt.createError(String::createFromAscii(rt, "probe"));
1700+
double callsPerCreateError = replacedErrorCalls();
1701+
1702+
JSError e(rt, "replaced");
1703+
EXPECT_EQ(replacedErrorCalls(), 2 * callsPerCreateError);
1704+
EXPECT_EQ(e.getMessage(), "replaced");
1705+
Object obj = e.value().getObject(rt);
1706+
EXPECT_TRUE(rt.instanceOf(
1707+
obj, rt.global().getPropertyAsFunction(rt, "OriginalError")));
1708+
EXPECT_EQ(obj.getProperty(rt, "message").getString(rt).utf8(rt), "replaced");
1709+
}
1710+
1711+
TEST_P(JSITest, DecoratorCreateErrorTest) {
1712+
// Counts the createError* calls that reach it, and forwards them to the
1713+
// underlying runtime.
1714+
class CountingRD : public RuntimeDecorator<Runtime, Runtime> {
1715+
public:
1716+
explicit CountingRD(Runtime& rt) : RuntimeDecorator(rt) {}
1717+
1718+
Value createError(const String& msg) override {
1719+
++count;
1720+
return RuntimeDecorator::createError(msg);
1721+
}
1722+
Value createEvalError(const String& msg) override {
1723+
++count;
1724+
return RuntimeDecorator::createEvalError(msg);
1725+
}
1726+
Value createRangeError(const String& msg) override {
1727+
++count;
1728+
return RuntimeDecorator::createRangeError(msg);
1729+
}
1730+
Value createReferenceError(const String& msg) override {
1731+
++count;
1732+
return RuntimeDecorator::createReferenceError(msg);
1733+
}
1734+
Value createSyntaxError(const String& msg) override {
1735+
++count;
1736+
return RuntimeDecorator::createSyntaxError(msg);
1737+
}
1738+
Value createTypeError(const String& msg) override {
1739+
++count;
1740+
return RuntimeDecorator::createTypeError(msg);
1741+
}
1742+
Value createURIError(const String& msg) override {
1743+
++count;
1744+
return RuntimeDecorator::createURIError(msg);
1745+
}
1746+
1747+
int count = 0;
1748+
};
1749+
1750+
class RD : public RuntimeDecorator<Runtime, Runtime> {
1751+
public:
1752+
explicit RD(Runtime& rt) : RuntimeDecorator(rt) {}
1753+
};
1754+
1755+
struct Count {
1756+
void before() {
1757+
++count;
1758+
}
1759+
1760+
int count = 0;
1761+
};
1762+
1763+
class CountRuntime : public WithRuntimeDecorator<Count> {
1764+
public:
1765+
explicit CountRuntime(Runtime& rt)
1766+
: WithRuntimeDecorator<Count>(rt, count_) {}
1767+
1768+
Count count_;
1769+
};
1770+
1771+
CountingRD counting(rt);
1772+
RD rd(counting);
1773+
CountRuntime crt(counting);
1774+
1775+
struct {
1776+
Value (Runtime::*create)(const String&);
1777+
const char* ctor;
1778+
} cases[] = {
1779+
{&Runtime::createError, "Error"},
1780+
{&Runtime::createEvalError, "EvalError"},
1781+
{&Runtime::createRangeError, "RangeError"},
1782+
{&Runtime::createReferenceError, "ReferenceError"},
1783+
{&Runtime::createSyntaxError, "SyntaxError"},
1784+
{&Runtime::createTypeError, "TypeError"},
1785+
{&Runtime::createURIError, "URIError"},
1786+
};
1787+
1788+
// Both decorators must forward createError* to the decorated runtime rather
1789+
// than fall back to the default implementation calling the global ctor.
1790+
for (const auto& c : cases) {
1791+
for (Runtime* drt :
1792+
{static_cast<Runtime*>(&rd), static_cast<Runtime*>(&crt)}) {
1793+
String msg = String::createFromAscii(*drt, "decorated");
1794+
int countBefore = counting.count;
1795+
int aroundBefore = crt.count_.count;
1796+
Value error = (drt->*c.create)(msg);
1797+
EXPECT_EQ(counting.count, countBefore + 1) << c.ctor;
1798+
if (drt == &crt) {
1799+
EXPECT_EQ(crt.count_.count, aroundBefore + 1) << c.ctor;
1800+
}
1801+
1802+
Object obj = error.getObject(*drt);
1803+
EXPECT_TRUE(drt->instanceOf(
1804+
obj, drt->global().getPropertyAsFunction(*drt, c.ctor)))
1805+
<< c.ctor;
1806+
EXPECT_EQ(
1807+
obj.getProperty(*drt, "message").getString(*drt).utf8(*drt),
1808+
"decorated")
1809+
<< c.ctor;
1810+
}
1811+
}
1812+
1813+
// JSError(rt, message) on a decorated runtime reaches the decorated
1814+
// runtime's createError.
1815+
int countBefore = counting.count;
1816+
JSError e(rd, "decorated JSError");
1817+
EXPECT_EQ(counting.count, countBefore + 1);
1818+
EXPECT_EQ(e.getMessage(), "decorated JSError");
1819+
}
1820+
16581821
TEST_P(JSITest, MicrotasksTest) {
16591822
try {
16601823
rt.global().setProperty(rt, "globalValue", String::createFromAscii(rt, ""));

‎scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6264,6 +6264,13 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
62646264
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
62656265
public virtual facebook::jsi::Instrumentation& instrumentation() override;
62666266
public virtual facebook::jsi::Object global() override;
6267+
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
6268+
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
6269+
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
6270+
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
6271+
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
6272+
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
6273+
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
62676274
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
62686275
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
62696276
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;
@@ -6370,6 +6377,13 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat
63706377
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
63716378
public virtual facebook::jsi::Instrumentation& instrumentation() override;
63726379
public virtual facebook::jsi::Object global() override;
6380+
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
6381+
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
6382+
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
6383+
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
6384+
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
6385+
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
6386+
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
63736387
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
63746388
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
63756389
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;

‎scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10880,6 +10880,13 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
1088010880
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
1088110881
public virtual facebook::jsi::Instrumentation& instrumentation() override;
1088210882
public virtual facebook::jsi::Object global() override;
10883+
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
10884+
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
10885+
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
10886+
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
10887+
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
10888+
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
10889+
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
1088310890
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
1088410891
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
1088510892
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;
@@ -10986,6 +10993,13 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat
1098610993
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
1098710994
public virtual facebook::jsi::Instrumentation& instrumentation() override;
1098810995
public virtual facebook::jsi::Object global() override;
10996+
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
10997+
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
10998+
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
10999+
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
11000+
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
11001+
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
11002+
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
1098911003
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
1099011004
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
1099111005
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;

‎scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6113,6 +6113,13 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
61136113
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
61146114
public virtual facebook::jsi::Instrumentation& instrumentation() override;
61156115
public virtual facebook::jsi::Object global() override;
6116+
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
6117+
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
6118+
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
6119+
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
6120+
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
6121+
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
6122+
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
61166123
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
61176124
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
61186125
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;
@@ -6219,6 +6226,13 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat
62196226
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
62206227
public virtual facebook::jsi::Instrumentation& instrumentation() override;
62216228
public virtual facebook::jsi::Object global() override;
6229+
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
6230+
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
6231+
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
6232+
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
6233+
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
6234+
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
6235+
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
62226236
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
62236237
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
62246238
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;

0 commit comments

Comments
 (0)