From c0e62c5cc2a008d12216210cf6d1ddf4a4cb5856 Mon Sep 17 00:00:00 2001 From: Dale Glass Date: Sat, 4 Mar 2023 18:32:00 +0100 Subject: [PATCH] Initial exception handling Make exception handling use the new ScriptException class. Add exception signal Throw exceptions in script evaluation Remove references to ScriptManager from ScriptEngine --- libraries/entities/src/EntityEditFilters.cpp | 55 ++++----- .../src/graphics-scripting/ScriptableMesh.cpp | 14 ++- libraries/script-engine/src/ScriptEngine.h | 84 +++++++++++-- libraries/script-engine/src/ScriptException.h | 80 ++++++++++++ libraries/script-engine/src/ScriptManager.cpp | 16 ++- libraries/script-engine/src/ScriptManager.h | 1 - .../script-engine/src/v8/ScriptEngineV8.cpp | 114 +++++++++++------- .../script-engine/src/v8/ScriptEngineV8.h | 25 ++-- 8 files changed, 297 insertions(+), 92 deletions(-) create mode 100644 libraries/script-engine/src/ScriptException.h diff --git a/libraries/entities/src/EntityEditFilters.cpp b/libraries/entities/src/EntityEditFilters.cpp index 42b1e5c580..3f31a6739a 100644 --- a/libraries/entities/src/EntityEditFilters.cpp +++ b/libraries/entities/src/EntityEditFilters.cpp @@ -30,7 +30,7 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { for (auto id : zoneIDs) { if (!id.isInvalidID()) { // for now, look it up in the tree (soon we need to cache or similar?) - EntityItemPointer itemPtr = _tree->findEntityByEntityItemID(id); + EntityItemPointer itemPtr = _tree->findEntityByEntityItemID(id); auto zone = std::dynamic_pointer_cast(itemPtr); if (!zone) { // TODO: maybe remove later? @@ -39,7 +39,7 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { zones.append(id); } } else { - // the null id is the global filter we put in the domain server's + // the null id is the global filter we put in the domain server's // advanced entity server settings zones.append(id); } @@ -49,7 +49,7 @@ QList EntityEditFilters::getZonesByPosition(glm::vec3& position) { bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& propertiesIn, EntityItemProperties& propertiesOut, bool& wasChanged, EntityTree::FilterType filterType, EntityItemID& itemID, const EntityItemPointer& existingEntity) { - + // get the ids of all the zones (plus the global entity edit filter) that the position // lies within auto zoneIDs = getZonesByPosition(position); @@ -57,12 +57,12 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper if (!itemID.isInvalidID() && id == itemID) { continue; } - - // get the filter pair, etc... + + // get the filter pair, etc... _lock.lockForRead(); FilterData filterData = _filterDataMap.value(id); _lock.unlock(); - + if (filterData.valid()) { if (filterData.rejectAll) { return false; @@ -159,13 +159,13 @@ bool EntityEditFilters::filter(glm::vec3& position, EntityItemProperties& proper // otherwise, assume it wants to pass all properties propertiesOut = propertiesIn; wasChanged = false; - + } else { return false; } } } - // if we made it here, + // if we made it here, return true; } @@ -177,23 +177,23 @@ void EntityEditFilters::removeFilter(EntityItemID entityID) { void EntityEditFilters::addFilter(EntityItemID entityID, QString filterURL) { QUrl scriptURL(filterURL); - - // setting it to an empty string is same as removing + + // setting it to an empty string is same as removing if (filterURL.size() == 0) { removeFilter(entityID); return; } - + // The following should be abstracted out for use in Agent.cpp (and maybe later AvatarMixer.cpp) if (scriptURL.scheme().isEmpty() || (scriptURL.scheme() == HIFI_URL_SCHEME_FILE)) { qWarning() << "Cannot load script from local filesystem, because assignment may be on a different computer."; scriptRequestFinished(entityID); return; } - + // first remove any existing info for this entity removeFilter(entityID); - + // reject all edits until we load the script FilterData filterData; filterData.rejectAll = true; @@ -201,7 +201,7 @@ void EntityEditFilters::addFilter(EntityItemID entityID, QString filterURL) { _lock.lockForWrite(); _filterDataMap.insert(entityID, filterData); _lock.unlock(); - + auto scriptRequest = DependencyManager::get()->createResourceRequest( this, scriptURL, true, -1, "EntityEditFilters::addFilter"); if (!scriptRequest) { @@ -232,18 +232,19 @@ static bool hasCorrectSyntax(const ScriptProgramPointer& program) { } static bool hadUncaughtExceptions(ScriptEngine& engine, const QString& fileName) { if (engine.hasUncaughtException()) { - const auto backtrace = engine.uncaughtExceptionBacktrace(); - const auto exception = engine.uncaughtException().toString(); - const auto line = QString::number(engine.uncaughtExceptionLineNumber()); + //const auto backtrace = engine.uncaughtException().backtrace; + //const auto exception = engine.uncaughtException().toString(); + //const auto line = QString::number(engine.uncaughtExceptionLineNumber()); + qCritical() << engine.uncaughtException(); engine.clearExceptions(); - static const QString SCRIPT_EXCEPTION_FORMAT = "[UncaughtException] %1 in %2:%3"; - auto message = QString(SCRIPT_EXCEPTION_FORMAT).arg(exception, fileName, line); - if (!backtrace.empty()) { - static const auto lineSeparator = "\n "; - message += QString("\n[Backtrace]%1%2").arg(lineSeparator, backtrace.join(lineSeparator)); - } - qCritical() << qPrintable(message); + //static const QString SCRIPT_EXCEPTION_FORMAT = "[UncaughtException] %1 in %2:%3"; + //auto message = QString(SCRIPT_EXCEPTION_FORMAT).arg(exception, fileName, line); + //if (!backtrace.empty()) { + // static const auto lineSeparator = "\n "; + // message += QString("\n[Backtrace]%1%2").arg(lineSeparator, backtrace.join(lineSeparator)); + //} + //qCritical() << qPrintable(message); return true; } return false; @@ -273,7 +274,7 @@ void EntityEditFilters::scriptRequestFinished(EntityItemID entityID) { FilterData filterData; filterData.engine = engine; filterData.rejectAll = false; - + // define the uncaughtException function ScriptEngine& engineRef = *engine; filterData.uncaughtExceptions = [&engineRef, urlString]() { return hadUncaughtExceptions(engineRef, urlString); }; @@ -377,11 +378,11 @@ void EntityEditFilters::scriptRequestFinished(EntityItemID entityID) { _lock.unlock(); qDebug() << "script request filter processed for entity id " << entityID; - + emit filterAdded(entityID, true); return; } - } + } } else if (scriptRequest) { const QString urlString = scriptRequest->getUrl().toString(); qCritical() << "Failed to download script"; diff --git a/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp b/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp index 33c6773e12..0387b66da4 100644 --- a/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp +++ b/libraries/graphics-scripting/src/graphics-scripting/ScriptableMesh.cpp @@ -285,7 +285,12 @@ glm::uint32 scriptable::ScriptableMesh::forEachVertex(const ScriptValue& _callba buffer_helpers::mesh::forEachVertex(mesh, [&](glm::uint32 index, const QVariantMap& values) { auto result = callback.call(scope, { js->toScriptValue(values), js->newValue(index), meshPart }); if (js->hasUncaughtException()) { - js->currentContext()->throwValue(js->uncaughtException()); + // TODO: I think the idea here is that toScriptValue or toValue might throw an exception + // in the script engine, which we're then propagating to the running script. This got broken + // by the scripting engine changes, needs fixing. + + qCCritical(graphics_scripting) << "Uncaught exception, handling broken, FIX ME: " << js->uncaughtException(); + //js->currentContext()->throwValue(js->uncaughtException()); return false; } numProcessed++; @@ -295,13 +300,13 @@ glm::uint32 scriptable::ScriptableMesh::forEachVertex(const ScriptValue& _callba } /*@jsdoc - * Called for each vertex when {@link GraphicsMesh.updateVertexAttributes} is called. The value returned by the script function + * Called for each vertex when {@link GraphicsMesh.updateVertexAttributes} is called. The value returned by the script function * should be the modified attributes to update the vertex with, or false to not update the particular vertex. * @callback GraphicsMesh~updateVertexAttributesCallback * @param {Object} attributes - The attributes of the vertex. * @param {number} index - The vertex index. * @param {object} properties - The properties of the mesh, per {@link GraphicsMesh}. - * @returns {Object|boolean} The attribute values to update the vertex with, or + * @returns {Object|boolean} The attribute values to update the vertex with, or * false to not update the vertex. */ glm::uint32 scriptable::ScriptableMesh::updateVertexAttributes(const ScriptValue& _callback) { @@ -325,7 +330,8 @@ glm::uint32 scriptable::ScriptableMesh::updateVertexAttributes(const ScriptValue auto obj = js->toScriptValue(values); auto result = callback.call(scope, { obj, js->newValue(index), meshPart }); if (js->hasUncaughtException()) { - js->currentContext()->throwValue(js->uncaughtException()); + qCCritical(graphics_scripting) << "Uncaught exception, handling broken, FIX ME: " << js->uncaughtException(); + //js->currentContext()->throwValue(js->uncaughtException()); return false; } if (result.isBool() && !result.toBool()) { diff --git a/libraries/script-engine/src/ScriptEngine.h b/libraries/script-engine/src/ScriptEngine.h index 1994da74a5..5e60c79238 100644 --- a/libraries/script-engine/src/ScriptEngine.h +++ b/libraries/script-engine/src/ScriptEngine.h @@ -24,6 +24,7 @@ #include #include "ScriptValue.h" +#include "ScriptException.h" class QByteArray; class QLatin1String; @@ -55,9 +56,28 @@ inline T scriptvalue_cast(const ScriptValue& value); * provide the full environment needed to execute scripts. * * To execute scripts that have access to the API, use ScriptManager. + * + * Exception handling + * ================== + * + * Exceptions are handled in two directions: exceptions thrown by the code executing in the scripting + * engine, but not captured by the running code are caught by this object and can be inspected. + * + * If an exception in the running code occurs, then the exception() signal is emitted. Also, + * hasUncaughtException() returns true, and uncaughException() returns the ScriptException with the + * details. Both the signal and uncaughtException() return the same information, and either can + * be used depending on what best fits the program. + * + * To inject an exception into the running script, use raiseException(). This may result in the script + * not capturing it and an uncaughtException happening as a result. */ class ScriptEngine { public: + + ScriptEngine(ScriptManager *manager = nullptr) : _manager(manager) { + + } + typedef ScriptValue (*FunctionSignature)(ScriptContext*, ScriptEngine*); typedef ScriptValue (*MarshalFunction)(ScriptEngine*, const void*); typedef bool (*DemarshalFunction)(const ScriptValue&, QVariant &dest); @@ -80,7 +100,9 @@ public: ScriptOwnership = 1, /** - * @brief Ownership is determined automatically + * @brief Ownership is determined automatically. + * If the object has a parent, it's deemed QtOwnership. + * If the object has no parent, it's deemed ScriptOwnership. * */ AutoOwnership = 2, @@ -266,7 +288,20 @@ public: * * @return ScriptManager* ScriptManager */ - virtual ScriptManager* manager() const = 0; + ScriptManager* manager() const { return _manager; } + + /** + * @brief Emit the current uncaught exception if there is one + * + * If there's an uncaught exception, emit it, and clear the exception status. + * + * This fails if there's no uncaught exception, there's no ScriptManager, + * or the engine is evaluating. + * + * @param debugHint A debugging hint to be added to the error message + * @return true There was an uncaught exception, and it was emitted + * @return false There was no uncaught exception + */ virtual bool maybeEmitUncaughtException(const QString& debugHint = QString()) = 0; virtual ScriptValue newArray(uint length = 0) = 0; virtual ScriptValue newArrayBuffer(const QByteArray& message) = 0; @@ -291,9 +326,9 @@ public: /** * @brief Causes an exception to be raised in the currently executing script * - * @param exception - * @return true - * @return false + * @param exception Exception to be thrown in the script + * @return true Exception was successfully thrown + * @return false Exception couldn't be thrown because no script is running */ virtual bool raiseException(const ScriptValue& exception) = 0; virtual void registerEnum(const QString& enumName, QMetaEnum newEnum) = 0; @@ -309,14 +344,36 @@ public: virtual void setThread(QThread* thread) = 0; //Q_INVOKABLE virtual void enterIsolateOnThisThread() = 0; virtual ScriptValue undefinedValue() = 0; - virtual ScriptValue uncaughtException() const = 0; - virtual QStringList uncaughtExceptionBacktrace() const = 0; - virtual int uncaughtExceptionLineNumber() const = 0; + + /** + * @brief Last uncaught exception, if any + * + * @return ScriptValue Uncaught exception from the script + */ + virtual ScriptException uncaughtException() const = 0; + virtual void updateMemoryCost(const qint64& deltaSize) = 0; virtual void requestCollectGarbage() = 0; + + /** + * @brief Test the underlying scripting engine + * + * This compiles, executes and verifies the execution of a trivial test program + * to make sure the underlying scripting engine actually works. + * + * @deprecated Test function, not part of the API, can be removed + */ virtual void compileTest() = 0; virtual QString scriptValueDebugDetails(const ScriptValue &value) = 0; virtual QString scriptValueDebugListMembers(const ScriptValue &value) = 0; + + /** + * @brief Log the current backtrace + * + * Logs the current backtrace for debugging + * + * @param title Informative title for the backtrace + */ virtual void logBacktrace(const QString &title) = 0; public: // helper to detect and log warnings when other code invokes QScriptEngine/BaseScriptEngine in thread-unsafe ways @@ -339,8 +396,19 @@ public: // not for public use, but I don't like how Qt strings this along with p virtual void registerCustomType(int type, MarshalFunction mf, DemarshalFunction df) = 0; virtual QStringList getCurrentScriptURLs() const = 0; + +signals: + /** + * @brief The script being run threw an exception + * + * @param exception Exception that was thrown + */ + void exception(const ScriptException &exception); + protected: ~ScriptEngine() {} // prevent explicit deletion of base class + + ScriptManager *_manager; }; Q_DECLARE_OPERATORS_FOR_FLAGS(ScriptEngine::QObjectWrapOptions); diff --git a/libraries/script-engine/src/ScriptException.h b/libraries/script-engine/src/ScriptException.h new file mode 100644 index 0000000000..174099bb4d --- /dev/null +++ b/libraries/script-engine/src/ScriptException.h @@ -0,0 +1,80 @@ +// +// ScriptException.h +// libraries/script-engine/src +// +// Created by Dale Glass on 27/02/2023. +// Copyright 2023 Overte e.V. +// +// Distributed under the Apache License, Version 2.0. +// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html + +#include +#include +#include + +#pragma once + +/** + * @brief Scripting exception + * + * Emitted from the scripting engine when an exception happens inside it. + * + */ +class ScriptException { + public: + + ScriptException(QString message = "", QString info = "", int line = 0, int column = 0, QStringList backtraceList = QStringList()) : + errorMessage(message), additionalInfo(info), errorLine(line), errorColumn(column), backtrace(backtraceList) { + + } + + /** + * @brief Error message + * + */ + QString errorMessage; + + /** + * @brief Additional information about the exception + * + * This is additional information added at the place where the exception happened. + * It may contain information about what the system was doing when the exception happened. + * + */ + QString additionalInfo; + + + /** + * @brief Error line + * + */ + int errorLine; + + /** + * @brief Error column + * + */ + int errorColumn; + + /** + * @brief Backtrace + * + */ + QStringList backtrace; + + bool isEmpty() const { return errorMessage.isEmpty(); } +}; + +inline QDebug operator<<(QDebug debug, const ScriptException& e) { + debug << "Exception:" + << e.errorMessage + << (e.additionalInfo.isEmpty() ? QString("") : "[" + e.additionalInfo + "]") + << " at line " << e.errorLine << ", column " << e.errorColumn; + + if (e.backtrace.length()) { + debug << "Backtrace:"; + debug << e.backtrace; + } + + return debug; +} \ No newline at end of file diff --git a/libraries/script-engine/src/ScriptManager.cpp b/libraries/script-engine/src/ScriptManager.cpp index 7833c683a3..46c3cacdf2 100644 --- a/libraries/script-engine/src/ScriptManager.cpp +++ b/libraries/script-engine/src/ScriptManager.cpp @@ -284,10 +284,16 @@ ScriptManager::ScriptManager(Context context, const QString& scriptContents, con // this is where all unhandled exceptions end up getting logged connect(this, &ScriptManager::unhandledException, this, [this](const ScriptValue& err) { + qCCritical(scriptengine) << "Caught exception"; + auto output = err.engine() == _engine ? err : _engine->makeError(err); if (!output.property("detail").isValid()) { output.setProperty("detail", "UnhandledException"); } + + // Unhandled exception kills the running script + stop(false); + logException(output); }); @@ -886,6 +892,14 @@ void ScriptManager::run() { PROFILE_RANGE(script, _fileNameString); _returnValue = _engine->evaluate(_scriptContents, _fileNameString); _engine->maybeEmitUncaughtException(__FUNCTION__); + + if (_engine->hasUncaughtException()) { + + qCWarning(scriptengine) << "Engine has uncaught exception, stopping"; + stop(); + emit unhandledException(_engine->cloneUncaughtException(__FUNCTION__)); + _engine->clearExceptions(); + } } #ifdef _WIN32 // VS13 does not sleep_until unless it uses the system_clock, see: @@ -1008,7 +1022,7 @@ void ScriptManager::run() { // only clear exceptions if we are not in the middle of evaluating if (!_engine->isEvaluating() && _engine->hasUncaughtException()) { qCWarning(scriptengine) << __FUNCTION__ << "---------- UNCAUGHT EXCEPTION --------"; - qCWarning(scriptengine) << "runInThread" << _engine->uncaughtException().toString(); + qCWarning(scriptengine) << "runInThread" << _engine->uncaughtException(); emit unhandledException(_engine->cloneUncaughtException(__FUNCTION__)); _engine->clearExceptions(); } diff --git a/libraries/script-engine/src/ScriptManager.h b/libraries/script-engine/src/ScriptManager.h index cae0a88f75..60cdc0a670 100644 --- a/libraries/script-engine/src/ScriptManager.h +++ b/libraries/script-engine/src/ScriptManager.h @@ -266,7 +266,6 @@ public: * qInfo() << "Done!" * @endcode * - * * @note * Technically, the ScriptManager isn't generic enough -- it implements things that imitate * Node.js for examine in the module loading code, which makes it JS specific. This code diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.cpp b/libraries/script-engine/src/v8/ScriptEngineV8.cpp index 6288c2c8f2..d9cdf09104 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.cpp +++ b/libraries/script-engine/src/v8/ScriptEngineV8.cpp @@ -291,14 +291,18 @@ bool ScriptEngineV8::raiseException(const V8ScriptValue& exception) { } bool ScriptEngineV8::maybeEmitUncaughtException(const QString& debugHint) { + + /* if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) { return false; } - if (!isEvaluating() && hasUncaughtException() && _scriptManager) { - emit _scriptManager->unhandledException(cloneUncaughtException(debugHint)); + if (!isEvaluating() && hasUncaughtException()) { + emit exception(cloneUncaughtException(debugHint)); clearExceptions(); return true; } + + */ return false; } @@ -388,8 +392,7 @@ v8::Platform* ScriptEngineV8::getV8Platform() { return platform.get(); } -ScriptEngineV8::ScriptEngineV8(ScriptManager* scriptManager) : - _scriptManager(scriptManager), _evaluatingCounter(0) +ScriptEngineV8::ScriptEngineV8(ScriptManager *manager) : ScriptEngine(manager), _evaluatingCounter(0) //V8TODO //_arrayBufferClass(new ArrayBufferClass(this)) { @@ -451,7 +454,7 @@ ScriptEngineV8::ScriptEngineV8(ScriptManager* scriptManager) : //_currentThread = QThread::currentThread(); - if (_scriptManager) { + //if (_scriptManager) { // V8TODO: port to V8 /*connect(this, &QScriptEngine::signalHandlerException, this, [this](const V8ScriptValue& exception) { if (hasUncaughtException()) { @@ -466,7 +469,7 @@ ScriptEngineV8::ScriptEngineV8(ScriptManager* scriptManager) : }, Qt::DirectConnection);*/ //moveToThread(scriptManager->thread()); //setThread(scriptManager->thread()); - } + //} } void ScriptEngineV8::registerEnum(const QString& enumName, QMetaEnum newEnum) { @@ -1044,10 +1047,6 @@ ScriptValue ScriptEngineV8::evaluateInClosure(const ScriptValue& _closure, } ScriptValue ScriptEngineV8::evaluate(const QString& sourceCode, const QString& fileName) { - if (_scriptManager && _scriptManager->isStopped()) { - return undefinedValue(); // bail early - } - //V8TODO if (QThread::currentThread() != thread()) { @@ -1132,8 +1131,12 @@ ScriptValue ScriptEngineV8::evaluate(const QString& sourceCode, const QString& f ScriptValue errorValue(new ScriptValueV8Wrapper(this, V8ScriptValue(this, runError->Get()))); qCDebug(scriptengine) << "Running script: \"" << fileName << "\" " << formatErrorMessageFromTryCatch(tryCatchRun); //V8TODO + + //raiseException(errorValue); //maybeEmitUncaughtException("evaluate"); + setUncaughtException(tryCatchRun, "script evaluation"); + _evaluatingCounter--; return errorValue; } @@ -1142,6 +1145,53 @@ ScriptValue ScriptEngineV8::evaluate(const QString& sourceCode, const QString& f return ScriptValue(new ScriptValueV8Wrapper(this, std::move(resultValue))); } +void ScriptEngineV8::setUncaughtException(const v8::TryCatch &tryCatch, const QString& info) { + if (!tryCatch.HasCaught()) { + qCWarning(scriptengine) << "setUncaughtException called without exception"; + clearExceptions(); + return; + } + + ScriptException ex; + ex.additionalInfo = info; + + v8::Locker locker(_v8Isolate); + v8::Isolate::Scope isolateScope(_v8Isolate); + v8::HandleScope handleScope(_v8Isolate); + v8::Context::Scope contextScope(getContext()); + QString result(""); + + QString errorMessage = ""; + QString errorBacktrace = ""; + //v8::String::Utf8Value utf8Value(getIsolate(), tryCatch.Exception()); + v8::String::Utf8Value utf8Value(getIsolate(), tryCatch.Message()->Get()); + + ex.errorMessage = QString(*utf8Value); + + v8::Local exceptionMessage = tryCatch.Message(); + if (!exceptionMessage.IsEmpty()) { + ex.errorLine = exceptionMessage->GetLineNumber(getContext()).FromJust(); + ex.errorColumn = exceptionMessage->GetStartColumn(getContext()).FromJust(); + v8::Local backtraceV8String; + if (tryCatch.StackTrace(getContext()).ToLocal(&backtraceV8String)) { + if (backtraceV8String->IsString()) { + if (v8::Local::Cast(backtraceV8String)->Length() > 0) { + v8::String::Utf8Value backtraceUtf8Value(getIsolate(), backtraceV8String); + QString errorBacktrace = *backtraceUtf8Value; + ex.backtrace = errorBacktrace.split("\n"); + + } + } + } + } + + qCDebug(scriptengine) << "Emitting exception:" << ex; + _uncaughtException = ex; + emit exception(ex); +} + + + QString ScriptEngineV8::formatErrorMessageFromTryCatch(v8::TryCatch &tryCatch) { v8::Locker locker(_v8Isolate); v8::Isolate::Scope isolateScope(_v8Isolate); @@ -1194,9 +1244,6 @@ void ScriptEngineV8::popContext() { } Q_INVOKABLE ScriptValue ScriptEngineV8::evaluate(const ScriptProgramPointer& program) { - if (_scriptManager && _scriptManager->isStopped()) { - return undefinedValue(); // bail early - } if (QThread::currentThread() != thread()) { ScriptValue result; @@ -1308,10 +1355,6 @@ ScriptValue ScriptEngineV8::globalObject() { return ScriptValue(new ScriptValueV8Wrapper(const_cast(this), std::move(global))); } -ScriptManager* ScriptEngineV8::manager() const { - return _scriptManager; -} - ScriptValue ScriptEngineV8::newArray(uint length) { v8::Locker locker(_v8Isolate); v8::Isolate::Scope isolateScope(_v8Isolate); @@ -1486,8 +1529,7 @@ void ScriptEngineV8::abortEvaluation() { } void ScriptEngineV8::clearExceptions() { - //V8TODO - //QScriptEngine::clearExceptions(); + _uncaughtException = ScriptException(); } ScriptContext* ScriptEngineV8::currentContext() const { @@ -1509,9 +1551,7 @@ ScriptContext* ScriptEngineV8::currentContext() const { } bool ScriptEngineV8::hasUncaughtException() const { - //V8TODO - //return QScriptEngine::hasUncaughtException(); - return false; + return !_uncaughtException.isEmpty(); } bool ScriptEngineV8::isEvaluating() const { @@ -1627,32 +1667,20 @@ void ScriptEngineV8::setThread(QThread* thread) { }*/ -ScriptValue ScriptEngineV8::uncaughtException() const { - //V8TODO - //V8ScriptValue result = QScriptEngine::uncaughtException(); - //return ScriptValue(new ScriptValueV8Wrapper(const_cast(this), std::move(result))); - return ScriptValue(); -} - -QStringList ScriptEngineV8::uncaughtExceptionBacktrace() const { - //V8TODO - //return QScriptEngine::uncaughtExceptionBacktrace(); - return QStringList(); -} - -int ScriptEngineV8::uncaughtExceptionLineNumber() const { - //V8TODO - //return QScriptEngine::uncaughtExceptionLineNumber(); - return 0; +ScriptException ScriptEngineV8::uncaughtException() const { + return _uncaughtException; } bool ScriptEngineV8::raiseException(const ScriptValue& exception) { //V8TODO //Q_ASSERT(false); - qCritical() << "Script exception occurred: " << exception.toString(); - /*ScriptValueV8Wrapper* unwrapped = ScriptValueV8Wrapper::unwrap(exception); - V8ScriptValue qException = unwrapped ? unwrapped->toV8Value() : QScriptEngine::newVariant(exception.toVariant()); - return raiseException(qException);*/ +// qCritical() << "Script exception occurred: " << exception.toString(); +// ScriptValueV8Wrapper* unwrapped = ScriptValueV8Wrapper::unwrap(exception); +// V8ScriptValue qException = unwrapped ? unwrapped->toV8Value() : QScriptEngine::newVariant(exception.toVariant()); + + // emit + //return raiseException(qException); + return false; } diff --git a/libraries/script-engine/src/v8/ScriptEngineV8.h b/libraries/script-engine/src/v8/ScriptEngineV8.h index 25bdcf4328..b02a73a552 100644 --- a/libraries/script-engine/src/v8/ScriptEngineV8.h +++ b/libraries/script-engine/src/v8/ScriptEngineV8.h @@ -35,6 +35,7 @@ #include "../ScriptEngine.h" #include "../ScriptManager.h" +#include "../ScriptException.h" //#include "V8Types.h" #include "ArrayBufferClass.h" @@ -61,7 +62,7 @@ class ScriptEngineV8 final : public QObject, public ScriptEngine, Q_OBJECT public: // construction - ScriptEngineV8(ScriptManager* scriptManager = nullptr); + ScriptEngineV8(ScriptManager *manager = nullptr); //////////////////////////////////////////////////////////////////////////////////////////////////////////////////// // NOTE - these are NOT intended to be public interfaces available to scripts, the are only Q_INVOKABLE so we can @@ -81,7 +82,7 @@ public: // ScriptEngine implementation //virtual ScriptValue lintScript(const QString& sourceCode, const QString& fileName, const int lineNumber = 1) override; virtual ScriptValue checkScriptSyntax(ScriptProgramPointer program) override; virtual ScriptValue makeError(const ScriptValue& other, const QString& type = "Error") override; - virtual ScriptManager* manager() const override; + // if there is a pending exception and we are at the top level (non-recursive) stack frame, this emits and resets it virtual bool maybeEmitUncaughtException(const QString& debugHint = QString()) override; @@ -127,9 +128,7 @@ public: // ScriptEngine implementation virtual void setThread(QThread* thread) override; //Q_INVOKABLE virtual void enterIsolateOnThisThread() override; virtual ScriptValue undefinedValue() override; - virtual ScriptValue uncaughtException() const override; - virtual QStringList uncaughtExceptionBacktrace() const override; - virtual int uncaughtExceptionLineNumber() const override; + virtual ScriptException uncaughtException() const override; virtual void updateMemoryCost(const qint64& deltaSize) override; virtual void requestCollectGarbage() override { while(!_v8Isolate->IdleNotificationDeadline(getV8Platform()->MonotonicallyIncreasingTime() + GARBAGE_COLLECTION_TIME_LIMIT_S)) {}; } virtual void compileTest() override; @@ -218,16 +217,28 @@ protected: const ValueOwnership& ownership = AutoOwnership);*/ void registerSystemTypes(); +signals: + /** + * @brief The script being run threw an exception + * + * @param exception Exception that was thrown + */ + void exception(const ScriptException &exception); protected: static QMutex _v8InitMutex; static std::once_flag _v8InitOnceFlag; static v8::Platform* getV8Platform(); + void setUncaughtException(const v8::TryCatch &tryCatch, const QString& info = QString()); + + ScriptException _uncaughtException; + + // V8TODO: clean up isolate when script engine is destroyed? v8::Isolate* _v8Isolate; //v8::UniquePersistent _v8Context; - + struct CustomMarshal { ScriptEngine::MarshalFunction marshalFunc; ScriptEngine::DemarshalFunction demarshalFunc; @@ -235,8 +246,6 @@ protected: using CustomMarshalMap = QHash; using CustomPrototypeMap = QHash; - QPointer _scriptManager; - mutable QReadWriteLock _customTypeProtect { QReadWriteLock::Recursive }; CustomMarshalMap _customTypes; CustomPrototypeMap _customPrototypes;