feat: propagate Sink error handling (#2631)

launcher/net/ByteArraySink.h

@@ -58,6 +58,7 @@ class ByteArraySink : public Sink {
             qWarning() << "ByteArraySink did not initialize the buffer because it's not addressable";
         if (initAllValidators(request))
             return Task::State::Running;
+        m_fail_reason = "Failed to initialize validators";
         return Task::State::Failed;
     };
 
@@ -69,12 +70,14 @@ class ByteArraySink : public Sink {
             qWarning() << "ByteArraySink did not write the buffer because it's not addressable";
         if (writeAllValidators(data))
             return Task::State::Running;
+        m_fail_reason = "Failed to write validators";
         return Task::State::Failed;
     }
 
     auto abort() -> Task::State override
     {
         failAllValidators();
+        m_fail_reason = "Aborted";
         return Task::State::Failed;
     }
 
@@ -82,12 +85,13 @@ class ByteArraySink : public Sink {
     {
         if (finalizeAllValidators(reply))
             return Task::State::Succeeded;
+        m_fail_reason = "Failed to finalize validators";
         return Task::State::Failed;
     }
 
     auto hasLocalData() -> bool override { return false; }
 
-   private:
+   protected:
     std::shared_ptr<QByteArray> m_output;
 };
 }  // namespace Net

launcher/net/FileSink.cpp

@@ -51,6 +51,7 @@ Task::State FileSink::init(QNetworkRequest& request)
     // create a new save file and open it for writing
     if (!FS::ensureFilePathExists(m_filename)) {
         qCCritical(taskNetLogC) << "Could not create folder for " + m_filename;
+        m_fail_reason = "Could not create folder";
         return Task::State::Failed;
     }
 
@@ -58,11 +59,13 @@ Task::State FileSink::init(QNetworkRequest& request)
     m_output_file.reset(new PSaveFile(m_filename));
     if (!m_output_file->open(QIODevice::WriteOnly)) {
         qCCritical(taskNetLogC) << "Could not open " + m_filename + " for writing";
+        m_fail_reason = "Could not open file";
         return Task::State::Failed;
     }
 
     if (initAllValidators(request))
         return Task::State::Running;
+    m_fail_reason = "Failed to initialize validators";
     return Task::State::Failed;
 }
 
@@ -73,6 +76,7 @@ Task::State FileSink::write(QByteArray& data)
         m_output_file->cancelWriting();
         m_output_file.reset();
         m_wroteAnyData = false;
+        m_fail_reason = "Failed to write validators";
         return Task::State::Failed;
     }
 
@@ -105,13 +109,16 @@ Task::State FileSink::finalize(QNetworkReply& reply)
     if (gotFile || m_wroteAnyData) {
         // ask validators for data consistency
         // we only do this for actual downloads, not 'your data is still the same' cache hits
-        if (!finalizeAllValidators(reply))
+        if (!finalizeAllValidators(reply)) {
+            m_fail_reason = "Failed to finalize validators";
             return Task::State::Failed;
+        }
 
         // nothing went wrong...
         if (!m_output_file->commit()) {
             qCCritical(taskNetLogC) << "Failed to commit changes to " << m_filename;
             m_output_file->cancelWriting();
+            m_fail_reason = "Failed to commit changes";
             return Task::State::Failed;
         }
     }

launcher/net/NetRequest.cpp

@@ -84,7 +84,8 @@ void NetRequest::executeTask()
             break;
         case State::Inactive:
         case State::Failed:
-            emit failed("Failed to initialize sink");
+            m_failReason = m_sink->failReason();
+            emit failed(m_sink->failReason());
             emit finished();
             return;
         case State::AbortedByUser:
