classify ScriptEngine errors better, and send them as signal to JS

This commit is contained in:
Brad Hefta-Gaub 2016-12-20 09:00:01 -08:00
parent c65471e880
commit 6a7fa63327
6 changed files with 108 additions and 41 deletions

View file

@ -5385,6 +5385,9 @@ void Application::registerScriptEngineWithApplicationServices(ScriptEngine* scri
// connect this script engines printedMessage signal to the global ScriptEngines onPrintedMessage
connect(scriptEngine, &ScriptEngine::printedMessage, DependencyManager::get<ScriptEngines>().data(), &ScriptEngines::onPrintedMessage);
connect(scriptEngine, &ScriptEngine::errorMessage, DependencyManager::get<ScriptEngines>().data(), &ScriptEngines::onErrorMessage);
connect(scriptEngine, &ScriptEngine::warningMessage, DependencyManager::get<ScriptEngines>().data(), &ScriptEngines::onWarningMessage);
connect(scriptEngine, &ScriptEngine::infoMessage, DependencyManager::get<ScriptEngines>().data(), &ScriptEngines::onInfoMessage);
}
bool Application::canAcceptURL(const QString& urlString) const {

View file

@ -86,6 +86,8 @@ static QScriptValue debugPrint(QScriptContext* context, QScriptEngine* engine){
.replace("\n", "\\n")
.replace("\r", "\\r")
.replace("'", "\\'");
// FIXME - this approach neeeds revisiting. print() comes here, which ends up doing an evaluate?
engine->evaluate("Script.print('" + message + "')");
return QScriptValue();
@ -130,20 +132,20 @@ QString encodeEntityIdIntoEntityUrl(const QString& url, const QString& entityID)
return url + " [EntityID:" + entityID + "]";
}
static bool hasCorrectSyntax(const QScriptProgram& program) {
static bool hasCorrectSyntax(const QScriptProgram& program, ScriptEngine* reportingEngine) {
const auto syntaxCheck = QScriptEngine::checkSyntax(program.sourceCode());
if (syntaxCheck.state() != QScriptSyntaxCheckResult::Valid) {
const auto error = syntaxCheck.errorMessage();
const auto line = QString::number(syntaxCheck.errorLineNumber());
const auto column = QString::number(syntaxCheck.errorColumnNumber());
const auto message = QString("[SyntaxError] %1 in %2:%3(%4)").arg(error, program.fileName(), line, column);
qCWarning(scriptengine) << qPrintable(message);
reportingEngine->scriptErrorMessage(qPrintable(message));
return false;
}
return true;
}
static bool hadUncaughtExceptions(QScriptEngine& engine, const QString& fileName) {
static bool hadUncaughtExceptions(QScriptEngine& engine, const QString& fileName, ScriptEngine* reportingEngine) {
if (engine.hasUncaughtException()) {
const auto backtrace = engine.uncaughtExceptionBacktrace();
const auto exception = engine.uncaughtException().toString();
@ -155,7 +157,7 @@ static bool hadUncaughtExceptions(QScriptEngine& engine, const QString& fileName
static const auto lineSeparator = "\n ";
message += QString("\n[Backtrace]%1%2").arg(lineSeparator, backtrace.join(lineSeparator));
}
qCWarning(scriptengine) << qPrintable(message);
reportingEngine->scriptErrorMessage(qPrintable(message));
return true;
}
return false;
@ -170,20 +172,20 @@ ScriptEngine::ScriptEngine(const QString& scriptContents, const QString& fileNam
DependencyManager::get<ScriptEngines>()->addScriptEngine(this);
connect(this, &QScriptEngine::signalHandlerException, this, [this](const QScriptValue& exception) {
hadUncaughtExceptions(*this, _fileNameString);
hadUncaughtExceptions(*this, _fileNameString, this);
});
setProcessEventsInterval(MSECS_PER_SECOND);
}
ScriptEngine::~ScriptEngine() {
qCDebug(scriptengine) << "Script Engine shutting down:" << getFilename();
scriptInfoMessage("Script Engine shutting down:" + getFilename());
auto scriptEngines = DependencyManager::get<ScriptEngines>();
if (scriptEngines) {
scriptEngines->removeScriptEngine(this);
} else {
qCWarning(scriptengine) << "Script destroyed after ScriptEngines!";
scriptWarningMessage("Script destroyed after ScriptEngines!");
}
}
@ -274,7 +276,7 @@ void ScriptEngine::runDebuggable() {
}
_lastUpdate = now;
// Debug and clear exceptions
hadUncaughtExceptions(*this, _fileNameString);
hadUncaughtExceptions(*this, _fileNameString, this);
});
timer->start(10);
@ -359,7 +361,7 @@ void ScriptEngine::waitTillDoneRunning() {
QThread::yieldCurrentThread();
}
qCDebug(scriptengine) << "Script Engine has stopped:" << getFilename();
scriptInfoMessage("Script Engine has stopped:" + getFilename());
}
}
@ -398,10 +400,25 @@ void ScriptEngine::scriptContentsAvailable(const QUrl& url, const QString& scrip
emit scriptLoaded(url.toString());
}
void ScriptEngine::scriptErrorMessage(const QString& message) {
qCCritical(scriptengine) << message;
emit errorMessage(message);
}
void ScriptEngine::scriptWarningMessage(const QString& message) {
qCWarning(scriptengine) << message;
emit warningMessage(message);
}
void ScriptEngine::scriptInfoMessage(const QString& message) {
qCInfo(scriptengine) << message;
emit infoMessage(message);
}
// FIXME - switch this to the new model of ScriptCache callbacks
void ScriptEngine::errorInLoadingScript(const QUrl& url) {
qCDebug(scriptengine) << "ERROR Loading file:" << url.toString() << "line:" << __LINE__;
emit errorLoadingScript(_fileNameString); // ??
scriptErrorMessage("ERROR Loading file:" + url.toString());
emit errorLoadingScript(_fileNameString);
}
// Even though we never pass AnimVariantMap directly to and from javascript, the queued invokeMethod of
@ -809,7 +826,7 @@ QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fi
// Check syntax
const QScriptProgram program(sourceCode, fileName, lineNumber);
if (!hasCorrectSyntax(program)) {
if (!hasCorrectSyntax(program, this)) {
return QScriptValue();
}
@ -817,7 +834,7 @@ QScriptValue ScriptEngine::evaluate(const QString& sourceCode, const QString& fi
const auto result = QScriptEngine::evaluate(program);
--_evaluatesPending;
const auto hadUncaughtException = hadUncaughtExceptions(*this, program.fileName());
const auto hadUncaughtException = hadUncaughtExceptions(*this, program.fileName(), this);
emit evaluationFinished(result, hadUncaughtException);
return result;
}
@ -934,10 +951,10 @@ void ScriptEngine::run() {
_lastUpdate = now;
// Debug and clear exceptions
hadUncaughtExceptions(*this, _fileNameString);
hadUncaughtExceptions(*this, _fileNameString, this);
}
qCDebug(scriptengine) << "Script Engine stopping:" << getFilename();
scriptInfoMessage("Script Engine stopping:" + getFilename());
stopAllTimers(); // make sure all our timers are stopped if the script is ending
emit scriptEnding();
@ -1044,7 +1061,7 @@ void ScriptEngine::timerFired() {
{
auto engine = DependencyManager::get<ScriptEngines>();
if (!engine || engine->isStopped()) {
qCDebug(scriptengine) << "Script.timerFired() while shutting down is ignored... parent script:" << getFilename();
scriptWarningMessage("Script.timerFired() while shutting down is ignored... parent script:" + getFilename());
return; // bail early
}
}
@ -1084,7 +1101,7 @@ QObject* ScriptEngine::setupTimerWithInterval(const QScriptValue& function, int
QObject* ScriptEngine::setInterval(const QScriptValue& function, int intervalMS) {
if (DependencyManager::get<ScriptEngines>()->isStopped()) {
qCDebug(scriptengine) << "Script.setInterval() while shutting down is ignored... parent script:" << getFilename();
scriptWarningMessage("Script.setInterval() while shutting down is ignored... parent script:" + getFilename());
return NULL; // bail early
}
@ -1093,7 +1110,7 @@ QObject* ScriptEngine::setInterval(const QScriptValue& function, int intervalMS)
QObject* ScriptEngine::setTimeout(const QScriptValue& function, int timeoutMS) {
if (DependencyManager::get<ScriptEngines>()->isStopped()) {
qCDebug(scriptengine) << "Script.setTimeout() while shutting down is ignored... parent script:" << getFilename();
scriptWarningMessage("Script.setTimeout() while shutting down is ignored... parent script:" + getFilename());
return NULL; // bail early
}
@ -1153,8 +1170,8 @@ void ScriptEngine::print(const QString& message) {
// all of the files have finished loading.
void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callback) {
if (DependencyManager::get<ScriptEngines>()->isStopped()) {
qCDebug(scriptengine) << "Script.include() while shutting down is ignored..."
<< "includeFiles:" << includeFiles << "parent script:" << getFilename();
scriptWarningMessage("Script.include() while shutting down is ignored... includeFiles:"
+ includeFiles.join(",") + "parent script:" + getFilename());
return; // bail early
}
QList<QUrl> urls;
@ -1182,7 +1199,7 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac
thisURL = expandScriptUrl(QUrl::fromLocalFile(expandScriptPath(file)));
QUrl defaultScriptsLoc = defaultScriptsLocation();
if (!defaultScriptsLoc.isParentOf(thisURL)) {
qCDebug(scriptengine) << "ScriptEngine::include -- skipping" << file << "-- outside of standard libraries";
scriptWarningMessage("Script.include() -- skipping" + file + "-- outside of standard libraries");
continue;
}
isStandardLibrary = true;
@ -1193,8 +1210,9 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac
if (!isStandardLibrary && !currentSandboxURL.isEmpty() && (thisURL.scheme() == "file") &&
(currentSandboxURL.scheme() != "file" ||
!thisURL.toString(strippingFlags).startsWith(currentSandboxURL.toString(strippingFlags), getSensitivity()))) {
qCWarning(scriptengine) << "Script.include() ignoring file path"
<< thisURL << "outside of original entity script" << currentSandboxURL;
scriptWarningMessage("Script.include() ignoring file path" + thisURL.toString()
+ "outside of original entity script" + currentSandboxURL.toString());
} else {
// We could also check here for CORS, but we don't yet.
// It turns out that QUrl.resolve will not change hosts and copy authority, so we don't need to check that here.
@ -1216,7 +1234,7 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac
for (QUrl url : urls) {
QString contents = data[url];
if (contents.isNull()) {
qCDebug(scriptengine) << "Error loading file: " << url << "line:" << __LINE__;
scriptErrorMessage("Error loading file: " + url.toString());
} else {
std::lock_guard<std::recursive_mutex> lock(_lock);
if (!_includedURLs.contains(url)) {
@ -1230,7 +1248,7 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac
doWithEnvironment(capturedEntityIdentifier, capturedSandboxURL, operation);
} else {
qCDebug(scriptengine) << "Script.include() skipping evaluation of previously included url:" << url;
scriptWarningMessage("Script.include() skipping evaluation of previously included url:" + url.toString());
}
}
}
@ -1259,8 +1277,8 @@ void ScriptEngine::include(const QStringList& includeFiles, QScriptValue callbac
void ScriptEngine::include(const QString& includeFile, QScriptValue callback) {
if (DependencyManager::get<ScriptEngines>()->isStopped()) {
qCDebug(scriptengine) << "Script.include() while shutting down is ignored... "
<< "includeFile:" << includeFile << "parent script:" << getFilename();
scriptWarningMessage("Script.include() while shutting down is ignored... includeFile:"
+ includeFile + "parent script:" + getFilename());
return; // bail early
}
@ -1274,13 +1292,13 @@ void ScriptEngine::include(const QString& includeFile, QScriptValue callback) {
// the Application or other context will connect to in order to know to actually load the script
void ScriptEngine::load(const QString& loadFile) {
if (DependencyManager::get<ScriptEngines>()->isStopped()) {
qCDebug(scriptengine) << "Script.load() while shutting down is ignored... "
<< "loadFile:" << loadFile << "parent script:" << getFilename();
scriptWarningMessage("Script.load() while shutting down is ignored... loadFile:"
+ loadFile + "parent script:" + getFilename());
return; // bail early
}
if (!currentEntityIdentifier.isInvalidID()) {
qCWarning(scriptengine) << "Script.load() from entity script is ignored... "
<< "loadFile:" << loadFile << "parent script:" << getFilename();
scriptWarningMessage("Script.load() from entity script is ignored... loadFile:"
+ loadFile + "parent script:" + getFilename());
return; // bail early
}
@ -1368,7 +1386,7 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co
auto fileName = isURL ? scriptOrURL : "EmbeddedEntityScript";
QScriptProgram program(contents, fileName);
if (!hasCorrectSyntax(program)) {
if (!hasCorrectSyntax(program, this)) {
if (!isFileUrl) {
scriptCache->addScriptToBadScriptList(scriptOrURL);
}
@ -1396,7 +1414,7 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co
});
testConstructor = sandbox.evaluate(program);
}
if (hadUncaughtExceptions(sandbox, program.fileName())) {
if (hadUncaughtExceptions(sandbox, program.fileName(), this)) {
return;
}
@ -1410,10 +1428,10 @@ void ScriptEngine::entityScriptContentAvailable(const EntityItemID& entityID, co
if (testConstructorValue.size() > maxTestConstructorValueSize) {
testConstructorValue = testConstructorValue.mid(0, maxTestConstructorValueSize) + "...";
}
qCDebug(scriptengine) << "Error -- ScriptEngine::loadEntityScript() entity:" << entityID
<< "failed to load entity script -- expected a function, got " + testConstructorType
<< "," << testConstructorValue
<< "," << scriptOrURL;
scriptErrorMessage("Error -- ScriptEngine::loadEntityScript() entity:" + entityID.toString()
+ "failed to load entity script -- expected a function, got " + testConstructorType
+ "," + testConstructorValue
+ "," + scriptOrURL);
if (!isFileUrl) {
scriptCache->addScriptToBadScriptList(scriptOrURL);
@ -1513,7 +1531,7 @@ void ScriptEngine::refreshFileScript(const EntityItemID& entityID) {
QString filePath = QUrl(details.scriptText).toLocalFile();
auto lastModified = QFileInfo(filePath).lastModified().toMSecsSinceEpoch();
if (lastModified > details.lastModified) {
qCDebug(scriptengine) << "Reloading modified script " << details.scriptText;
scriptInfoMessage("Reloading modified script " + details.scriptText);
QFile file(filePath);
file.open(QIODevice::ReadOnly);
@ -1521,7 +1539,7 @@ void ScriptEngine::refreshFileScript(const EntityItemID& entityID) {
this->unloadEntityScript(entityID);
this->entityScriptContentAvailable(entityID, details.scriptText, scriptContents, true, true);
if (!_entityScripts.contains(entityID)) {
qWarning() << "Reload script " << details.scriptText << " failed";
scriptWarningMessage("Reload script " + details.scriptText + " failed");
} else {
details = _entityScripts[entityID];
}
@ -1548,7 +1566,7 @@ void ScriptEngine::doWithEnvironment(const EntityItemID& entityID, const QUrl& s
#else
operation();
#endif
hadUncaughtExceptions(*this, _fileNameString);
hadUncaughtExceptions(*this, _fileNameString, this);
currentEntityIdentifier = oldIdentifier;
currentSandboxURL = oldSandboxURL;

View file

@ -174,6 +174,10 @@ public:
void setEmitScriptUpdatesFunction(std::function<bool()> func) { _emitScriptUpdates = func; }
void scriptErrorMessage(const QString& message);
void scriptWarningMessage(const QString& message);
void scriptInfoMessage(const QString& message);
public slots:
void callAnimationStateHandler(QScriptValue callback, AnimVariantMap parameters, QStringList names, bool useNames, AnimVariantResultHandler resultHandler);
void updateMemoryCost(const qint64&);
@ -187,6 +191,8 @@ signals:
void cleanupMenuItem(const QString& menuItemString);
void printedMessage(const QString& message);
void errorMessage(const QString& message);
void warningMessage(const QString& message);
void infoMessage(const QString& message);
void runningStateChanged();
void evaluationFinished(QScriptValue result, bool isException);
void loadScript(const QString& scriptName, bool isUserLoaded);

View file

@ -37,6 +37,26 @@ void ScriptEngines::onPrintedMessage(const QString& message) {
emit printedMessage(message, scriptEngine->getFilename());
}
void ScriptEngines::onErrorMessage(const QString& message) {
auto scriptEngine = qobject_cast<ScriptEngine*>(sender());
emit errorMessage(message, scriptEngine->getFilename());
}
void ScriptEngines::onWarningMessage(const QString& message) {
auto scriptEngine = qobject_cast<ScriptEngine*>(sender());
emit warningMessage(message, scriptEngine->getFilename());
}
void ScriptEngines::onInfoMessage(const QString& message) {
auto scriptEngine = qobject_cast<ScriptEngine*>(sender());
emit infoMessage(message, scriptEngine->getFilename());
}
void ScriptEngines::onErrorLoadingScript(const QString& url) {
auto scriptEngine = qobject_cast<ScriptEngine*>(sender());
emit errorLoadingScript(url, scriptEngine->getFilename());
}
ScriptEngines::ScriptEngines()
: _scriptsLocationHandle("scriptsLocation", DESKTOP_LOCATION)
{

View file

@ -74,10 +74,18 @@ signals:
void scriptCountChanged();
void scriptsReloading();
void scriptLoadError(const QString& filename, const QString& error);
void printedMessage(const QString& message, const QString& filename);
void printedMessage(const QString& message, const QString& engineName);
void errorMessage(const QString& message, const QString& engineName);
void warningMessage(const QString& message, const QString& engineName);
void infoMessage(const QString& message, const QString& engineName);
void errorLoadingScript(const QString& url, const QString& engineName);
public slots:
void onPrintedMessage(const QString& message);
void onErrorMessage(const QString& message);
void onWarningMessage(const QString& message);
void onInfoMessage(const QString& message);
void onErrorLoadingScript(const QString& url);
protected slots:
void onScriptFinished(const QString& fileNameString, ScriptEngine* engine);

View file

@ -23,3 +23,15 @@ window.closed.connect(function() { Script.stop(); });
ScriptDiscoveryService.printedMessage.connect(function(message, scriptFileName) {
window.sendToQml("[" + scriptFileName + "] " + message);
});
ScriptDiscoveryService.warningMessage.connect(function(message, scriptFileName) {
window.sendToQml("[" + scriptFileName + "] WARNING - " + message);
});
ScriptDiscoveryService.errorMessage.connect(function(message, scriptFileName) {
window.sendToQml("[" + scriptFileName + "] ERROR - " + message);
});
ScriptDiscoveryService.infoMessage.connect(function(message, scriptFileName) {
window.sendToQml("[" + scriptFileName + "] INFO - " + message);
});