From acd19f7c40eebc6edc86ab3503414598b3c8ccfd Mon Sep 17 00:00:00 2001
From: Dale Glass <dale@daleglass.net>
Date: Sun, 5 Mar 2023 21:54:14 +0100
Subject: [PATCH] Review fix: add flag to choose whether to abort script on
exception
---
libraries/script-engine/src/ScriptManager.cpp | 9 +++++----
libraries/script-engine/src/ScriptManager.h | 19 +++++++++++++++++++
tests/script-engine/src/ScriptEngineTests.cpp | 10 ++++++++--
tests/script-engine/src/ScriptEngineTests.h | 2 ++
4 files changed, 34 insertions(+), 6 deletions(-)
diff --git a/libraries/script-engine/src/ScriptManager.cpp b/libraries/script-engine/src/ScriptManager.cpp
index 2629e31c1b..26d3e01d1b 100644
--- a/libraries/script-engine/src/ScriptManager.cpp
+++ b/libraries/script-engine/src/ScriptManager.cpp
@@ -305,9 +305,10 @@ ScriptManager::ScriptManager(Context context, const QString& scriptContents, con
}
// Unhandled exception kills the running script
- stop(false);
-
- logException(output);
+ if (_abortOnUncaughtException) {
+ stop(false);
+ logException(output);
+ }
});
#endif
@@ -909,7 +910,7 @@ void ScriptManager::run() {
PROFILE_RANGE(script, _fileNameString);
_returnValue = _engine->evaluate(_scriptContents, _fileNameString);
- if (_engine->hasUncaughtException()) {
+ if (_engine->hasUncaughtException() && _abortOnUncaughtException) {
qCWarning(scriptengine) << "Engine has uncaught exception, stopping";
stop();
diff --git a/libraries/script-engine/src/ScriptManager.h b/libraries/script-engine/src/ScriptManager.h
index 9491b1e47b..968ec599cb 100644
--- a/libraries/script-engine/src/ScriptManager.h
+++ b/libraries/script-engine/src/ScriptManager.h
@@ -1180,6 +1180,23 @@ public:
*/
std::shared_ptr<ScriptException> getUncaughtException() const;
+ /**
+ * @brief Whether this engine will abort on an uncaught exception
+ *
+ * @warning This probably should be refactored into a more comprehensive per-script flags system
+ * @return true
+ * @return false
+ */
+ bool getAbortOnUncaughtException() const { return _abortOnUncaughtException; }
+
+ /**
+ * @brief Whether to abort on an uncaught exception
+ *
+ * @warning This probably should be refactored into a more comprehensive per-script flags system
+ * @param value
+ */
+ void setAbortOnUncaughtException(bool value) { _abortOnUncaughtException = value; }
+
public slots:
/**
@@ -1555,6 +1572,8 @@ protected:
ScriptManagerScriptingInterfacePointer _scriptingInterface;
+ bool _abortOnUncaughtException{ false };
+
friend ScriptManagerPointer newScriptManager(Context context, const QString& scriptContents, const QString& fileNameString);
friend class ScriptManagerScriptingInterface;
diff --git a/tests/script-engine/src/ScriptEngineTests.cpp b/tests/script-engine/src/ScriptEngineTests.cpp
index 825f6f4658..c2a344c603 100644
--- a/tests/script-engine/src/ScriptEngineTests.cpp
+++ b/tests/script-engine/src/ScriptEngineTests.cpp
@@ -57,6 +57,8 @@ ScriptManagerPointer ScriptEngineTests::makeManager(const QString &scriptSource,
ScriptManagerPointer sm = newScriptManager(ScriptManager::NETWORKLESS_TEST_SCRIPT, scriptSource, scriptFilename);
+ sm->setAbortOnUncaughtException(true);
+
connect(sm.get(), &ScriptManager::scriptLoaded, [](const QString& filename){
qWarning() << "Loaded script" << filename;
});
@@ -227,8 +229,7 @@ void ScriptEngineTests::testRaiseExceptionAndCatch() {
" print(\"Caught!\");"
" }"
"}"
- "Script.stop(true);"
- ;
+ "Script.stop(true);";
QString printed;
auto sm = makeManager(script, "testRaiseCatch.js");
@@ -248,6 +249,11 @@ void ScriptEngineTests::testRaiseExceptionAndCatch() {
}
+void ScriptEngineTests::testSignal() {
+
+}
+
+
void ScriptEngineTests::scriptTest() {
return;
diff --git a/tests/script-engine/src/ScriptEngineTests.h b/tests/script-engine/src/ScriptEngineTests.h
index b02a6846fa..a0ba970f8a 100644
--- a/tests/script-engine/src/ScriptEngineTests.h
+++ b/tests/script-engine/src/ScriptEngineTests.h
@@ -63,6 +63,8 @@ private slots:
void testInvokeNonInvokable();
void testRaiseException();
void testRaiseExceptionAndCatch();
+ void testSignal();
+
private: