From 6ac2596293cc04636f447240527c76ed1a3c3716 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Mon, 6 Mar 2023 20:13:16 +0100 Subject: [PATCH] Capture exceptions that happen in signals in ScriptEngine, add test --- .../script-engine/src/v8/ScriptEngineV8.h | 2 ++ .../src/v8/ScriptObjectV8Proxy.cpp | 20 ++++++------ tests/script-engine/src/ScriptEngineTests.cpp | 31 +++++++++++++++++++ tests/script-engine/src/ScriptEngineTests.h | 1 + 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.h b/libraries/script-engine/src/v8/ScriptEngineV8.h index b701eddc36..0a3ef444fb 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.h +++ b/libraries/script-engine/src/v8/ScriptEngineV8.h @@ -225,6 +225,8 @@ protected: void setUncaughtException(const v8::TryCatch &tryCatch, const QString& info = QString()); void setUncaughtException(std::shared_ptr exception); + friend class ScriptSignalV8Proxy; + std::shared_ptr _uncaughtException; diff --git a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp index bed092bbd5..4af680c024 100644 --- a/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp +++ b/libraries/script-engine/src/v8/ScriptObjectV8Proxy.cpp @@ -96,7 +96,7 @@ V8ScriptValue ScriptObjectV8Proxy::newQObject(ScriptEngineV8* engine, QObject* o //if (proxy) return engine->newObject(proxy.get(), engine->newVariant(QVariant::fromValue(proxy)));; } } - + bool ownsObject; switch (ownership) { case ScriptEngine::QtOwnership: @@ -243,12 +243,12 @@ void ScriptObjectV8Proxy::investigate() { v8::Isolate::Scope isolateScope(isolate); v8::HandleScope handleScope(_engine->getIsolate()); v8::Context::Scope contextScope(_engine->getContext()); - + //auto objectTemplate = _v8ObjectTemplate.Get(_engine->getIsolate()); auto objectTemplate = v8::ObjectTemplate::New(_engine->getIsolate()); objectTemplate->SetInternalFieldCount(3); objectTemplate->SetHandler(v8::NamedPropertyHandlerConfiguration(v8Get, v8Set, nullptr, nullptr, v8GetPropertyNames)); - + const QMetaObject* metaObject = qobject->metaObject(); //qDebug(scriptengine) << "Investigate: " << metaObject->className(); @@ -286,7 +286,7 @@ void ScriptObjectV8Proxy::investigate() { for (int idx = startIdx; idx < num; ++idx) { QMetaMethod method = metaObject->method(idx); //qDebug(scriptengine) << "Investigate: " << metaObject->className() << " Method: " << method.name(); - + // perhaps keep this comment? Calls (like AudioScriptingInterface::playSound) seem to expect non-public methods to be script-accessible /* if (method.access() != QMetaMethod::Public) continue;*/ @@ -1011,12 +1011,12 @@ void ScriptMethodV8Proxy::call(const v8::FunctionCallbackInfo& argume isolate->ThrowError("Referencing deleted native object"); return; } - + //v8::HandleScope handleScope(_engine->getIsolate()); int scriptNumArgs = arguments.Length(); int numArgs = std::min(scriptNumArgs, _numMaxParams); - + const int scriptValueTypeId = qMetaTypeId(); int parameterConversionFailureId = 0; @@ -1202,7 +1202,7 @@ void ScriptMethodV8Proxy::call(const v8::FunctionCallbackInfo& argume int scriptNumArgs = context->argumentCount(); int numArgs = std::min(scriptNumArgs, _numMaxParams); - + const int scriptValueTypeId = qMetaTypeId(); int parameterConversionFailureId = 0; @@ -1474,6 +1474,8 @@ int ScriptSignalV8Proxy::qt_metacall(QMetaObject::Call call, int id, void** argu qCDebug(scriptengine) << "Signal proxy " << fullName() << " connection call failed: \"" << _engine->formatErrorMessageFromTryCatch(tryCatch) << "\nThis provided: " << conn.thisValue.get()->IsObject(); + + _engine->setUncaughtException(tryCatch, "Error in signal proxy"); } //_engine->popContext(); } @@ -1524,7 +1526,7 @@ void ScriptSignalV8Proxy::connect(ScriptValue arg0, ScriptValue arg1) { isolate->ThrowError("Referencing deleted native object"); return; } - + //v8::HandleScope handleScope(isolate); // untangle the arguments @@ -1630,7 +1632,7 @@ void ScriptSignalV8Proxy::connect(ScriptValue arg0, ScriptValue arg1) { Connection newConnection(callbackThis, callback); //newConn.callback = callback; //newConn.thisValue = callbackThis; - + withWriteLock([&]{ _connections.append(newConnection); }); diff --git a/tests/script-engine/src/ScriptEngineTests.cpp b/tests/script-engine/src/ScriptEngineTests.cpp index b0004128f0..0066115772 100644 --- a/tests/script-engine/src/ScriptEngineTests.cpp +++ b/tests/script-engine/src/ScriptEngineTests.cpp @@ -271,6 +271,37 @@ void ScriptEngineTests::testSignal() { QVERIFY(printed.length() >= 10); } +void ScriptEngineTests::testSignalWithException() { + QString script = + "var count = 0;" + "Script.update.connect(function(deltaTime) {" + " count++;" + " print(deltaTime);" + " if (count >= 3) {" + " Script.stop(true);" + " }" + " nonExist();" + "});"; + + QStringList printed; + int exceptionCount = 0; + + auto sm = makeManager(script, "testSignalWithException.js"); + + connect(sm.get(), &ScriptManager::printedMessage, [&printed](const QString& message, const QString& engineName){ + printed.append(message); + }); + + connect(sm.get(), &ScriptManager::unhandledException, [&exceptionCount](std::shared_ptr exception){ + exceptionCount++; + }); + + + sm->run(); + QVERIFY(printed.length() >= 3); + QVERIFY(exceptionCount >= 3); +} + void ScriptEngineTests::scriptTest() { return; diff --git a/tests/script-engine/src/ScriptEngineTests.h b/tests/script-engine/src/ScriptEngineTests.h index a0ba970f8a..6fabf5f0cd 100644 --- a/tests/script-engine/src/ScriptEngineTests.h +++ b/tests/script-engine/src/ScriptEngineTests.h @@ -64,6 +64,7 @@ private slots: void testRaiseException(); void testRaiseExceptionAndCatch(); void testSignal(); + void testSignalWithException();