Merge pull request #5036 from nextcloud/feature/improveNetworkErrorHa…

…ndling Feature/improve network error handling
nextcloud · Jun 28, 2023 · 0a65df1 · 0a65df1
2 parents a7e10a1 + 033a37e
commit 0a65df1
Show file tree

Hide file tree

Showing 9 changed files with 80 additions and 14 deletions.
diff --git a/src/gui/accountstate.h b/src/gui/accountstate.h
@@ -47,6 +47,7 @@ class AccountState : public QObject, public QSharedData
 {
     Q_OBJECT
     Q_PROPERTY(AccountPtr account MEMBER _account)
+    Q_PROPERTY(State state READ state NOTIFY stateChanged)
 
 public:
     enum State {

diff --git a/src/gui/tray/activitylistmodel.cpp b/src/gui/tray/activitylistmodel.cpp
@@ -48,6 +48,11 @@ ActivityListModel::ActivityListModel(AccountState *accountState,
     : QAbstractListModel(parent)
     , _accountState(accountState)
 {
+    if (_accountState) {
+        connect(_accountState, &AccountState::stateChanged,
+                this, &ActivityListModel::accountStateChanged);
+        _accountStateWasConnected = false;
+    }
 }
 
 QHash<int, QByteArray> ActivityListModel::roleNames() const
@@ -90,8 +95,17 @@ QHash<int, QByteArray> ActivityListModel::roleNames() const
 
 void ActivityListModel::setAccountState(AccountState *state)
 {
+    if (_accountState == state) {
+        return;
+    }
+
     _accountState = state;
     Q_EMIT accountStateChanged();
+    if (_accountState) {
+        connect(_accountState, &AccountState::stateChanged,
+                this, &ActivityListModel::accountStateHasChanged);
+        _accountStateWasConnected = false;
+    }
 }
 
 void ActivityListModel::setCurrentItem(const int currentItem)
@@ -558,15 +572,50 @@ void ActivityListModel::addEntriesToActivityList(const ActivityList &activityLis
     setHasSyncConflicts(conflictsFound);
 }
 
+void ActivityListModel::accountStateHasChanged()
+{
+    if (!_accountState) {
+        return;
+    }
+
+    if (_accountStateWasConnected == _accountState->isConnected()) {
+        return;
+    }
+
+    if (!_accountState->isConnected()) {
+        _durationSinceDisconnection.start();
+    } else {
+        _durationSinceDisconnection.invalidate();
+    }
+
+    _accountStateWasConnected = _accountState->isConnected();
+}
+
 void ActivityListModel::addErrorToActivityList(const Activity &activity, const ErrorType type)
 {
-    qCDebug(lcActivity) << "Error successfully added to the notification list: " << type << activity._message << activity._subject << activity._syncResultStatus << activity._syncFileItemStatus;
-    auto modifiedActivity = activity;
-    if (type == ErrorType::NetworkError) {
-        modifiedActivity._subject = tr("Network error occurred: client will retry syncing.");
+    auto shouldAddError = false;
+
+    switch (type)
+    {
+    case ErrorType::NetworkError:
+        if (_durationSinceDisconnection.isValid() && _durationSinceDisconnection.hasExpired(3 * 60 *1000)) {
+            shouldAddError = true;
+        }
+        break;
+    case ErrorType::SyncError:
+        shouldAddError = true;
+        break;
+    }
+
+    if (shouldAddError) {
+        qCDebug(lcActivity) << "Error successfully added to the notification list: " << type << activity._message << activity._subject << activity._syncResultStatus << activity._syncFileItemStatus;
+        auto modifiedActivity = activity;
+        if (type == ErrorType::NetworkError) {
+            modifiedActivity._subject = tr("Network error occurred: client will retry syncing.");
+        }
+        addEntriesToActivityList({modifiedActivity});
+        _notificationErrorsLists.prepend(modifiedActivity);
     }
-    addEntriesToActivityList({modifiedActivity});
-    _notificationErrorsLists.prepend(modifiedActivity);
 }
 
 void ActivityListModel::addIgnoredFileToList(const Activity &newActivity)

diff --git a/src/gui/tray/activitylistmodel.h b/src/gui/tray/activitylistmodel.h
@@ -128,7 +128,7 @@ public slots:
     void slotTriggerDismiss(const int activityIndex);
 
     void addNotificationToActivityList(const OCC::Activity &activity);
-    void addErrorToActivityList(const OCC::Activity &activity, const ErrorType type);
+    void addErrorToActivityList(const OCC::Activity &activity, const OCC::ActivityListModel::ErrorType type);
     void addIgnoredFileToList(const OCC::Activity &newActivity);
     void addSyncFileItemToActivityList(const OCC::Activity &activity);
     void removeActivityFromActivityList(int row);
@@ -163,6 +163,7 @@ protected slots:
 
 private slots:
     void addEntriesToActivityList(const OCC::ActivityList &activityList);
+    void accountStateHasChanged();
 
 private:
     static QVariantList convertLinksToMenuEntries(const Activity &activity);
@@ -209,6 +210,10 @@ private slots:
 
     bool _hasSyncConflicts = false;
 
+    bool _accountStateWasConnected = false;
+
+    QElapsedTimer _durationSinceDisconnection;
+
     static constexpr quint32 MaxActionButtons = 3;
 };
 }

diff --git a/src/gui/tray/usermodel.h b/src/gui/tray/usermodel.h
@@ -117,7 +117,7 @@ public slots:
     void slotItemCompleted(const QString &folder, const OCC::SyncFileItemPtr &item);
     void slotProgressInfo(const QString &folder, const OCC::ProgressInfo &progress);
     void slotAddError(const QString &folderAlias, const QString &message, OCC::ErrorCategory category);
-    void slotAddErrorToGui(const QString &folderAlias, const OCC::SyncFileItem::Status status, const QString &errorMessage, const QString &subject, const ErrorCategory category);
+    void slotAddErrorToGui(const QString &folderAlias, const OCC::SyncFileItem::Status status, const QString &errorMessage, const QString &subject, const OCC::ErrorCategory category);
     void slotNotificationRequestFinished(int statusCode);
     void slotNotifyNetworkError(QNetworkReply *reply);
     void slotEndNotificationRequest(int replyCode);

diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp
@@ -629,7 +629,7 @@ void PropagateUploadFileCommon::slotPollFinished()
     finalize();
 }
 
-void PropagateUploadFileCommon::done(SyncFileItem::Status status, const QString &errorString, const ErrorCategory category)
+void PropagateUploadFileCommon::done(const SyncFileItem::Status status, const QString &errorString, const ErrorCategory category)
 {
     _finished = true;
     PropagateItemJob::done(status, errorString, category);

diff --git a/src/libsync/propagateupload.h b/src/libsync/propagateupload.h
@@ -281,7 +281,7 @@ private slots:
     void slotPollFinished();
 
 protected:
-    void done(SyncFileItem::Status status, const QString &errorString = QString(), const ErrorCategory category = ErrorCategory::NoError) override;
+    void done(const SyncFileItem::Status status, const QString &errorString = QString(), const ErrorCategory category = ErrorCategory::NoError) override;
 
     /**
      * Aborts all running network jobs, except for the ones that mayAbortJob

diff --git a/test/testactivitylistmodel.cpp b/test/testactivitylistmodel.cpp
@@ -445,6 +445,17 @@ class TestActivityListModel : public QObject
         return model;
     }
 
+    void testActivityAdd(void(OCC::ActivityListModel::*addingMethod)(const OCC::Activity&, OCC::ActivityListModel::ErrorType), OCC::Activity &activity) {
+        const auto model = testingALM();
+        QCOMPARE(model->rowCount(), 0);
+
+        (model.data()->*addingMethod)(activity, OCC::ActivityListModel::ErrorType::SyncError);
+        QCOMPARE(model->rowCount(), 1);
+
+        const auto index = model->index(0, 0);
+        QVERIFY(index.isValid());
+    }
+
     void testActivityAdd(void(OCC::ActivityListModel::*addingMethod)(const OCC::Activity&), OCC::Activity &activity) {
         const auto model = testingALM();
         QCOMPARE(model->rowCount(), 0);

diff --git a/test/testdownload.cpp b/test/testdownload.cpp
@@ -61,7 +61,7 @@ private slots:
     {
         FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };
         fakeFolder.syncEngine().setIgnoreHiddenFiles(true);
-        QSignalSpy completeSpy(&fakeFolder.syncEngine(), &SyncEngine::itemCompleted);
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), &OCC::SyncEngine::itemCompleted);
         auto size = 30 * 1000 * 1000;
         fakeFolder.remoteModifier().insert("A/a0", size);
 
@@ -96,7 +96,7 @@ private slots:
 
         FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
         fakeFolder.syncEngine().setIgnoreHiddenFiles(true);
-        QSignalSpy completeSpy(&fakeFolder.syncEngine(), &SyncEngine::itemCompleted);
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), &OCC::SyncEngine::itemCompleted);
         auto size = 3'500'000;
         fakeFolder.remoteModifier().insert("A/broken", size);
 
@@ -236,7 +236,7 @@ private slots:
         resendActual = 0;
         resendExpected = 10;
 
-        QSignalSpy completeSpy(&fakeFolder.syncEngine(), &SyncEngine::itemCompleted);
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), &OCC::SyncEngine::itemCompleted);
         QVERIFY(!fakeFolder.syncOnce());
         QCOMPARE(resendActual, 4); // the 4th fails because it only resends 3 times
         QCOMPARE(getItem(completeSpy, "A/resendme")->_status, SyncFileItem::NormalError);

diff --git a/test/testsyncfilestatustracker.cpp b/test/testsyncfilestatustracker.cpp
@@ -16,7 +16,7 @@ class StatusPushSpy : public QSignalSpy
     SyncEngine &_syncEngine;
 public:
     StatusPushSpy(SyncEngine &syncEngine)
-        : QSignalSpy(&syncEngine.syncFileStatusTracker(), &SyncFileStatusTracker::fileStatusChanged)
+        : QSignalSpy(&syncEngine.syncFileStatusTracker(), &OCC::SyncFileStatusTracker::fileStatusChanged)
         , _syncEngine(syncEngine)
     { }