diff --git a/ReactCommon/jsi/jsi/jsi.cpp b/ReactCommon/jsi/jsi/jsi.cpp index 065589a4214ae6..e7b6ac94ed020a 100644 --- a/ReactCommon/jsi/jsi/jsi.cpp +++ b/ReactCommon/jsi/jsi/jsi.cpp @@ -37,6 +37,27 @@ std::string kindToString(const Value& v, Runtime* rt = nullptr) { } } +// getPropertyAsFunction() will try to create a JSError. If the +// failure is in building a JSError, this will lead to infinite +// recursion. This function is used in place of getPropertyAsFunction +// when building JSError, to avoid that infinite recursion. +Value callGlobalFunction(Runtime& runtime, const char* name, const Value& arg) { + Value v = runtime.global().getProperty(runtime, name); + if (!v.isObject()) { + throw JSINativeException( + std::string("callGlobalFunction: JS global property '") + name + + "' is " + kindToString(v, &runtime) + ", expected a Function"); + } + Object o = v.getObject(runtime); + if (!o.isFunction(runtime)) { + throw JSINativeException( + std::string("callGlobalFunction: JS global property '") + name + + "' is a non-callable Object, expected a Function"); + } + Function f = std::move(o).getFunction(runtime); + return f.call(runtime, arg); +} + } // namespace namespace detail { @@ -142,9 +163,7 @@ Function Object::getPropertyAsFunction(Runtime& runtime, const char* name) kindToString(std::move(obj), &runtime) + ", expected a Function"); }; - Runtime::PointerValue* value = obj.ptr_; - obj.ptr_ = nullptr; - return Function(value); + return std::move(obj).getFunction(runtime); } Array Object::asArray(Runtime& runtime) const& { @@ -347,7 +366,11 @@ JSError::JSError(Runtime& rt, Value&& value) { JSError::JSError(Runtime& rt, std::string msg) : message_(std::move(msg)) { try { setValue( - rt, rt.global().getPropertyAsFunction(rt, "Error").call(rt, message_)); + rt, + callGlobalFunction(rt, "Error", String::createFromUtf8(rt, message_))); + } catch (const std::exception& ex) { + message_ = std::string(ex.what()) + " (while raising " + message_ + ")"; + setValue(rt, String::createFromUtf8(rt, message_)); } catch (...) { setValue(rt, Value()); } @@ -360,6 +383,8 @@ JSError::JSError(Runtime& rt, std::string msg, std::string stack) e.setProperty(rt, "message", String::createFromUtf8(rt, message_)); e.setProperty(rt, "stack", String::createFromUtf8(rt, stack_)); setValue(rt, std::move(e)); + } catch (const std::exception& ex) { + setValue(rt, String::createFromUtf8(rt, ex.what())); } catch (...) { setValue(rt, Value()); } @@ -380,20 +405,23 @@ void JSError::setValue(Runtime& rt, Value&& value) { if (message_.empty()) { jsi::Value message = obj.getProperty(rt, "message"); if (!message.isUndefined()) { - message_ = message.toString(rt).utf8(rt); + message_ = + callGlobalFunction(rt, "String", message).getString(rt).utf8(rt); } } if (stack_.empty()) { jsi::Value stack = obj.getProperty(rt, "stack"); if (!stack.isUndefined()) { - stack_ = stack.toString(rt).utf8(rt); + stack_ = + callGlobalFunction(rt, "String", stack).getString(rt).utf8(rt); } } } if (message_.empty()) { - message_ = value_->toString(rt).utf8(rt); + message_ = + callGlobalFunction(rt, "String", *value_).getString(rt).utf8(rt); } if (stack_.empty()) { @@ -403,6 +431,13 @@ void JSError::setValue(Runtime& rt, Value&& value) { if (what_.empty()) { what_ = message_ + "\n\n" + stack_; } + } catch (const std::exception& ex) { + message_ = std::string("[Exception while creating message string: ") + + ex.what() + "]"; + stack_ = std::string("Exception while creating stack string: ") + + ex.what() + "]"; + what_ = + std::string("Exception while getting value fields: ") + ex.what() + "]"; } catch (...) { message_ = "[Exception caught creating message string]"; stack_ = "[Exception caught creating stack string]"; diff --git a/ReactCommon/jsi/jsi/test/testlib.cpp b/ReactCommon/jsi/jsi/test/testlib.cpp index a99b4e97a206e7..7db87ba4c17186 100644 --- a/ReactCommon/jsi/jsi/test/testlib.cpp +++ b/ReactCommon/jsi/jsi/test/testlib.cpp @@ -939,7 +939,7 @@ unsigned countOccurences(const std::string& of, const std::string& in) { } // namespace TEST_P(JSITest, JSErrorsArePropagatedNicely) { - unsigned callsBeoreError = 5; + unsigned callsBeforeError = 5; Function sometimesThrows = function( "function sometimesThrows(shouldThrow, callback) {" @@ -953,9 +953,9 @@ TEST_P(JSITest, JSErrorsArePropagatedNicely) { rt, PropNameID::forAscii(rt, "callback"), 0, - [&sometimesThrows, &callsBeoreError]( + [&sometimesThrows, &callsBeforeError]( Runtime& rt, const Value& thisVal, const Value* args, size_t count) { - return sometimesThrows.call(rt, --callsBeoreError == 0, args[0]); + return sometimesThrows.call(rt, --callsBeforeError == 0, args[0]); }); try { @@ -975,6 +975,55 @@ TEST_P(JSITest, JSErrorsCanBeConstructedWithStack) { EXPECT_EQ(err.getStack(), "stack"); } +TEST_P(JSITest, JSErrorDoesNotInfinitelyRecurse) { + Value globalString = rt.global().getProperty(rt, "String"); + rt.global().setProperty(rt, "String", Value::undefined()); + try { + eval("throw Error('whoops')"); + FAIL() << "expected exception"; + } catch (const JSError& ex) { + EXPECT_EQ( + ex.getMessage(), + "[Exception while creating message string: callGlobalFunction: " + "JS global property 'String' is undefined, expected a Function]"); + } + rt.global().setProperty(rt, "String", globalString); + + Value globalError = rt.global().getProperty(rt, "Error"); + rt.global().setProperty(rt, "Error", Value::undefined()); + try { + rt.global().getPropertyAsFunction(rt, "NotAFunction"); + FAIL() << "expected exception"; + } catch (const JSError& ex) { + EXPECT_EQ( + ex.getMessage(), + "callGlobalFunction: JS global property 'Error' is undefined, " + "expected a Function (while raising getPropertyAsObject: " + "property 'NotAFunction' is undefined, expected an Object)"); + } + + // If Error is missing, this is fundamentally a problem with JS code + // messing up the global object, so it should present in JS code as + // a catchable string. Not an Error (because that's broken), or as + // a C++ failure. + + auto fails = [](Runtime& rt, const Value&, const Value*, size_t) -> Value { + return rt.global().getPropertyAsObject(rt, "NotAProperty"); + }; + EXPECT_EQ( + function("function (f) { try { f(); return 'undefined'; }" + "catch (e) { return typeof e; } }") + .call( + rt, + Function::createFromHostFunction( + rt, PropNameID::forAscii(rt, "fails"), 0, fails)) + .getString(rt) + .utf8(rt), + "string"); + + rt.global().setProperty(rt, "Error", globalError); +} + TEST_P(JSITest, ScopeDoesNotCrashTest) { Scope scope(rt); Object o(rt);