From e51aedeb35a7e99b2ff6a28c8bf649cf177ff4bf Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 11 Mar 2016 11:36:52 -0800 Subject: [PATCH] fix error handling for diff request types --- .../src/assets/UploadAssetTask.cpp | 5 ++- interface/resources/qml/AssetServer.qml | 12 +++---- .../AssetMappingsScriptingInterface.cpp | 35 ++++--------------- .../AssetMappingsScriptingInterface.h | 4 +-- libraries/networking/src/MappingRequest.cpp | 21 +++++++++++ libraries/networking/src/MappingRequest.h | 1 + 6 files changed, 40 insertions(+), 38 deletions(-) diff --git a/assignment-client/src/assets/UploadAssetTask.cpp b/assignment-client/src/assets/UploadAssetTask.cpp index e850f29991..e82b99fe99 100644 --- a/assignment-client/src/assets/UploadAssetTask.cpp +++ b/assignment-client/src/assets/UploadAssetTask.cpp @@ -65,16 +65,19 @@ void UploadAssetTask::run() { replyPacket->writePrimitive(AssetServerError::NoError); replyPacket->write(hash); } else if (file.open(QIODevice::WriteOnly) && file.write(fileData) == qint64(fileSize)) { + qDebug() << "Wrote file" << hash << "to disk. Upload complete"; file.close(); replyPacket->writePrimitive(AssetServerError::NoError); replyPacket->write(hash); } else { + qWarning() << "Failed to upload or write to file" << hexHash << " - upload failed."; + // upload has failed - remove the file and return an error auto removed = file.remove(); if (!removed) { - qWarning() << "Removal of failed upload file" << hash << "failed."; + qWarning() << "Removal of failed upload file" << hexHash << "failed."; } replyPacket->writePrimitive(AssetServerError::FileOperationFailed); diff --git a/interface/resources/qml/AssetServer.qml b/interface/resources/qml/AssetServer.qml index c5ebb04819..1be5867413 100644 --- a/interface/resources/qml/AssetServer.qml +++ b/interface/resources/qml/AssetServer.qml @@ -57,7 +57,7 @@ Window { if (err) { console.log("Asset browser - error deleting path: ", path, err); - box = errorMessageBox("There was an error deleting:\n" + path + "\n" + Assets.getErrorString(err)); + box = errorMessageBox("There was an error deleting:\n" + path + "\n" + err); box.selected.connect(reload); } else { console.log("Asset browser - finished deleting path: ", path); @@ -88,7 +88,7 @@ Window { Assets.renameMapping(oldPath, newPath, function(err) { if (err) { console.log("Asset browser - error renaming: ", oldPath, "=>", newPath, " - error ", err); - box = errorMessageBox("There was an error renaming:\n" + oldPath + " to " + newPath + "\n" + Assets.getErrorString(err)); + box = errorMessageBox("There was an error renaming:\n" + oldPath + " to " + newPath + "\n" + err); box.selected.connect(reload); } else { console.log("Asset browser - finished rename: ", oldPath, "=>", newPath); @@ -130,10 +130,10 @@ Window { treeView.selection.clear(); } - function handleGetMappingsError(errorCode) { + function handleGetMappingsError(errorString) { errorMessageBox( "There was a problem retreiving the list of assets from your Asset Server.\n" - + Assets.getErrorString(errorCode) + + errorString ); } @@ -168,7 +168,7 @@ Window { if (!index) { index = treeView.selection.currentIndex; } - + var path = assetProxyModel.data(index, 0x100); if (!path) { return; @@ -248,7 +248,7 @@ Window { Assets.uploadFile(fileUrl, directory + filename, function(err, path) { if (err) { console.log("Asset Browser - error uploading: ", fileUrl, " - error ", err); - var box = errorMessage("There was an error uploading:\n" + fileUrl + "\n" + Assets.getErrorString(err)); + var box = errorMessageBox("There was an error uploading:\n" + fileUrl + "\n" + err); box.selected.connect(reload); } else { console.log("Asset Browser - finished uploading: ", fileUrl); diff --git a/interface/src/scripting/AssetMappingsScriptingInterface.cpp b/interface/src/scripting/AssetMappingsScriptingInterface.cpp index 32b697459c..d4f5431d32 100644 --- a/interface/src/scripting/AssetMappingsScriptingInterface.cpp +++ b/interface/src/scripting/AssetMappingsScriptingInterface.cpp @@ -27,34 +27,13 @@ AssetMappingsScriptingInterface::AssetMappingsScriptingInterface() { _proxyModel.sort(0); } -QString AssetMappingsScriptingInterface::getErrorString(int errorCode) const { - switch (errorCode) { - case MappingRequest::NoError: - return "No error"; - case MappingRequest::NotFound: - return "Asset not found"; - case MappingRequest::NetworkError: - return "Unable to communicate with Asset Server"; - case MappingRequest::PermissionDenied: - return "Permission denied"; - case MappingRequest::InvalidPath: - return "Path is invalid"; - case MappingRequest::InvalidHash: - return "Hash is invalid"; - case MappingRequest::UnknownError: - return "Asset Server internal error"; - default: - return QString(); - } -} - void AssetMappingsScriptingInterface::setMapping(QString path, QString hash, QJSValue callback) { auto assetClient = DependencyManager::get(); auto request = assetClient->createSetMappingRequest(path, hash); connect(request, &SetMappingRequest::finished, this, [this, callback](SetMappingRequest* request) mutable { if (callback.isCallable()) { - QJSValueList args { uint8_t(request->getError()), request->getPath() }; + QJSValueList args { request->getErrorString(), request->getPath() }; callback.call(args); } @@ -70,7 +49,7 @@ void AssetMappingsScriptingInterface::getMapping(QString path, QJSValue callback connect(request, &GetMappingRequest::finished, this, [this, callback](GetMappingRequest* request) mutable { if (callback.isCallable()) { - QJSValueList args { uint8_t(request->getError()) }; + QJSValueList args { request->getErrorString() }; callback.call(args); } @@ -108,7 +87,7 @@ void AssetMappingsScriptingInterface::uploadFile(QString path, QString mapping, QObject::connect(upload, &AssetUpload::finished, this, [=](AssetUpload* upload, const QString& hash) mutable { if (upload->getError() != AssetUpload::NoError) { if (callback.isCallable()) { - QJSValueList args { uint8_t(upload->getError()) }; + QJSValueList args { upload->getErrorString() }; callback.call(args); } } else { @@ -127,7 +106,7 @@ void AssetMappingsScriptingInterface::deleteMappings(QStringList paths, QJSValue connect(request, &DeleteMappingsRequest::finished, this, [this, callback](DeleteMappingsRequest* request) mutable { if (callback.isCallable()) { - QJSValueList args { uint8_t(request->getError()) }; + QJSValueList args { request->getErrorString() }; callback.call(args); } @@ -150,7 +129,7 @@ void AssetMappingsScriptingInterface::getAllMappings(QJSValue callback) { } if (callback.isCallable()) { - QJSValueList args { uint8_t(request->getError()), map }; + QJSValueList args { request->getErrorString(), map }; callback.call(args); } @@ -166,7 +145,7 @@ void AssetMappingsScriptingInterface::renameMapping(QString oldPath, QString new connect(request, &RenameMappingRequest::finished, this, [this, callback](RenameMappingRequest* request) mutable { if (callback.isCallable()) { - QJSValueList args { uint8_t(request->getError()) }; + QJSValueList args { request->getErrorString() }; callback.call(args); } @@ -293,7 +272,7 @@ void AssetMappingModel::refresh() { } } } else { - emit errorGettingMappings(static_cast(request->getError())); + emit errorGettingMappings(request->getErrorString()); } }); diff --git a/interface/src/scripting/AssetMappingsScriptingInterface.h b/interface/src/scripting/AssetMappingsScriptingInterface.h index 99ced25d29..9a427a05ed 100644 --- a/interface/src/scripting/AssetMappingsScriptingInterface.h +++ b/interface/src/scripting/AssetMappingsScriptingInterface.h @@ -39,7 +39,7 @@ bool isKnownFolder(QString path) const; signals: - void errorGettingMappings(int error); + void errorGettingMappings(QString errorString); private: QHash _pathToItemMap; @@ -59,8 +59,6 @@ public: Q_INVOKABLE bool isKnownMapping(QString path) const { return _assetMappingModel.isKnownMapping(path); } Q_INVOKABLE bool isKnownFolder(QString path) const { return _assetMappingModel.isKnownFolder(path); } - Q_INVOKABLE QString getErrorString(int errorCode) const; - Q_INVOKABLE void setMapping(QString path, QString hash, QJSValue callback = QJSValue()); Q_INVOKABLE void getMapping(QString path, QJSValue callback = QJSValue()); Q_INVOKABLE void uploadFile(QString path, QString mapping, QJSValue callback = QJSValue()); diff --git a/libraries/networking/src/MappingRequest.cpp b/libraries/networking/src/MappingRequest.cpp index 7b675050a1..be24c6b9d2 100644 --- a/libraries/networking/src/MappingRequest.cpp +++ b/libraries/networking/src/MappingRequest.cpp @@ -25,6 +25,27 @@ void MappingRequest::start() { doStart(); }; +QString MappingRequest::getErrorString() const { + switch (_error) { + case MappingRequest::NoError: + return "No error"; + case MappingRequest::NotFound: + return "Asset not found"; + case MappingRequest::NetworkError: + return "Unable to communicate with Asset Server"; + case MappingRequest::PermissionDenied: + return "Permission denied"; + case MappingRequest::InvalidPath: + return "Path is invalid"; + case MappingRequest::InvalidHash: + return "Hash is invalid"; + case MappingRequest::UnknownError: + return "Asset Server internal error"; + default: + return QString(); + } +} + GetMappingRequest::GetMappingRequest(const AssetPath& path) : _path(path.trimmed()) { }; diff --git a/libraries/networking/src/MappingRequest.h b/libraries/networking/src/MappingRequest.h index 504a0cb2fa..206e383c33 100644 --- a/libraries/networking/src/MappingRequest.h +++ b/libraries/networking/src/MappingRequest.h @@ -33,6 +33,7 @@ public: Q_INVOKABLE void start(); Error getError() const { return _error; } + Q_INVOKABLE QString getErrorString() const; protected: Error _error { NoError };