@@ -259,6 +260,7 @@ void NetRequest::downloadFinished()
     } else if (m_state == State::Failed) {
         qCDebug(logCat) << getUid().toString() << "Request failed in previous step:" << m_url.toString();
         m_sink->abort();
+        m_failReason = m_reply->errorString();
         emit failed(m_reply->errorString());
         emit finished();
         return;
@@ -278,7 +280,8 @@ void NetRequest::downloadFinished()
         if (m_state != State::Succeeded) {
             qCDebug(logCat) << getUid().toString() << "Request failed to write:" << m_url.toString();
             m_sink->abort();
-            emit failed("failed to write in sink");
+            m_failReason = m_sink->failReason();
+            emit failed(m_sink->failReason());
             emit finished();
             return;
         }
@@ -289,7 +292,8 @@ void NetRequest::downloadFinished()
     if (m_state != State::Succeeded) {
         qCDebug(logCat) << getUid().toString() << "Request failed to finalize:" << m_url.toString();
         m_sink->abort();
-        emit failed("failed to finalize the request");
+        m_failReason = m_sink->failReason();
+        emit failed(m_sink->failReason());
         emit finished();
         return;
     }
@@ -305,7 +309,7 @@ void NetRequest::downloadReadyRead()
         auto data = m_reply->readAll();
         m_state = m_sink->write(data);
         if (m_state == State::Failed) {
-            qCCritical(logCat) << getUid().toString() << "Failed to process response chunk";
+            qCCritical(logCat) << getUid().toString() << "Failed to process response chunk:" << m_sink->failReason();
         }
         // qDebug() << "Request" << m_url.toString() << "gained" << data.size() << "bytes";
     } else {

launcher/net/PasteUpload.cpp

@@ -36,6 +36,7 @@
  */
 
 #include "PasteUpload.h"
+#include <qobject.h>
 
 #include <QHttpPart>
 #include <QJsonArray>
@@ -99,86 +100,101 @@ QNetworkReply* PasteUpload::getReply(QNetworkRequest& request)
     return nullptr;
 };
 
-auto PasteUpload::Sink::init(QNetworkRequest&) -> Task::State
+auto PasteUpload::Sink::finalize(QNetworkReply& reply) -> Task::State
 {
-    m_output.clear();
+    if (!finalizeAllValidators(reply)) {
-    return Task::State::Running;
+        m_fail_reason = "Failed to finalize validators";
-};
+        return Task::State::Failed;
+    }
+    int statusCode = reply.attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
 
-auto PasteUpload::Sink::write(QByteArray& data) -> Task::State
+    if (reply.error() != QNetworkReply::NetworkError::NoError) {
-{
+        m_fail_reason = QObject::tr("Network error: %1").arg(reply.errorString());
-    m_output.append(data);
+        return Task::State::Failed;
-    return Task::State::Running;
+    } else if (statusCode != 200 && statusCode != 201) {
-}
+        QString reasonPhrase = reply.attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString();
+        m_fail_reason =
+            QObject::tr("Error: %1 returned unexpected status code %2 %3").arg(m_d->url().toString()).arg(statusCode).arg(reasonPhrase);
+        return Task::State::Failed;
+    }
 
-auto PasteUpload::Sink::abort() -> Task::State
+    switch (m_d->m_paste_type) {
-{
-    m_output.clear();
-    return Task::State::Failed;
-}
-
-auto PasteUpload::Sink::finalize(QNetworkReply&) -> Task::State
-{
-    switch (m_paste_type) {
         case PasteUpload::NullPointer:
-            m_result->link = QString::fromUtf8(m_output).trimmed();
+            m_d->m_pasteLink = QString::fromUtf8(*m_output).trimmed();
             break;
         case PasteUpload::Hastebin: {
             QJsonParseError jsonError;
-            auto doc = QJsonDocument::fromJson(m_output, &jsonError);
+            auto doc = QJsonDocument::fromJson(*m_output, &jsonError);
             if (jsonError.error != QJsonParseError::NoError) {
                 qDebug() << "hastebin server did not reply with JSON" << jsonError.errorString();
+                m_fail_reason =
+                    QObject::tr("Failed to parse response from hastebin server: expected JSON but got an invalid response. Error: %1")
+                        .arg(jsonError.errorString());
                 return Task::State::Failed;
             }
             auto obj = doc.object();
             if (obj.contains("key") && obj["key"].isString()) {
                 QString key = doc.object()["key"].toString();
-                m_result->link = m_base_url + "/" + key;
+                m_d->m_pasteLink = m_d->m_baseUrl + "/" + key;
             } else {
                 qDebug() << "Log upload failed:" << doc.toJson();
+                m_fail_reason = QObject::tr("Error: %1 returned a malformed response body").arg(m_d->url().toString());
                 return Task::State::Failed;
             }
             break;
         }
         case PasteUpload::Mclogs: {
             QJsonParseError jsonError;
-            auto doc = QJsonDocument::fromJson(m_output, &jsonError);
+            auto doc = QJsonDocument::fromJson(*m_output, &jsonError);
             if (jsonError.error != QJsonParseError::NoError) {
                 qDebug() << "mclogs server did not reply with JSON" << jsonError.errorString();
+                m_fail_reason =
+                    QObject::tr("Failed to parse response from mclogs server: expected JSON but got an invalid response. Error: %1")
+                        .arg(jsonError.errorString());
                 return Task::State::Failed;
             }
             auto obj = doc.object();
             if (obj.contains("success") && obj["success"].isBool()) {
                 bool success = obj["success"].toBool();
                 if (success) {
-                    m_result->link = obj["url"].toString();
+                    m_d->m_pasteLink = obj["url"].toString();
                 } else {
-                    m_result->error = obj["error"].toString();
+                    QString error = obj["error"].toString();
+                    m_fail_reason = QObject::tr("Error: %1 returned an error: %2").arg(m_d->url().toString(), error);
+                    return Task::State::Failed;
                 }
             } else {
                 qDebug() << "Log upload failed:" << doc.toJson();
+                m_fail_reason = QObject::tr("Error: %1 returned a malformed response body").arg(m_d->url().toString());
                 return Task::State::Failed;
             }
             break;
         }
         case PasteUpload::PasteGG:
             QJsonParseError jsonError;
-            auto doc = QJsonDocument::fromJson(m_output, &jsonError);
+            auto doc = QJsonDocument::fromJson(*m_output, &jsonError);
             if (jsonError.error != QJsonParseError::NoError) {
                 qDebug() << "pastegg server did not reply with JSON" << jsonError.errorString();
+                m_fail_reason =
+                    QObject::tr("Failed to parse response from pasteGG server: expected JSON but got an invalid response. Error: %1")
+                        .arg(jsonError.errorString());
                 return Task::State::Failed;
             }
             auto obj = doc.object();
             if (obj.contains("status") && obj["status"].isString()) {
                 QString status = obj["status"].toString();
                 if (status == "success") {
-                    m_result->link = m_base_url + "/p/anonymous/" + obj["result"].toObject()["id"].toString();
+                    m_d->m_pasteLink = m_d->m_baseUrl + "/p/anonymous/" + obj["result"].toObject()["id"].toString();
                 } else {
-                    m_result->error = obj["error"].toString();
+                    QString error = obj["error"].toString();
-                    m_result->extra_message = (obj.contains("message") && obj["message"].isString()) ? obj["message"].toString() : "none";
+                    QString message = (obj.contains("message") && obj["message"].isString()) ? obj["message"].toString() : "none";
+                    m_fail_reason =
+                        QObject::tr("Error: %1 returned an error code: %2\nError message: %3").arg(m_d->url().toString(), error, message);
+                    return Task::State::Failed;
                 }
             } else {
                 qDebug() << "Log upload failed:" << doc.toJson();
+                m_fail_reason = QObject::tr("Error: %1 returned a malformed response body").arg(m_d->url().toString());
                 return Task::State::Failed;
             }
             break;
@@ -186,23 +202,18 @@ auto PasteUpload::Sink::finalize(QNetworkReply&) -> Task::State
     return Task::State::Succeeded;
 }
 
-Net::NetRequest::Ptr PasteUpload::make(const QString& log, PasteUpload::PasteType pasteType, QString customBaseURL, ResultPtr result)
+PasteUpload::PasteUpload(const QString& log, QString url, PasteType pasteType) : m_log(log), m_baseUrl(url), m_paste_type(pasteType)
-{
-    auto base = PasteUpload::PasteTypes.at(pasteType);
-    QString baseUrl = customBaseURL.isEmpty() ? base.defaultBase : customBaseURL;
-    auto up = makeShared<PasteUpload>(log, pasteType);
-
-    // HACK: Paste's docs say the standard API path is at /api/<version> but the official instance paste.gg doesn't follow that??
-    if (pasteType == PasteUpload::PasteGG && baseUrl == base.defaultBase)
-        up->m_url = "https://api.paste.gg/v1/pastes";
-    else
-        up->m_url = baseUrl + base.endpointPath;
-
-    up->m_sink.reset(new Sink(pasteType, baseUrl, result));
-    return up;
-}
-
-PasteUpload::PasteUpload(const QString& log, PasteType pasteType) : m_log(log), m_paste_type(pasteType)
 {
     anonymizeLog(m_log);
+    auto base = PasteUpload::PasteTypes.at(pasteType);
+    if (m_baseUrl.isEmpty())
+        m_baseUrl = base.defaultBase;
+
+    // HACK: Paste's docs say the standard API path is at /api/<version> but the official instance paste.gg doesn't follow that??
+    if (pasteType == PasteUpload::PasteGG && m_baseUrl == base.defaultBase)
+        m_url = "https://api.paste.gg/v1/pastes";
+    else
+        m_url = m_baseUrl + base.endpointPath;
+
+    m_sink.reset(new Sink(this));
 }

launcher/net/PasteUpload.h

@@ -35,6 +35,7 @@
 
 #pragma once
 
+#include "net/ByteArraySink.h"
 #include "net/NetRequest.h"
 #include "tasks/Task.h"
 
@@ -67,40 +68,29 @@ class PasteUpload : public Net::NetRequest {
     };
 
     static const std::array<PasteTypeInfo, 4> PasteTypes;
-    struct Result {
-        QString link;
-        QString error;
-        QString extra_message;
-    };
 
-    using ResultPtr = std::shared_ptr<Result>;
+    class Sink : public Net::ByteArraySink {
-
-    class Sink : public Net::Sink {
        public:
-        Sink(const PasteType pasteType, const QString base_url, ResultPtr result)
+        Sink(PasteUpload* p) : Net::ByteArraySink(std::make_shared<QByteArray>()), m_d(p) {};
-            : m_paste_type(pasteType), m_base_url(base_url), m_result(result) {};
         virtual ~Sink() = default;
 
        public:
-        auto init(QNetworkRequest& request) -> Task::State override;
-        auto write(QByteArray& data) -> Task::State override;
-        auto abort() -> Task::State override;
         auto finalize(QNetworkReply& reply) -> Task::State override;
-        auto hasLocalData() -> bool override { return false; }
 
        private:
-        const PasteType m_paste_type;
+        PasteUpload* m_d;
-        const QString m_base_url;
-        ResultPtr m_result;
-        QByteArray m_output;
     };
-    PasteUpload(const QString& log, PasteType pasteType);
+    friend Sink;
+
+    PasteUpload(const QString& log, QString url, PasteType pasteType);
     virtual ~PasteUpload() = default;
 
-    static NetRequest::Ptr make(const QString& log, PasteType pasteType, QString baseURL, ResultPtr result);
+    QString pasteLink() { return m_pasteLink; }
 
    private:
     virtual QNetworkReply* getReply(QNetworkRequest&) override;
     QString m_log;
+    QString m_pasteLink;
+    QString m_baseUrl;
     const PasteType m_paste_type;
-};
+};

launcher/net/Sink.h

@@ -52,6 +52,8 @@ class Sink {
 
     virtual auto hasLocalData() -> bool = 0;
 
+    QString failReason() const { return m_fail_reason; }
+
     void addValidator(Validator* validator)
     {
         if (validator) {
@@ -95,5 +97,6 @@ class Sink {
 
    protected:
     std::vector<std::shared_ptr<Validator>> validators;
+    QString m_fail_reason;
 };
 }  // namespace Net

launcher/screenshots/ImgurAlbumCreation.cpp

@@ -86,6 +86,7 @@ auto ImgurAlbumCreation::Sink::write(QByteArray& data) -> Task::State
 auto ImgurAlbumCreation::Sink::abort() -> Task::State
 {
     m_output.clear();
+    m_fail_reason = "Aborted";
     return Task::State::Failed;
 }
 
@@ -95,11 +96,13 @@ auto ImgurAlbumCreation::Sink::finalize(QNetworkReply&) -> Task::State
     QJsonDocument doc = QJsonDocument::fromJson(m_output, &jsonError);
     if (jsonError.error != QJsonParseError::NoError) {
         qDebug() << jsonError.errorString();
+        m_fail_reason = "Invalid json reply";
         return Task::State::Failed;
     }
     auto object = doc.object();
     if (!object.value("success").toBool()) {
         qDebug() << doc.toJson();
+        m_fail_reason = "Failed to create album";
         return Task::State::Failed;
     }
     m_result->deleteHash = object.value("data").toObject().value("deletehash").toString();

launcher/screenshots/ImgurUpload.cpp

@@ -90,6 +90,7 @@ auto ImgurUpload::Sink::write(QByteArray& data) -> Task::State
 auto ImgurUpload::Sink::abort() -> Task::State
 {
     m_output.clear();
+    m_fail_reason = "Aborted";
     return Task::State::Failed;
 }
 
@@ -99,11 +100,13 @@ auto ImgurUpload::Sink::finalize(QNetworkReply&) -> Task::State
     QJsonDocument doc = QJsonDocument::fromJson(m_output, &jsonError);
     if (jsonError.error != QJsonParseError::NoError) {
         qDebug() << "imgur server did not reply with JSON" << jsonError.errorString();
+        m_fail_reason = "Invalid json reply";
         return Task::State::Failed;
     }
     auto object = doc.object();
     if (!object.value("success").toBool()) {
         qDebug() << "Screenshot upload not successful:" << doc.toJson();
+        m_fail_reason = "Screenshot was not uploaded successfully";
         return Task::State::Failed;
     }
     m_shot->m_imgurId = object.value("data").toObject().value("id").toString();

launcher/tasks/ConcurrentTask.cpp

@@ -118,10 +118,29 @@ void ConcurrentTask::executeNextSubTask()
     }
     if (m_queue.isEmpty()) {
         if (m_doing.isEmpty()) {
-            if (m_failed.isEmpty())
+            if (m_failed.isEmpty()) {
                 emitSucceeded();
-            else
+            } else if (m_failed.count() == 1) {
-                emitFailed(tr("One or more subtasks failed"));
+                auto task = m_failed.keys().first();
+                auto reason = task->failReason();
+                if (reason.isEmpty()) {  // clearly a bug somewhere
+                    reason = tr("Task failed");
+                }
+                emitFailed(reason);
+            } else {
+                QStringList failReason;
+                for (auto t : m_failed) {
+                    auto reason = t->failReason();
+                    if (!reason.isEmpty()) {
+                        failReason << reason;
+                    }
+                }
+                if (failReason.isEmpty()) {
+                    emitFailed(tr("Multiple subtasks failed"));
+                } else {
+                    emitFailed(tr("Multiple subtasks failed\n%1").arg(failReason.join("\n")));
+                }
+            }
         }
         return;
     }

launcher/ui/GuiUtil.cpp

@@ -43,11 +43,10 @@
 #include <QFileDialog>
 #include <QStandardPaths>
 
-#include <memory>
-
 #include "FileSystem.h"
 #include "logs/AnonymizeLog.h"
 #include "net/NetJob.h"
+#include "net/NetRequest.h"
 #include "net/PasteUpload.h"
 #include "ui/dialogs/CustomMessageBox.h"
 #include "ui/dialogs/ProgressDialog.h"
@@ -134,10 +133,10 @@ std::optional<QString> GuiUtil::uploadPaste(const QString& name, const QString&
         textToUpload = truncateLogForMclogs(text);
     }
 
-    auto result = std::make_shared<PasteUpload::Result>();
     auto job = NetJob::Ptr(new NetJob("Log Upload", APPLICATION->network()));
 
-    job->addNetAction(PasteUpload::make(textToUpload, pasteType, baseURL, result));
+    auto pasteJob = new PasteUpload(textToUpload, baseURL, pasteType);
+    job->addNetAction(Net::NetRequest::Ptr(pasteJob));
     QObject::connect(job.get(), &Task::failed, [parentWidget](QString reason) {
         CustomMessageBox::selectable(parentWidget, QObject::tr("Failed to upload logs!"), reason, QMessageBox::Critical)->show();
     });
@@ -148,27 +147,19 @@ std::optional<QString> GuiUtil::uploadPaste(const QString& name, const QString&
     });
 
     if (dialog.execWithTask(job.get()) == QDialog::Accepted) {
-        if (!result->error.isEmpty() || !result->extra_message.isEmpty()) {
+        if (pasteJob->pasteLink().isEmpty()) {
-            QString message = QObject::tr("Error: %1").arg(result->error);
-            if (!result->extra_message.isEmpty()) {
-                message += QObject::tr("\nError message: %1").arg(result->extra_message);
-            }
-            CustomMessageBox::selectable(parentWidget, QObject::tr("Failed to upload logs!"), message, QMessageBox::Critical)->show();
-            return {};
-        }
-        if (result->link.isEmpty()) {
             CustomMessageBox::selectable(parentWidget, QObject::tr("Failed to upload logs!"), "The upload link is empty",
                                          QMessageBox::Critical)
                 ->show();
             return {};
         }
-        setClipboardText(result->link);
+        setClipboardText(pasteJob->pasteLink());
         CustomMessageBox::selectable(
             parentWidget, QObject::tr("Upload finished"),
-            QObject::tr("The <a href=\"%1\">link to the uploaded log</a> has been placed in your clipboard.").arg(result->link),
+            QObject::tr("The <a href=\"%1\">link to the uploaded log</a> has been placed in your clipboard.").arg(pasteJob->pasteLink()),
             QMessageBox::Information)
             ->exec();
-        return result->link;
+        return pasteJob->pasteLink();
     }
     return {};
 }