Skip to content

Use createError in JSError and forward it in RuntimeDecorator #58961

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
lavenzg wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Use createError in JSError and forward it in RuntimeDecorator #58961

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
51 changes: 51 additions & 0 deletions packages/react-native/ReactCommon/jsi/jsi/decorator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,28 @@ class RuntimeDecorator : public Base, private jsi::Instrumentation {
return *this;
}

Value createError(const String& msg) override {
return plain().createError(msg);
}
Value createEvalError(const String& msg) override {
return plain().createEvalError(msg);
}
Value createRangeError(const String& msg) override {
return plain().createRangeError(msg);
}
Value createReferenceError(const String& msg) override {
return plain().createReferenceError(msg);
}
Value createSyntaxError(const String& msg) override {
return plain().createSyntaxError(msg);
}
Value createTypeError(const String& msg) override {
return plain().createTypeError(msg);
}
Value createURIError(const String& msg) override {
return plain().createURIError(msg);
}

protected:
// plain is generally going to be a reference to an object managed
// by a derived class. We cache it here so this class can be
Expand Down Expand Up @@ -706,6 +728,35 @@ class WithRuntimeDecorator : public RuntimeDecorator<Plain, Base> {
return RD::isInspectable();
}

Value createError(const String& msg) override {
Around around{with_};
return RD::createError(msg);
}
Value createEvalError(const String& msg) override {
Around around{with_};
return RD::createEvalError(msg);
}
Value createRangeError(const String& msg) override {
Around around{with_};
return RD::createRangeError(msg);
}
Value createReferenceError(const String& msg) override {
Around around{with_};
return RD::createReferenceError(msg);
}
Value createSyntaxError(const String& msg) override {
Around around{with_};
return RD::createSyntaxError(msg);
}
Value createTypeError(const String& msg) override {
Around around{with_};
return RD::createTypeError(msg);
}
Value createURIError(const String& msg) override {
Around around{with_};
return RD::createURIError(msg);
}

