History

Further exception work on V8

* Get rid of maybeEmitUncaughtException * Mostly get rid of makeError * Introduce exception hierarchy, change exceptions to shared_ptr * Simplify exception throwing code

Dale Glass committed 3 years ago

Commit 67e7a731 parent c0e62c510 files · +249 −260Browse files

Showing the changes to libraries/script-engine/src/ScriptManager.cpp (1 of 10 files). Show all

Unified · Split

libraries/script-engine/src/ScriptManager.cpp+34−28
@@ -282,6 +282,11 @@ ScriptManager::ScriptManager(Context context, const QString& scriptContents, con · 281 unmodified lines
282282 processLevelMaxRetries = 1;
283283 }
284284
285+
286+#if 0
287+ // V8TODO: Is this actually needed? Exceptions will already be logged on the
288+ // script engine side.
289+
285290 // this is where all unhandled exceptions end up getting logged
286291 connect(this, &ScriptManager::unhandledException, this, [this](const ScriptValue& err) {
287292 qCCritical(scriptengine) << "Caught exception";
@@ -296,6 +301,7 @@ ScriptManager::ScriptManager(Context context, const QString& scriptContents, con · 8 unmodified lines
296301
297302 logException(output);
298303 });
304+#endif
299305
300306 if (_type == Type::ENTITY_CLIENT || _type == Type::ENTITY_SERVER) {
301307 QObject::connect(this, &ScriptManager::update, this, [this]() {
@@ -891,13 +897,11 @@ void ScriptManager::run() { · 589 unmodified lines
891897 {
892898 PROFILE_RANGE(script, _fileNameString);
893899 _returnValue = _engine->evaluate(_scriptContents, _fileNameString);
894− _engine->maybeEmitUncaughtException(__FUNCTION__);
895900
896901 if (_engine->hasUncaughtException()) {
897902
898903 qCWarning(scriptengine) << "Engine has uncaught exception, stopping";
899904 stop();
900− emit unhandledException(_engine->cloneUncaughtException(__FUNCTION__));
901905 _engine->clearExceptions();
902906 }
903907 }
@@ -1023,7 +1027,7 @@ void ScriptManager::run() { · 119 unmodified lines
10231027 if (!_engine->isEvaluating() && _engine->hasUncaughtException()) {
10241028 qCWarning(scriptengine) << __FUNCTION__ << "---------- UNCAUGHT EXCEPTION --------";
10251029 qCWarning(scriptengine) << "runInThread" << _engine->uncaughtException();
1026− emit unhandledException(_engine->cloneUncaughtException(__FUNCTION__));
1030+ emit unhandledException(_engine->uncaughtException());
10271031 _engine->clearExceptions();
10281032 }
10291033 }
@@ -1258,9 +1262,8 @@ QString ScriptManager::_requireResolve(const QString& moduleId, const QString& r · 228 unmodified lines
12581262 }
12591263 auto message = QString("Cannot find module '%1' (%2)").arg(displayId);
12601264
1261− auto throwResolveError = [&](const ScriptValue& error) -> QString {
1262− _engine->raiseException(error);
1263− _engine->maybeEmitUncaughtException("require.resolve");
1265+ auto throwResolveError = [&](const QString& error, const QString &details = QString()) -> QString {
1266+ _engine->raiseException(error, "require.resolve: " + details);
12641267 return QString();
12651268 };
12661269
@@ -1269,7 +1272,7 @@ QString ScriptManager::_requireResolve(const QString& moduleId, const QString& r · 2 unmodified lines
12691272 if (idLength < 1 || idLength > MAX_MODULE_ID_LENGTH) {
12701273 auto details = QString("rejecting invalid module id size (%1 chars [1,%2])")
12711274 .arg(idLength).arg(MAX_MODULE_ID_LENGTH);
1272− return throwResolveError(_engine->makeError(_engine->newValue(message.arg(details)), "RangeError"));
1275+ return throwResolveError(details, "RangeError");
12731276 }
12741277
12751278 // this regex matches: absolute, dotted or path-like URLs
@@ -1296,15 +1299,15 @@ QString ScriptManager::_requireResolve(const QString& moduleId, const QString& r · 20 unmodified lines
12961299 if (QFileInfo(unanchoredUrl.toLocalFile()).isFile()) {
12971300 auto msg = QString("relative module ids must be anchored; use './%1' instead")
12981301 .arg(moduleId);
1299− return throwResolveError(_engine->makeError(_engine->newValue(message.arg(msg))));
1302+ return throwResolveError(message.arg(msg));
13001303 }
13011304 }
1302− return throwResolveError(_engine->makeError(_engine->newValue(message.arg("system module not found"))));
1305+ return throwResolveError(message.arg("system module not found"));
13031306 }
13041307 }
13051308
13061309 if (url.isRelative()) {
1307− return throwResolveError(_engine->makeError(_engine->newValue(message.arg("could not resolve module id"))));
1310+ return throwResolveError(message.arg("could not resolve module id"));
13081311 }
13091312
13101313 // if it looks like a local file, verify that it's an allowed path and really a file
@@ -1317,22 +1320,21 @@ QString ScriptManager::_requireResolve(const QString& moduleId, const QString& r · 6 unmodified lines
13171320
13181321 bool disallowOutsideFiles = !PathUtils::defaultScriptsLocation().isParentOf(canonical) && !currentSandboxURL.isLocalFile();
13191322 if (disallowOutsideFiles && !PathUtils::isDescendantOf(canonical, currentSandboxURL)) {
1320− return throwResolveError(_engine->makeError(_engine->newValue(message.arg(
1323+ return throwResolveError(message.arg(
13211324 QString("path '%1' outside of origin script '%2' '%3'")
13221325 .arg(PathUtils::stripFilename(url))
13231326 .arg(PathUtils::stripFilename(currentSandboxURL))
13241327 .arg(canonical.toString())
1325− ))));
1328+ ));
13261329 }
13271330 if (!file.exists()) {
1328− return throwResolveError(_engine->makeError(_engine->newValue(message.arg("path does not exist: " + url.toLocalFile()))));
1331+ return throwResolveError(message.arg("path does not exist: " + url.toLocalFile()));
13291332 }
13301333 if (!file.isFile()) {
1331− return throwResolveError(_engine->makeError(_engine->newValue(message.arg("path is not a file: " + url.toLocalFile()))));
1334+ return throwResolveError(message.arg("path is not a file: " + url.toLocalFile()));
13321335 }
13331336 }
13341337
1335− _engine->maybeEmitUncaughtException(__FUNCTION__);
13361338 return url.toString();
13371339 }
13381340
@@ -1488,7 +1490,7 @@ ScriptValue ScriptManager::instantiateModule(const ScriptValue& module, const QS · 149 unmodified lines
14881490 //_engine->scriptValueDebugDetails(module);
14891491 result = _engine->evaluateInClosure(closure, _engine->newProgram( sourceCode, modulePath ));
14901492 }
1491− _engine->maybeEmitUncaughtException(__FUNCTION__);
1493+
14921494 return result;
14931495 }
14941496
@@ -1510,9 +1512,8 @@ ScriptValue ScriptManager::require(const QString& moduleId) { · 15 unmodified lines
15101512 #ifdef DEBUG_JS_MODULES
15111513 qCWarning(scriptengine_module) << "throwing module error:" << error.toString() << modulePath << error.property("stack").toString();
15121514 #endif
1513− _engine->raiseException(error);
1515+ _engine->raiseException(error, "module error");
15141516 }
1515− _engine->maybeEmitUncaughtException("module");
15161517 return _engine->nullValue();
15171518 };
15181519
@@ -1520,7 +1521,6 @@ ScriptValue ScriptManager::require(const QString& moduleId) { · 1 unmodified line
15201521 QString modulePath = _requireResolve(moduleId);
15211522 if (modulePath.isNull() || _engine->hasUncaughtException()) {
15221523 // the resolver already threw an exception -- bail early
1523− _engine->maybeEmitUncaughtException(__FUNCTION__);
15241524 return _engine->nullValue();
15251525 }
15261526
@@ -1546,7 +1546,6 @@ ScriptValue ScriptManager::require(const QString& moduleId) { · 19 unmodified lines
15461546 qCDebug(scriptengine_module) << QString("require - using cached module for '%1' (loaded: %2)")
15471547 .arg(moduleId).arg(module.property("loaded").toString());
15481548 registerModuleWithParent(module, parent);
1549− _engine->maybeEmitUncaughtException("cached module");
15501549 return exports;
15511550 }
15521551
@@ -1594,7 +1593,6 @@ ScriptValue ScriptManager::require(const QString& moduleId) { · 41 unmodified lines
15941593
15951594 //qCDebug(scriptengine_module) << "//ScriptManager::require(" << moduleId << ")";
15961595
1597− _engine->maybeEmitUncaughtException(__FUNCTION__);
15981596 //qCDebug(scriptengine_module) << "Exports: " << _engine->scriptValueDebugDetails(module.property("exports"));
15991597 return module.property("exports");
16001598 }
@@ -1670,7 +1668,9 @@ void ScriptManager::include(const QStringList& includeFiles, const ScriptValue& · 69 unmodified lines
16701668
16711669 doWithEnvironment(capturedEntityIdentifier, capturedSandboxURL, operation);
16721670 if(_engine->hasUncaughtException()) {
1673− emit unhandledException(_engine->cloneUncaughtException("evaluateInclude"));
1671+ auto ex = _engine->uncaughtException();
1672+ ex->additionalInfo += "; evaluateInClosure";
1673+ emit unhandledException(ex);
16741674 _engine->clearExceptions();
16751675 }
16761676 } else {
@@ -1995,11 +1995,14 @@ void ScriptManager::entityScriptContentAvailable(const EntityItemID& entityID, c · 318 unmodified lines
19951995 //syntaxError.setProperty("detail", entityID.toString());
19961996 //V8TODO
19971997 //emit unhandledException(syntaxError);
1998+
19981999 return;
19992000 }
20002001 if (!program) {
20012002 setError("Bad program (isNull)", EntityScriptStatus::ERROR_RUNNING_SCRIPT);
2002− emit unhandledException(_engine->makeError(_engine->newValue("program.isNull")));
2003+ std::shared_ptr<ScriptException> ex = std::make_shared<ScriptEngineException>("Program is Null", "Bad program in entityScriptContentAvailable");
2004+ emit unhandledException(ex);
2005+
20032006 return; // done processing script
20042007 }
20052008
@@ -2163,7 +2166,10 @@ void ScriptManager::entityScriptContentAvailable(const EntityItemID& entityID, c · 157 unmodified lines
21632166 entityScriptObject = entityScriptConstructor.construct();
21642167
21652168 if (_engine->hasUncaughtException()) {
2166− entityScriptObject = _engine->cloneUncaughtException("(construct " + entityID.toString() + ")");
2169+ // V8TODO: Why were we copying the uncaught exception here? Does anything
2170+ // actually make use of that?
2171+ // entityScriptObject = _engine->cloneUncaughtException("(construct " + entityID.toString() + ")");
2172+ entityScriptObject = _engine->nullValue();
21672173 _engine->clearExceptions();
21682174 }
21692175 };
@@ -2171,9 +2177,10 @@ void ScriptManager::entityScriptContentAvailable(const EntityItemID& entityID, c · 1 unmodified line
21712177 doWithEnvironment(entityID, sandboxURL, initialization);
21722178
21732179 if (entityScriptObject.isError()) {
2174− auto exception = entityScriptObject;
2175− setError(formatException(exception, _enableExtendedJSExceptions.get()), EntityScriptStatus::ERROR_RUNNING_SCRIPT);
2176− emit unhandledException(exception);
2180+ // auto exception = entityScriptObject;
2181+ // setError(formatException(exception, _enableExtendedJSExceptions.get()), EntityScriptStatus::ERROR_RUNNING_SCRIPT);
2182+ // emit unhandledException(exception);
2183+ // V8TODO: Is this needed? Wouldn't the ScriptManager have already emitted the exception?
21772184 return;
21782185 }
21792186
@@ -2340,7 +2347,6 @@ void ScriptManager::doWithEnvironment(const EntityItemID& entityID, const QUrl& · 160 unmodified lines
23402347 #else
23412348 operation();
23422349 #endif
2343− _engine->maybeEmitUncaughtException(!entityID.isNull() ? entityID.toString() : __FUNCTION__);
23442350 currentEntityIdentifier = oldIdentifier;
23452351 currentSandboxURL = oldSandboxURL;
23462352 }