Blacklist: Don't let errors become warnings #5516

Before, blacklisted errors were set to FileIgnored status and hence
displayed as warnings. Now, they have their own BlacklistedError
category which allows them to appear as errors in the issues list and in
the shell integration icons.

diff --git a/src/gui/protocolwidget.cpp b/src/gui/protocolwidget.cpp
index 9d7058dd8b84d1c68b05e3a3e23dc49bddec005e..9803e796bd36e49ac73d03b89711972e7fb0d69a 100644 (file)
--- a/src/gui/protocolwidget.cpp
+++ b/src/gui/protocolwidget.cpp
@@ -186,7 +186,8 @@ QTreeWidgetItem *ProtocolWidget::createCompletedTreewidgetItem(const QString &fo
 
     QIcon icon;
     if (item._status == SyncFileItem::NormalError
-        || item._status == SyncFileItem::FatalError) {
+        || item._status == SyncFileItem::FatalError
+        || item._status == SyncFileItem::BlacklistedError) {
         icon = Theme::instance()->syncStateIcon(SyncResult::Error);
     } else if (Progress::isWarningKind(item._status)) {
         icon = Theme::instance()->syncStateIcon(SyncResult::Problem);

diff --git a/src/libsync/owncloudpropagator.cpp b/src/libsync/owncloudpropagator.cpp
index f77dfd860ec18079b32ab1cc932712c6ade236c4..48ba29c0a17164763bc63649ce6ba12310b9e10c 100644 (file)
--- a/src/libsync/owncloudpropagator.cpp
+++ b/src/libsync/owncloudpropagator.cpp
@@ -195,8 +195,7 @@ static void blacklistUpdate(SyncJournalDb *journal, SyncFileItem &item)
     // An ignoreDuration of 0 mean we're tracking the error, but not actively
     // suppressing it.
     if (item._hasBlacklistEntry && newEntry._ignoreDuration > 0) {
-        item._status = SyncFileItem::FileIgnored;
-        item._errorString.prepend(PropagateItemJob::tr("Continue blacklisting:") + " ");
+        item._status = SyncFileItem::BlacklistedError;
 
         qCInfo(lcPropagator) << "blacklisting " << item._file
                              << " for " << newEntry._ignoreDuration
@@ -260,6 +259,7 @@ void PropagateItemJob::done(SyncFileItem::Status statusArg, const QString &error
     case SyncFileItem::Conflict:
     case SyncFileItem::FileIgnored:
     case SyncFileItem::NoStatus:
+    case SyncFileItem::BlacklistedError:
         // nothing
         break;
     }

diff --git a/src/libsync/progressdispatcher.cpp b/src/libsync/progressdispatcher.cpp
index 8473bc3ada236a285be86743bffda3116b27e0d0..2f86888ae4376f4a67a8d6e628f53c8ab1024518 100644 (file)
--- a/src/libsync/progressdispatcher.cpp
+++ b/src/libsync/progressdispatcher.cpp
@@ -90,7 +90,8 @@ bool Progress::isWarningKind(SyncFileItem::Status kind)
 {
     return kind == SyncFileItem::SoftError || kind == SyncFileItem::NormalError
         || kind == SyncFileItem::FatalError || kind == SyncFileItem::FileIgnored
-        || kind == SyncFileItem::Conflict || kind == SyncFileItem::Restoration;
+        || kind == SyncFileItem::Conflict || kind == SyncFileItem::Restoration
+        || kind == SyncFileItem::BlacklistedError;
 }
 
 bool Progress::isIgnoredKind(SyncFileItem::Status kind)

diff --git a/src/libsync/syncengine.cpp b/src/libsync/syncengine.cpp
index faf003fbf6473cc606628b4a25a9372284a1b1e8..5c1f1f10c74a6b26f779e6404b82e21ec732972b 100644 (file)
--- a/src/libsync/syncengine.cpp
+++ b/src/libsync/syncengine.cpp
@@ -256,12 +256,20 @@ bool SyncEngine::checkErrorBlacklisting(SyncFileItem &item)
         }
     }
 
+    int waitSeconds = entry._lastTryTime + entry._ignoreDuration - now;
     qCInfo(lcEngine) << "Item is on blacklist: " << entry._file
                      << "retries:" << entry._retryCount
-                     << "for another" << (entry._lastTryTime + entry._ignoreDuration - now) << "s";
-    item._instruction = CSYNC_INSTRUCTION_ERROR;
-    item._status = SyncFileItem::FileIgnored;
-    item._errorString = tr("The item is not synced because of previous errors: %1").arg(entry._errorString);
+                     << "for another" << waitSeconds << "s";
+
+    // We need to indicate that we skip this file due to blacklisting
+    // for reporting and for making sure we don't update the blacklist
+    // entry yet.
+    // Classification is this _instruction and _status
+    item._instruction = CSYNC_INSTRUCTION_IGNORE;
+    item._status = SyncFileItem::BlacklistedError;
+
+    auto waitSecondsStr = Utility::durationToDescriptiveString1(1000 * waitSeconds);
+    item._errorString = tr("%1 (skipped due to earlier error, trying again in %2)").arg(entry._errorString, waitSecondsStr);
 
     return true;
 }

diff --git a/src/libsync/syncfileitem.h b/src/libsync/syncfileitem.h
index 3eefac5cee2f6ce4a0e1d1fb42bf939b667cdb1e..b84c9ae33a5cd87985c8539bd269f504c32b12f2 100644 (file)
--- a/src/libsync/syncfileitem.h
+++ b/src/libsync/syncfileitem.h
@@ -53,7 +53,7 @@ public:
         SoftLink = CSYNC_FTW_TYPE_SLINK
     };
 
-    enum Status {
+    enum Status { // stored in 4 bits
         NoStatus,
 
         FatalError, ///< Error that causes the sync to stop
@@ -63,7 +63,16 @@ public:
         Success, ///< The file was properly synced
         Conflict, ///< The file was properly synced, but a conflict was created
         FileIgnored, ///< The file is in the ignored list (or blacklisted with no retries left)
-        Restoration ///< The file was restored because what should have been done was not allowed
+        Restoration, ///< The file was restored because what should have been done was not allowed
+
+        /** For files whose errors were blacklisted.
+         *
+         * If _instruction == IGNORE, the file wasn't even reattempted.
+         *
+         * These errors should usually be shown as NormalErrors, but not be as loud
+         * as them.
+         */
+        BlacklistedError
     };
 
     SyncFileItem()

diff --git a/src/libsync/syncfilestatustracker.cpp b/src/libsync/syncfilestatustracker.cpp
index db1f778c20d7e87ac712e154e7536ab64fb31734..e643099b29ca2b11f41b0e77d3286dca74f241d2 100644 (file)
--- a/src/libsync/syncfilestatustracker.cpp
+++ b/src/libsync/syncfilestatustracker.cpp
@@ -65,6 +65,7 @@ static inline bool showErrorInSocketApi(const SyncFileItem &item)
     return item._instruction == CSYNC_INSTRUCTION_ERROR
         || status == SyncFileItem::NormalError
         || status == SyncFileItem::FatalError
+        || status == SyncFileItem::BlacklistedError
         || item._hasBlacklistEntry;
 }