Propagator: Don't abort sync on error 503

Only do it when it is actually a maintenance mode

Issues #5088, #5859, https://github.com/owncloud/enterprise/issues/2637

src/libsync/owncloudpropagator_p.h

@@ -59,8 +59,7 @@ inline QByteArray getEtagFromReply(QNetworkReply *reply)
  * Given an error from the network, map to a SyncFileItem::Status error
  */
 inline SyncFileItem::Status classifyError(QNetworkReply::NetworkError nerror,
-    int httpCode,
-    bool *anotherSyncNeeded = NULL)
+    int httpCode, bool *anotherSyncNeeded = nullptr, const QByteArray &errorBody = QByteArray())
 {
     Q_ASSERT(nerror != QNetworkReply::NoError); // we should only be called when there is an error
 
@@ -76,9 +75,10 @@ inline SyncFileItem::Status classifyError(QNetworkReply::NetworkError nerror,
     }
 
     if (httpCode == 503) {
-        // "Service unavailable"
-        // Happens for maintenance mode and other temporary outages
-        return SyncFileItem::FatalError;
+        // When the server is in maintenance mode, we want to exit the sync immediatly
+        // so that we do not flood the server with many requests
+        return errorBody.contains(R"(>Sabre\DAV\Exception\ServiceUnavailable<)") ?
+            SyncFileItem::FatalError : SyncFileItem::NormalError;
     }
 
     if (httpCode == 412) {

src/libsync/propagatedownload.cpp

@@ -697,13 +697,16 @@ void PropagateDownloadFile::slotGetFinished()
             propagator()->_journal->avoidReadFromDbOnNextSync(_item->_file);
         }
 
+        QByteArray errorBody;
+        QString errorString = _item->_httpErrorCode >= 400 ? job->errorStringParsingBody(&errorBody)
+                                                           : job->errorString();
         SyncFileItem::Status status = job->errorStatus();
         if (status == SyncFileItem::NoStatus) {
             status = classifyError(err, _item->_httpErrorCode,
-                &propagator()->_anotherSyncNeeded);
+                &propagator()->_anotherSyncNeeded, errorBody);
         }
 
-        done(status,_item->_httpErrorCode >= 400 ? job->errorStringParsingBody() : job->errorString());
+        done(status, errorString);
         return;
     }
 

src/libsync/propagateupload.cpp

@@ -524,7 +524,7 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job)
     checkResettingErrors();
 
     SyncFileItem::Status status = classifyError(job->reply()->error(), _item->_httpErrorCode,
-        &propagator()->_anotherSyncNeeded);
+        &propagator()->_anotherSyncNeeded, replyContent);
 
     // Insufficient remote storage.
     if (_item->_httpErrorCode == 507) {

test/testdownload.cpp

@@ -121,6 +121,30 @@ private slots:
         QCOMPARE(getItem(completeSpy, "A/broken")->_status, SyncFileItem::NormalError);
         QVERIFY(getItem(completeSpy, "A/broken")->_errorString.contains(serverMessage));
     }
+
+    void serverMaintenence() {
+        // Server in maintenance must abort the sync.
+
+        FakeFolder fakeFolder{FileInfo::A12_B12_C12_S12()};
+        fakeFolder.remoteModifier().insert("A/broken");
+        fakeFolder.setServerOverride([&](QNetworkAccessManager::Operation op, const QNetworkRequest &request, QIODevice *) -> QNetworkReply * {
+            if (op == QNetworkAccessManager::GetOperation) {
+                return new FakeErrorReply(op, request, this, 503,
+                    "<?xml version=\"1.0\" encoding=\"utf-8\"?>\n"
+                    "<d:error xmlns:d=\"DAV:\" xmlns:s=\"http://sabredav.org/ns\">\n"
+                    "<s:exception>Sabre\\DAV\\Exception\\ServiceUnavailable</s:exception>\n"
+                    "<s:message>System in maintenance mode.</s:message>\n"
+                    "</d:error>");
+            }
+            return nullptr;
+        });
+
+        QSignalSpy completeSpy(&fakeFolder.syncEngine(), &SyncEngine::itemCompleted);
+        QVERIFY(!fakeFolder.syncOnce()); // Fail because A/broken
+        // FatalError means the sync was aborted, which is what we want
+        QCOMPARE(getItem(completeSpy, "A/broken")->_status, SyncFileItem::FatalError);
+        QVERIFY(getItem(completeSpy, "A/broken")->_errorString.contains("System in maintenance mode"));
+    }
 };
 
 QTEST_GUILESS_MAIN(TestDownload)

test/testsyncengine.cpp

@@ -239,8 +239,8 @@ private slots:
         fakeFolder.remoteModifier().insert("Y/Z/d7");
         fakeFolder.remoteModifier().insert("Y/Z/d8");
         fakeFolder.remoteModifier().insert("Y/Z/d9");
-        fakeFolder.serverErrorPaths().append("Y/Z/d2", 503); // 503 is a fatal error
-        fakeFolder.serverErrorPaths().append("Y/Z/d3", 503); // 503 is a fatal error
+        fakeFolder.serverErrorPaths().append("Y/Z/d2", 503);
+        fakeFolder.serverErrorPaths().append("Y/Z/d3", 503);
         QVERIFY(!fakeFolder.syncOnce());
         QCoreApplication::processEvents(); // should not crash
 
@@ -251,12 +251,12 @@ private slots:
             QVERIFY(!seen.contains(item->_file)); // signal only sent once per item
             seen.insert(item->_file);
             if (item->_file == "Y/Z/d2") {
-                QVERIFY(item->_status == SyncFileItem::FatalError);
-            } else if(item->_file == "Y/Z/d3") {
+                QVERIFY(item->_status == SyncFileItem::NormalError);
+            } else if (item->_file == "Y/Z/d3") {
                 QVERIFY(item->_status != SyncFileItem::Success);
+            } else if (!item->isDirectory()) {
+                QVERIFY(item->_status == SyncFileItem::Success);
             }
-            // We do not know about the other files - maybe the sync was aborted,
-            // maybe they finished before the error caused the abort.
         }
     }