diff --git a/src/libsync/owncloudpropagator_p.h b/src/libsync/owncloudpropagator_p.h index 4983e570ad..6a5c077bfc 100644 --- a/src/libsync/owncloudpropagator_p.h +++ b/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) { diff --git a/src/libsync/propagatedownload.cpp b/src/libsync/propagatedownload.cpp index 364156fd1f..14e8dfdc63 100644 --- a/src/libsync/propagatedownload.cpp +++ b/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; } diff --git a/src/libsync/propagateupload.cpp b/src/libsync/propagateupload.cpp index d50658e022..3db62cee7c 100644 --- a/src/libsync/propagateupload.cpp +++ b/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) { diff --git a/test/testdownload.cpp b/test/testdownload.cpp index 75c2bb8437..b6969b0338 100644 --- a/test/testdownload.cpp +++ b/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, + "\n" + "\n" + "Sabre\\DAV\\Exception\\ServiceUnavailable\n" + "System in maintenance mode.\n" + ""); + } + 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) diff --git a/test/testsyncengine.cpp b/test/testsyncengine.cpp index 91c2e046a1..4fcb1049a0 100644 --- a/test/testsyncengine.cpp +++ b/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. } }