From 8bd89db060d79b5af9f4f430fbb517d6207c638e Mon Sep 17 00:00:00 2001
From: Gres <gres@gres.biz>
Date: Thu, 9 Apr 2020 21:01:13 +0300
Subject: [PATCH] Collect updater errors and signal only on final failure

---
 src/service/updates.cpp | 43 ++++++++++++++++++++++++++++++-----------
 src/service/updates.h   |  3 +++
 2 files changed, 35 insertions(+), 11 deletions(-)

diff --git a/src/service/updates.cpp b/src/service/updates.cpp
index 21e0e75..609266d 100644
--- a/src/service/updates.cpp
+++ b/src/service/updates.cpp
@@ -189,8 +189,10 @@ void Loader::startDownloadUpdates(const QUrl &previous)
     url = updateUrls_[index + 1];
   }
 
-  if (url.isEmpty())
+  if (url.isEmpty()) {
+    dumpErrors();
     return;
+  }
 
   auto reply = network_->get(QNetworkRequest(url));
   if (reply->error() != QNetworkReply::NoError)
@@ -203,14 +205,14 @@ void Loader::handleUpdateReply(QNetworkReply *reply)
 
   const auto url = reply->url();
   if (reply->error() != QNetworkReply::NoError) {
-    emit error(toError(*reply));
+    addError(toError(*reply));
     startDownloadUpdates(url);
     return;
   }
 
   const auto replyData = reply->readAll();
   if (replyData.isEmpty()) {
-    emit error(tr("Received empty updates info from %1").arg(url.toString()));
+    addError(tr("Received empty updates info from %1").arg(url.toString()));
     startDownloadUpdates(url);
     return;
   }
@@ -219,7 +221,7 @@ void Loader::handleUpdateReply(QNetworkReply *reply)
       url.toString().endsWith(".zip") ? unpack(replyData) : replyData;
 
   if (unpacked.isEmpty()) {
-    emit error(
+    addError(
         tr("Empty updates info after unpacking from %1").arg(url.toString()));
     startDownloadUpdates(url);
     return;
@@ -228,12 +230,14 @@ void Loader::handleUpdateReply(QNetworkReply *reply)
   SOFT_ASSERT(model_, return );
   const auto parseError = model_->parse(unpacked);
   if (!parseError.isEmpty()) {
-    emit error(tr("Failed to parse updates from %1 (%2)")
-                   .arg(url.toString(), parseError));
+    addError(tr("Failed to parse updates from %1 (%2)")
+                 .arg(url.toString(), parseError));
     startDownloadUpdates(url);
     return;
   }
 
+  errors_.clear();
+
   if (model_->hasUpdates())
     emit updatesAvailable();
 }
@@ -308,7 +312,7 @@ bool Loader::handleComponentReply(QNetworkReply *reply)
   downloads_.erase(reply);
 
   if (reply->error() != QNetworkReply::NoError) {
-    emit error(toError(*reply));
+    addError(toError(*reply));
 
     if (!startDownload(*file))
       finishUpdate();
@@ -326,7 +330,7 @@ bool Loader::handleComponentReply(QNetworkReply *reply)
   const auto url = reply->url();
   const auto replyData = reply->readAll();
   if (replyData.isEmpty()) {
-    emit error(tr("Empty data downloaded from %1").arg(url.toString()));
+    addError(tr("Empty data downloaded from %1").arg(url.toString()));
 
     if (!startDownload(*file))
       finishUpdate();
@@ -339,7 +343,7 @@ bool Loader::handleComponentReply(QNetworkReply *reply)
   const auto unpacked = mustUnpack ? unpack(replyData) : replyData;
 
   if (unpacked.isEmpty()) {
-    emit error(tr("Empty data after unpacking from %1").arg(url.toString()));
+    addError(tr("Empty data after unpacking from %1").arg(url.toString()));
 
     if (!startDownload(*file))
       finishUpdate();
@@ -370,7 +374,8 @@ void Loader::finishUpdate(const QString &error)
   for (const auto &i : downloads_) i.first->deleteLater();
   downloads_.clear();
   if (!error.isEmpty())
-    emit this->error(error);
+    addError(error);
+  dumpErrors();
   SOFT_ASSERT(model_, return );
   model_->updateStates();
 }
@@ -381,9 +386,10 @@ void Loader::commitUpdate()
   Installer installer(currentActions_);
   if (installer.commit()) {
     model_->resetProgress();
+    errors_.clear();
     emit updated();
   } else {
-    emit error(tr("Update failed: %1").arg(installer.errorString()));
+    addError(tr("Update failed: %1").arg(installer.errorString()));
   }
   finishUpdate();
 }
@@ -403,6 +409,21 @@ Model *Loader::model() const
   return model_;
 }
 
+void Loader::addError(const QString &text)
+{
+  LTRACE() << text;
+  errors_.append(text);
+}
+
+void Loader::dumpErrors()
+{
+  if (errors_.isEmpty())
+    return;
+  const auto summary = errors_.join('\n');
+  emit error(summary);
+  errors_.clear();
+}
+
 Model::Model(QObject *parent)
   : QAbstractItemModel(parent)
 {
diff --git a/src/service/updates.h b/src/service/updates.h
index 403c989..a2a8085 100644
--- a/src/service/updates.h
+++ b/src/service/updates.h
@@ -137,6 +137,8 @@ signals:
   void error(const QString& error);
 
 private:
+  void addError(const QString& text);
+  void dumpErrors();
   void handleReply(QNetworkReply* reply);
   bool handleComponentReply(QNetworkReply* reply);
   void handleUpdateReply(QNetworkReply* reply);
@@ -152,6 +154,7 @@ private:
   Urls updateUrls_;
   QString downloadPath_;
   std::map<QNetworkReply*, File*> downloads_;
+  QStringList errors_;
   UserActions currentActions_;
 };