Discovery: Do not abort the sync in case of error 404 (or 500)
authorOlivier Goffart <ogoffart@woboq.com>
Tue, 4 Jun 2019 14:00:43 +0000 (16:00 +0200)
committerKevin Ottens <kevin.ottens@nextcloud.com>
Tue, 15 Dec 2020 09:58:51 +0000 (10:58 +0100)
Issue: #7199

src/libsync/discovery.cpp
test/testremotediscovery.cpp

index 60226d4042a65db964f2325867e2ff195f586483..1d75772dd78180aba0425d7615a7bbcd899bcb40 100644 (file)
@@ -1359,20 +1359,18 @@ DiscoverySingleDirectoryJob *ProcessDirectoryJob::startAsyncServerQuery()
                 }
             };
 
-            if (results.error().code == 403) {
+            auto code = results.error().code;
+            qCWarning(lcDisco) << "Server error in directory" << _currentFolder._server << code;
+            if (code == 403 || code == 404 || code == 500 || code == 503) {
                 // 403 Forbidden can be sent by the server if the file firewall is active.
                 // A file or directory should be ignored and sync must continue. See #3490
-                qCWarning(lcDisco, "Directory access Forbidden (File Firewall?)");
-                ignoreOrFatal();
-            } else if (results.error().code == 503) {
                 // The server usually replies with the custom "503 Storage not available"
                 // if some path is temporarily unavailable. But in some cases a standard 503
                 // is returned too. Thus we can't distinguish the two and will treat any
                 // 503 as request to ignore the folder. See #3113 #2884.
-                qCWarning(lcDisco(), "Storage was not available!");
+                // Similarly, the server might also return 404 or 500 in case of bugs. #7199
                 ignoreOrFatal();
             } else {
-                qCWarning(lcDisco) << "Server error in directory" << _currentFolder._server << results.error().message;
                 fatalError();
             }
         }
index f59cffdbb2373d2867019b3744a60d14519cf72d..d2e52d92a35002af7c827d0cf894fd8fd890241a 100644 (file)
@@ -64,17 +64,20 @@ private slots:
         qRegisterMetaType<ErrorCategory>();
         QTest::addColumn<int>("errorKind");
         QTest::addColumn<QString>("expectedErrorString");
+        QTest::addColumn<bool>("syncSucceeds");
 
         QString itemErrorMessage = "Internal Server Fake Error";
 
-        QTest::newRow("403") << 403 << itemErrorMessage;
-        QTest::newRow("404") << 404 << itemErrorMessage;
-        QTest::newRow("500") << 500 << itemErrorMessage;
-        QTest::newRow("503") << 503 << itemErrorMessage;
+        QTest::newRow("400") << 400 << itemErrorMessage << false;
+        QTest::newRow("401") << 401 << itemErrorMessage << false;
+        QTest::newRow("403") << 403 << itemErrorMessage << true;
+        QTest::newRow("404") << 404 << itemErrorMessage << true;
+        QTest::newRow("500") << 500 << itemErrorMessage << true;
+        QTest::newRow("503") << 503 << itemErrorMessage << true;
         // 200 should be an error since propfind should return 207
-        QTest::newRow("200") << 200 << itemErrorMessage;
-        QTest::newRow("InvalidXML") << +InvalidXML << "Unknown error";
-        QTest::newRow("Timeout") << +Timeout << "Operation canceled";
+        QTest::newRow("200") << 200 << itemErrorMessage << false;
+        QTest::newRow("InvalidXML") << +InvalidXML << "Unknown error" << false;
+        QTest::newRow("Timeout") << +Timeout << "Operation canceled" << false;
     }
 
 
@@ -83,8 +86,7 @@ private slots:
     {
         QFETCH(int, errorKind);
         QFETCH(QString, expectedErrorString);
-        // 403/503 just ignore the temporarily unavailable directory
-        bool syncSucceeds = errorKind == 503 || errorKind == 403;
+        QFETCH(bool, syncSucceeds);
 
         FakeFolder fakeFolder{ FileInfo::A12_B12_C12_S12() };