Work on script engine exceptions, cleanup

libraries/entities/src/EntityScriptingInterface.cpp

@@ -1574,7 +1574,8 @@ bool EntityPropertyMetadataRequest::script(EntityItemID entityID, const ScriptVa
     if (!request->isStarted()) {
         request->deleteLater();
         auto engine = handler.engine();
-        callScopedHandlerObject(handler, engine->makeError(engine->newValue("Entities Scripting Provider unavailable"), "InternalError"), ScriptValue());
+        // It was originally "InternalError", but there's no such error data type in V8
+        callScopedHandlerObject(handler, engine->makeError(engine->newValue("Entities Scripting Provider unavailable"), "Error"), ScriptValue());
         return false;
     }
     return true;

libraries/script-engine-qtscript/src/BaseScriptEngine.h

@@ -1,102 +0,0 @@
-//
-//  BaseScriptEngine.h
-//  libraries/script-engine/src
-//
-//  Created by Timothy Dedischew on 02/01/17.
-//  Copyright 2017 High Fidelity, Inc.
-//  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
-//  SPDX-License-Identifier: Apache-2.0
-//
-
-#ifndef hifi_BaseScriptEngine_h
-#define hifi_BaseScriptEngine_h
-
-/*#include <functional>
-#include <QtCore/QDebug>
-#include <QtCore/QSharedPointer>
-#include <QtScript/QScriptEngine>
-
-class ScriptEngineQtScript;
-using ScriptEngineQtScriptPointer = QSharedPointer<ScriptEngineQtScript>;
-
-// common base class for extending QScriptEngine itself
-class BaseScriptEngine : public QScriptEngine, public QEnableSharedFromThis<BaseScriptEngine> {
-    Q_OBJECT
-public:
-    // threadsafe "unbound" version of QScriptEngine::nullValue()
-    static const QScriptValue unboundNullValue() { return QScriptValue(0, QScriptValue::NullValue); }
-
-    BaseScriptEngine() {}
-
-    *@jsdoc
-     * @function Script.lintScript
-     * @param {string} sourceCode - Source code.
-     * @param {string} fileName - File name.
-     * @param {number} [lineNumber=1] - Line number.
-     * @returns {object} Object.
-     * @deprecated This function is deprecated and will be removed.
-     *
-    Q_INVOKABLE QScriptValue lintScript(const QString& sourceCode, const QString& fileName, const int lineNumber = 1);
-
-    *@jsdoc
-     * @function Script.makeError
-     * @param {object} [other] - Other.
-     * @param {string} [type="Error"] - Error.
-     * @returns {object} Object.
-     * @deprecated This function is deprecated and will be removed.
-     *
-    Q_INVOKABLE QScriptValue makeError(const QScriptValue& other = QScriptValue(), const QString& type = "Error");
-    
-    QScriptValue cloneUncaughtException(const QString& detail = QString());
-    QScriptValue evaluateInClosure(const QScriptValue& locals, const QScriptProgram& program);
-
-    // if there is a pending exception and we are at the top level (non-recursive) stack frame, this emits and resets it
-    bool maybeEmitUncaughtException(const QString& debugHint = QString());
-
-    // if the currentContext() is valid then throw the passed exception; otherwise, immediately emit it.
-    // note: this is used in cases where C++ code might call into JS API methods directly
-    bool raiseException(const QScriptValue& exception);
-
-    // helper to detect and log warnings when other code invokes QScriptEngine/BaseScriptEngine in thread-unsafe ways
-    static bool IS_THREADSAFE_INVOCATION(const QThread *thread, const QString& method);
-signals:
-    *@jsdoc
-     * @function Script.signalHandlerException
-     * @param {object} exception - Exception.
-     * @returns {Signal}
-     * @deprecated This signal is deprecated and will be removed.
-     *
-    // Script.signalHandlerException is exposed by QScriptEngine.
-    
-protected:
-    // like `newFunction`, but allows mapping inline C++ lambdas with captures as callable QScriptValues
-    // even though the context/engine parameters are redundant in most cases, the function signature matches `newFunction`
-    // anyway so that newLambdaFunction can be used to rapidly prototype / test utility APIs and then if becoming
-    // permanent more easily promoted into regular static newFunction scenarios.
-    QScriptValue newLambdaFunction(std::function<QScriptValue(QScriptContext *context, QScriptEngine* engine)> operation, const QScriptValue& data = QScriptValue(), const QScriptEngine::ValueOwnership& ownership = QScriptEngine::AutoOwnership);
-
-#ifdef DEBUG_JS
-    static void _debugDump(const QString& header, const QScriptValue& object, const QString& footer = QString());
-#endif
-};
-
-// Lambda helps create callable QScriptValues out of std::functions:
-// (just meant for use from within the script engine itself)
-class Lambda : public QObject {
-    Q_OBJECT
-public:
-    Lambda(QScriptEngine *engine, std::function<QScriptValue(QScriptContext *context, QScriptEngine* engine)> operation, QScriptValue data);
-    ~Lambda();
-    public slots:
-        QScriptValue call();
-    QString toString() const;
-private:
-    QScriptEngine* engine;
-    std::function<QScriptValue(QScriptContext *context, QScriptEngine* engine)> operation;
-    QScriptValue data;
-};*/
-
-#endif // hifi_BaseScriptEngine_h

libraries/script-engine/src/ScriptManager.cpp

@@ -2029,7 +2029,7 @@ void ScriptManager::entityScriptContentAvailable(const EntityItemID& entityID, c
     }
 
     // SANITY/PERFORMANCE CHECK USING SANDBOX
-    // V8TODO: can be skipped for now but needs to be implemented before release
+    // V8TODO: can be skipped for now should we implement it before merging V8 branch?
 
     /*const int SANDBOX_TIMEOUT = 0.25 * MSECS_PER_SECOND;
     ScriptEnginePointer sandbox = newScriptEngine();

libraries/script-engine/src/v8/ScriptEngineV8.cpp

@@ -1212,40 +1212,42 @@ bool ScriptEngineV8::raiseException(const QString& error, const QString &reason)
 
 bool ScriptEngineV8::raiseException(const ScriptValue& exception, const QString &reason) {
     //V8TODO Im not sure how to finish these
-//    qCCritical(scriptengine_v8) << "Script exception occurred: " << exception.toString();
+    qCCritical(scriptengine_v8) << "Script exception occurred: " << exception.toString();
-//    ScriptValueV8Wrapper* unwrapped = ScriptValueV8Wrapper::unwrap(exception);
+    ScriptValueV8Wrapper* unwrapped = ScriptValueV8Wrapper::unwrap(exception);
-//    V8ScriptValue qException = unwrapped ? unwrapped->toV8Value() : QScriptEngine::newVariant(exception.toVariant());
+    ScriptValue qException = unwrapped ? exception : newVariant(exception.toVariant());
-
-  //  emit
-    //return raiseException(qException);
-
-//    qCCritical(scriptengine_v8) << "Raise exception for reason" << reason << "NOT IMPLEMENTED!";
-//    return false;
 
     return raiseException(ScriptValueV8Wrapper::fullUnwrap(this, exception));
 }
 
-
-
 bool ScriptEngineV8::raiseException(const V8ScriptValue& exception) {
     if (!IS_THREADSAFE_INVOCATION(thread(), __FUNCTION__)) {
         return false;
     }
 
+    v8::Locker locker(_v8Isolate);
+    v8::Isolate::Scope isolateScope(_v8Isolate);
+    v8::HandleScope handleScope(_v8Isolate);
+    v8::Context::Scope contextScope(getContext());
 
-    _v8Isolate->ThrowException(exception.constGet());
+    auto trace = v8::StackTrace::CurrentStackTrace(_v8Isolate, 2);
-
+    if (trace->GetFrameCount() > 0) {
-    /*if (QScriptEngine::currentContext()) {
         // we have an active context / JS stack frame so throw the exception per usual
-        QScriptEngine::currentContext()->throwValue(makeError(exception));
+        //currentContext()->throwValue(makeError(exception));
+        auto thrown = makeError(ScriptValue(new ScriptValueV8Wrapper(this, exception)));
+        auto thrownV8 = ScriptValueV8Wrapper::fullUnwrap(this, thrown);
+        _v8Isolate->ThrowException(thrownV8.get());
         return true;
-    } else if (_scriptManager) {
+    } else if (_manager) {
         // we are within a pure C++ stack frame (ie: being called directly by other C++ code)
         // in this case no context information is available so just emit the exception for reporting
-        V8ScriptValue thrown = makeError(exception);
+        ScriptValue thrown = makeError(ScriptValue(new ScriptValueV8Wrapper(this, exception)));
-        emit _scriptManager->unhandledException(ScriptValue(new ScriptValueV8Wrapper(this, std::move(thrown))));
+        auto scriptRuntimeException = std::make_shared<ScriptRuntimeException>();
-    }*/
+        ScriptValue message = thrown.property("stack"); //This contains more details along with the error message
-    //emit _scriptManager->unhandledException(ScriptValue(new ScriptValueV8Wrapper(this, std::move(thrown))));
+        scriptRuntimeException->errorMessage = message.toString();
+        scriptRuntimeException->thrownValue = thrown;
+        emit _manager->unhandledException(scriptRuntimeException);
+    }
+
     return false;
 }