// The jsi:: prefix is necessary because MSVC compiler complains C2247:
// Instrumentation is not accessible because RuntimeDecorator uses private
// to inherit from Instrumentation.
Expand Down
4 changes: 1 addition & 3 deletions packages/react-native/ReactCommon/jsi/jsi/jsi.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -967,9 +967,7 @@ JSError::JSError(IRuntime& rt, Value&& value) {

JSError::JSError(IRuntime& rt, std::string msg) : message_(std::move(msg)) {
try {
setValue(
rt,
callGlobalFunction(rt, "Error", String::createFromUtf8(rt, message_)));
setValue(rt, rt.createError(String::createFromUtf8(rt, message_)));
} catch (const JSIException& ex) {
message_ = std::string(ex.what()) + " (while raising " + message_ + ")";
setValue(rt, String::createFromUtf8(rt, message_));
Expand Down
179 changes: 171 additions & 8 deletions packages/react-native/ReactCommon/jsi/jsi/test/testlib.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1209,10 +1209,24 @@ TEST_P(JSITest, JSErrorsCanBeConstructedWithStack) {
}

TEST_P(JSITest, JSErrorDoesNotInfinitelyRecurse) {
Value globalError = rt.global().getProperty(rt, "Error");
rt.global().setProperty(rt, "Error", Value::undefined());
// JSError(rt, message) creates its value with createError. Use the default
// implementation, which calls the global Error constructor, so that removing
// it below makes creating the Error fail even if the runtime has a native
// createError.
class RD : public RuntimeDecorator<Runtime, Runtime> {
public:
explicit RD(Runtime& rt) : RuntimeDecorator(rt) {}

Value createError(const String& msg) override {
return Runtime::createError(msg);
}
};
RD drt(rt);

Value globalError = drt.global().getProperty(drt, "Error");
drt.global().setProperty(drt, "Error", Value::undefined());
try {
rt.global().getPropertyAsFunction(rt, "NotAFunction");
drt.global().getPropertyAsFunction(drt, "NotAFunction");
FAIL() << "expected exception";
} catch (const JSError& ex) {
EXPECT_EQ(
Expand All @@ -1235,14 +1249,14 @@ TEST_P(JSITest, JSErrorDoesNotInfinitelyRecurse) {
"function (f) { try { f(); return 'undefined'; }"
"catch (e) { return typeof e; } }")
.call(
rt,
drt,
Function::createFromHostFunction(
rt, PropNameID::forAscii(rt, "fails"), 0, fails))
.getString(rt)
.utf8(rt),
drt, PropNameID::forAscii(drt, "fails"), 0, fails))
.getString(drt)
.utf8(drt),
"string");

rt.global().setProperty(rt, "Error", globalError);
drt.global().setProperty(drt, "Error", globalError);
}

TEST_P(JSITest, JSErrorStackOverflowHandling) {
Expand Down Expand Up @@ -1655,6 +1669,155 @@ TEST_P(JSITest, CreateErrorTest) {
}
}

TEST_P(JSITest, JSErrorMessageTest) {
// JSError(rt, message) creates a new Error instance.
{
JSError e(rt, "native error");
EXPECT_EQ(e.getMessage(), "native error");
Object obj = e.value().getObject(rt);
EXPECT_TRUE(
rt.instanceOf(obj, rt.global().getPropertyAsFunction(rt, "Error")));
EXPECT_EQ(
obj.getProperty(rt, "message").getString(rt).utf8(rt), "native error");
}

// JSError(rt, message) creates its value with Runtime::createError, so a
// replaced global Error constructor is invoked exactly when createError
// invokes it (e.g. the default jsi::Runtime implementation does, runtimes
// with a native implementation do not).
eval(
"var OriginalError = Error;"
"var replacedErrorCalls = 0;"
"Error = function(message) {"
" ++replacedErrorCalls;"
" return new OriginalError(message);"
"};");
auto replacedErrorCalls = [&] {
return rt.global().getProperty(rt, "replacedErrorCalls").getNumber();
};

rt.createError(String::createFromAscii(rt, "probe"));
double callsPerCreateError = replacedErrorCalls();

JSError e(rt, "replaced");
EXPECT_EQ(replacedErrorCalls(), 2 * callsPerCreateError);
EXPECT_EQ(e.getMessage(), "replaced");
Object obj = e.value().getObject(rt);
EXPECT_TRUE(rt.instanceOf(
obj, rt.global().getPropertyAsFunction(rt, "OriginalError")));
EXPECT_EQ(obj.getProperty(rt, "message").getString(rt).utf8(rt), "replaced");
}

TEST_P(JSITest, DecoratorCreateErrorTest) {
// Counts the createError* calls that reach it, and forwards them to the
// underlying runtime.
class CountingRD : public RuntimeDecorator<Runtime, Runtime> {
public:
explicit CountingRD(Runtime& rt) : RuntimeDecorator(rt) {}

Value createError(const String& msg) override {
++count;
return RuntimeDecorator::createError(msg);
}
Value createEvalError(const String& msg) override {
++count;
return RuntimeDecorator::createEvalError(msg);
}
Value createRangeError(const String& msg) override {
++count;
return RuntimeDecorator::createRangeError(msg);
}
Value createReferenceError(const String& msg) override {
++count;
return RuntimeDecorator::createReferenceError(msg);
}
Value createSyntaxError(const String& msg) override {
++count;
return RuntimeDecorator::createSyntaxError(msg);
}
Value createTypeError(const String& msg) override {
++count;
return RuntimeDecorator::createTypeError(msg);
}
Value createURIError(const String& msg) override {
++count;
return RuntimeDecorator::createURIError(msg);
}

int count = 0;
};

class RD : public RuntimeDecorator<Runtime, Runtime> {
public:
explicit RD(Runtime& rt) : RuntimeDecorator(rt) {}
};

struct Count {
void before() {
++count;
}

int count = 0;
};

class CountRuntime : public WithRuntimeDecorator<Count> {
public:
explicit CountRuntime(Runtime& rt)
: WithRuntimeDecorator<Count>(rt, count_) {}

Count count_;
};

CountingRD counting(rt);
RD rd(counting);
CountRuntime crt(counting);

struct {
Value (Runtime::*create)(const String&);
const char* ctor;
} cases[] = {
{&Runtime::createError, "Error"},
{&Runtime::createEvalError, "EvalError"},
{&Runtime::createRangeError, "RangeError"},
{&Runtime::createReferenceError, "ReferenceError"},
{&Runtime::createSyntaxError, "SyntaxError"},
{&Runtime::createTypeError, "TypeError"},
{&Runtime::createURIError, "URIError"},
};

// Both decorators must forward createError* to the decorated runtime rather
// than fall back to the default implementation calling the global ctor.
for (const auto& c : cases) {
for (Runtime* drt :
{static_cast<Runtime*>(&rd), static_cast<Runtime*>(&crt)}) {
String msg = String::createFromAscii(*drt, "decorated");
int countBefore = counting.count;
int aroundBefore = crt.count_.count;
Value error = (drt->*c.create)(msg);
EXPECT_EQ(counting.count, countBefore + 1) << c.ctor;
if (drt == &crt) {
EXPECT_EQ(crt.count_.count, aroundBefore + 1) << c.ctor;
}

Object obj = error.getObject(*drt);
EXPECT_TRUE(drt->instanceOf(
obj, drt->global().getPropertyAsFunction(*drt, c.ctor)))
<< c.ctor;
EXPECT_EQ(
obj.getProperty(*drt, "message").getString(*drt).utf8(*drt),
"decorated")
<< c.ctor;
}
}

// JSError(rt, message) on a decorated runtime reaches the decorated
// runtime's createError.
int countBefore = counting.count;
JSError e(rd, "decorated JSError");
EXPECT_EQ(counting.count, countBefore + 1);
EXPECT_EQ(e.getMessage(), "decorated JSError");
}

TEST_P(JSITest, MicrotasksTest) {
try {
rt.global().setProperty(rt, "globalValue", String::createFromAscii(rt, ""));
Expand Down
14 changes: 14 additions & 0 deletions scripts/cxx-api/api-snapshots/ReactAndroidDebugCxx.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6264,6 +6264,13 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
public virtual facebook::jsi::Instrumentation& instrumentation() override;
public virtual facebook::jsi::Object global() override;
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;
Expand Down Expand Up @@ -6370,6 +6377,13 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
public virtual facebook::jsi::Instrumentation& instrumentation() override;
public virtual facebook::jsi::Object global() override;
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;
Expand Down
14 changes: 14 additions & 0 deletions scripts/cxx-api/api-snapshots/ReactAndroidFrameworksCxx.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10880,6 +10880,13 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
public virtual facebook::jsi::Instrumentation& instrumentation() override;
public virtual facebook::jsi::Object global() override;
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;
Expand Down Expand Up @@ -10986,6 +10993,13 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
public virtual facebook::jsi::Instrumentation& instrumentation() override;
public virtual facebook::jsi::Object global() override;
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;
Expand Down
14 changes: 14 additions & 0 deletions scripts/cxx-api/api-snapshots/ReactAndroidNewarchCxx.api
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6113,6 +6113,13 @@ class facebook::jsi::RuntimeDecorator : public facebook::jsi::Runtime {
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
public virtual facebook::jsi::Instrumentation& instrumentation() override;
public virtual facebook::jsi::Object global() override;
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;
Expand Down Expand Up @@ -6219,6 +6226,13 @@ class facebook::jsi::WithRuntimeDecorator : public facebook::jsi::RuntimeDecorat
public virtual facebook::jsi::ICast* castInterface(const facebook::jsi::UUID& interfaceUUID) override;
public virtual facebook::jsi::Instrumentation& instrumentation() override;
public virtual facebook::jsi::Object global() override;
public virtual facebook::jsi::Value createError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createEvalError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createRangeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createReferenceError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createSyntaxError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createTypeError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value createURIError(const facebook::jsi::String& msg) override;
public virtual facebook::jsi::Value evaluateJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, const std::string& sourceURL) override;
public virtual facebook::jsi::Value evaluatePreparedJavaScript(const std::shared_ptr<const facebook::jsi::PreparedJavaScript>& js) override;
public virtual std::shared_ptr<const facebook::jsi::PreparedJavaScript> prepareJavaScript(const std::shared_ptr<const facebook::jsi::Buffer>& buffer, std::string sourceURL) override;
Expand Down
Loading
Loading