New Discovery Algorithm: handle server errors

diff --git a/src/csync/csync_update.cpp b/src/csync/csync_update.cpp
index eb1395a67905983161d57891330085865582536c..12448ed68f9532929bb12ffb1525f54fa06260c6 100644 (file)
--- a/src/csync/csync_update.cpp
+++ b/src/csync/csync_update.cpp
@@ -539,39 +539,6 @@ int csync_ftw(CSYNC *ctx, const char *uri, csync_walker_fn fn,
       return 0;
   }
 
-  if (!(dh = csync_vio_opendir(ctx, uri))) {
-      if (ctx->abort) {
-          qCDebug(lcUpdate, "Aborted!");
-          ctx->status_code = CSYNC_STATUS_ABORTED;
-          goto error;
-      }
-      /* permission denied */
-      ctx->status_code = csync_errno_to_status(errno, CSYNC_STATUS_OPENDIR_ERROR);
-
-      // 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
-      if (errno == ERRNO_FORBIDDEN) {
-          qCWarning(lcUpdate, "Directory access Forbidden (File Firewall?)");
-          if( mark_current_item_ignored(ctx, previous_fs, CSYNC_STATUS_FORBIDDEN) ) {
-              return 0;
-          }
-          /* if current_fs is not defined here, better throw an error */
-      }
-      // 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.
-      else if(errno == ERRNO_STORAGE_UNAVAILABLE || errno == ERRNO_SERVICE_UNAVAILABLE) {
-          qCWarning(lcUpdate, "Storage was not available!");
-          if( mark_current_item_ignored(ctx, previous_fs, CSYNC_STATUS_STORAGE_UNAVAILABLE ) ) {
-              return 0;
-          }
-          /* if current_fs is not defined here, better throw an error */
-      } else {
-          qCWarning(lcUpdate, "opendir failed for %s - errno %d", uri, errno);
-      }
-      goto error;
-  }
 
   while (true) {
     // Get the next item in the directory

diff --git a/src/libsync/discovery.cpp b/src/libsync/discovery.cpp
index a345a2a62903bc223e00d872474f1899e1e10671..a579cc1a892a09f86227ef039e13170c791b29ec 100644 (file)
--- a/src/libsync/discovery.cpp
+++ b/src/libsync/discovery.cpp
@@ -29,53 +29,46 @@ namespace OCC {
 
 Q_LOGGING_CATEGORY(lcDisco, "sync.discovery", QtInfoMsg)
 
-static RemoteInfo remoteInfoFromCSync(const csync_file_stat_t &x)
-{
-    RemoteInfo ri;
-    ri.name = QFileInfo(QString::fromUtf8(x.path)).fileName();
-    ri.etag = x.etag;
-    ri.fileId = x.file_id;
-    ri.checksumHeader = x.checksumHeader;
-    ri.modtime = x.modtime;
-    ri.size = x.size;
-    ri.isDirectory = x.type == ItemTypeDirectory;
-    ri.remotePerm = x.remotePerm;
-    return ri;
-}
-
-DiscoverServerJob::DiscoverServerJob(const AccountPtr &account, const QString &path, QObject *parent)
-    : DiscoverySingleDirectoryJob(account, path, parent)
-{
-    connect(this, &DiscoverySingleDirectoryJob::finishedWithResult, this, [this] {
-        auto csync_results = takeResults();
-        QVector<RemoteInfo> results;
-        std::transform(csync_results.begin(), csync_results.end(), std::back_inserter(results),
-            [](const auto &x) { return remoteInfoFromCSync(*x); });
-        emit this->finished(results);
-    });
-
-    connect(this, &DiscoverySingleDirectoryJob::finishedWithError, this,
-        [this](int code, const QString &msg) {
-            emit this->finished({ code, msg });
-        });
-}
-
 void ProcessDirectoryJob::start()
 {
     qCInfo(lcDisco) << "STARTING" << _currentFolder._server << _queryServer << _currentFolder._local << _queryLocal;
 
-    DiscoverServerJob *serverJob = nullptr;
+    DiscoverySingleDirectoryJob *serverJob = nullptr;
     if (_queryServer == NormalQuery) {
-        serverJob = new DiscoverServerJob(_discoveryData->_account, _discoveryData->_remoteFolder + _currentFolder._server, this);
-        connect(serverJob, &DiscoverServerJob::finished, this, [this](const auto &results) {
+        serverJob = new DiscoverySingleDirectoryJob(_discoveryData->_account, _discoveryData->_remoteFolder + _currentFolder._server, this);
+        connect(serverJob, &DiscoverySingleDirectoryJob::finished, this, [this](const auto &results) {
             if (results) {
                 _serverEntries = *results;
                 _hasServerEntries = true;
                 if (_hasLocalEntries)
                     process();
             } else {
-                qWarning() << results.errorMessage();
-                qFatal("TODO: ERROR HANDLING");
+                if (results.errorCode() == 403) {
+                    // 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?)");
+                    if (_dirItem) {
+                        _dirItem->_instruction = CSYNC_INSTRUCTION_IGNORE;
+                        _dirItem->_errorString = results.errorMessage();
+                        emit finished();
+                        return;
+                    }
+                } else if (results.errorCode() == 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!");
+                    if (_dirItem) {
+                        _dirItem->_instruction = CSYNC_INSTRUCTION_IGNORE;
+                        _dirItem->_errorString = results.errorMessage();
+                        emit finished();
+                        return;
+                    }
+                }
+                emit _discoveryData->fatalError(tr("Server replied with an error while reading directory '%1' : %2")
+                    .arg(_currentFolder._server, results.errorMessage()));
+                emit finished();
             }
         });
         serverJob->start();
@@ -115,12 +108,6 @@ void ProcessDirectoryJob::start()
                 }
             } else if (errno == ENOENT) {
                 errorString = tr("Directory not found: %1").arg(_discoveryData->_localDir + _currentFolder._local);
-                if (_dirItem) {
-                    _dirItem->_instruction = CSYNC_INSTRUCTION_IGNORE;
-                    _dirItem->_errorString = errorString;
-                    emit finished();
-                    return;
-                }
             } else if (errno == ENOTDIR) {
                 // Not a directory..
                 // Just consider it is empty
@@ -133,6 +120,7 @@ void ProcessDirectoryJob::start()
             emit finished();
             return;
         }
+        errno = 0;
         while (auto dirent = csync_vio_local_readdir(dh)) {
             LocalInfo i;
             static QTextCodec *codec = QTextCodec::codecForName("UTF-8");
@@ -158,6 +146,12 @@ void ProcessDirectoryJob::start()
                 qFatal("FIXME:  NEED TO CARE ABOUT THE OTHER STUFF ");
             _localEntries.push_back(i);
         }
+        if (errno != 0) {
+            // Note: Windows vio converts any error into EACCES
+            qCWarning(lcDisco) << "readdir failed for file in " << _currentFolder._local << " - errno: " << errno;
+            emit _discoveryData->fatalError(tr("Error while reading directory %1").arg(_discoveryData->_localDir + _currentFolder._local));
+            emit finished();
+        }
         csync_vio_local_closedir(dh);
     }
     _hasLocalEntries = true;

diff --git a/src/libsync/discovery.h b/src/libsync/discovery.h
index 43444b03046322e412a36322bc99b22d9aa1f597..638aa902423a29eddc21b9ae898837432d83073f 100644 (file)
--- a/src/libsync/discovery.h
+++ b/src/libsync/discovery.h
@@ -24,44 +24,6 @@ class ExcludedFiles;
 namespace OCC {
 class SyncJournalDb;
 
-struct RemoteInfo
-{
-    QString name;
-    QByteArray etag;
-    QByteArray fileId;
-    QByteArray checksumHeader;
-    OCC::RemotePermissions remotePerm;
-    time_t modtime = 0;
-    int64_t size = 0;
-    bool isDirectory = false;
-    bool isValid() const { return !name.isNull(); }
-};
-
-struct LocalInfo
-{
-    QString name;
-    time_t modtime = 0;
-    int64_t size = 0;
-    uint64_t inode = 0;
-    bool isDirectory = false;
-    bool isHidden = false;
-    bool isVirtualFile = false;
-    bool isValid() const { return !name.isNull(); }
-};
-
-/**
- * Do the propfind on the server.
- * TODO: merge with DiscoverySingleDirectoryJob
- */
-class DiscoverServerJob : public DiscoverySingleDirectoryJob
-{
-    Q_OBJECT
-public:
-    explicit DiscoverServerJob(const AccountPtr &account, const QString &path, QObject *parent = 0);
-signals:
-    void finished(const Result<QVector<RemoteInfo>> &result);
-};
-
 class ProcessDirectoryJob : public QObject
 {
     Q_OBJECT
@@ -122,7 +84,7 @@ private:
     bool _hasServerEntries = false;
     bool _hasLocalEntries = false;
     int _pendingAsyncJobs = 0;
-    QPointer<DiscoverServerJob> _serverJob;
+    QPointer<DiscoverySingleDirectoryJob> _serverJob;
     std::deque<ProcessDirectoryJob *> _queuedJobs;
     QVector<ProcessDirectoryJob *> _runningJobs;
     QueryMode _queryServer;

diff --git a/src/libsync/discoveryphase.cpp b/src/libsync/discoveryphase.cpp
index 9e5dcdb3275701165a59dc5b4609d892945caf3d..855d941cb570708761b1766a95a725f291b19038 100644 (file)
--- a/src/libsync/discoveryphase.cpp
+++ b/src/libsync/discoveryphase.cpp
@@ -181,80 +181,6 @@ void DiscoveryJob::update_job_update_callback(bool local,
     }
 }*/
 
-// Only use for error cases! It will always set an error errno
-int get_errno_from_http_errcode(int err, const QString &reason)
-{
-    int new_errno = EIO;
-
-    switch (err) {
-    case 401: /* Unauthorized */
-    case 402: /* Payment Required */
-    case 407: /* Proxy Authentication Required */
-    case 405:
-        new_errno = EPERM;
-        break;
-    case 301: /* Moved Permanently */
-    case 303: /* See Other */
-    case 404: /* Not Found */
-    case 410: /* Gone */
-        new_errno = ENOENT;
-        break;
-    case 408: /* Request Timeout */
-    case 504: /* Gateway Timeout */
-        new_errno = EAGAIN;
-        break;
-    case 423: /* Locked */
-        new_errno = EACCES;
-        break;
-    case 403: /* Forbidden */
-        new_errno = ERRNO_FORBIDDEN;
-        break;
-    case 400: /* Bad Request */
-    case 409: /* Conflict */
-    case 411: /* Length Required */
-    case 412: /* Precondition Failed */
-    case 414: /* Request-URI Too Long */
-    case 415: /* Unsupported Media Type */
-    case 424: /* Failed Dependency */
-    case 501: /* Not Implemented */
-        new_errno = EINVAL;
-        break;
-    case 507: /* Insufficient Storage */
-        new_errno = ENOSPC;
-        break;
-    case 206: /* Partial Content */
-    case 300: /* Multiple Choices */
-    case 302: /* Found */
-    case 305: /* Use Proxy */
-    case 306: /* (Unused) */
-    case 307: /* Temporary Redirect */
-    case 406: /* Not Acceptable */
-    case 416: /* Requested Range Not Satisfiable */
-    case 417: /* Expectation Failed */
-    case 422: /* Unprocessable Entity */
-    case 500: /* Internal Server Error */
-    case 502: /* Bad Gateway */
-    case 505: /* HTTP Version Not Supported */
-        new_errno = EIO;
-        break;
-    case 503: /* Service Unavailable */
-        // https://github.com/owncloud/core/pull/26145/files
-        if (reason == "Storage not available" || reason == "Storage is temporarily not available") {
-            new_errno = ERRNO_STORAGE_UNAVAILABLE;
-        } else {
-            new_errno = ERRNO_SERVICE_UNAVAILABLE;
-        }
-        break;
-    case 413: /* Request Entity too Large */
-        new_errno = EFBIG;
-        break;
-    default:
-        new_errno = EIO;
-    }
-    return new_errno;
-}
-
-
 DiscoverySingleDirectoryJob::DiscoverySingleDirectoryJob(const AccountPtr &account, const QString &path, QObject *parent)
     : QObject(parent)
     , _subPath(path)
@@ -305,50 +231,48 @@ void DiscoverySingleDirectoryJob::abort()
     }
 }
 
-static void propertyMapToFileStat(const QMap<QString, QString> &map, csync_file_stat_t *file_stat)
+static void propertyMapToFileStat(const QMap<QString, QString> &map, RemoteInfo &result)
 {
     for (auto it = map.constBegin(); it != map.constEnd(); ++it) {
         QString property = it.key();
         QString value = it.value();
         if (property == "resourcetype") {
-            if (value.contains("collection")) {
-                file_stat->type = ItemTypeDirectory;
-            } else {
-                file_stat->type = ItemTypeFile;
-            }
+            result.isDirectory = value.contains("collection");
         } else if (property == "getlastmodified") {
-            file_stat->modtime = oc_httpdate_parse(value.toUtf8());
+            result.modtime = oc_httpdate_parse(value.toUtf8());
         } else if (property == "getcontentlength") {
             // See #4573, sometimes negative size values are returned
             bool ok = false;
             qlonglong ll = value.toLongLong(&ok);
             if (ok && ll >= 0) {
-                file_stat->size = ll;
+                result.size = ll;
             } else {
-                file_stat->size = 0;
+                result.size = 0;
             }
         } else if (property == "getetag") {
-            file_stat->etag = Utility::normalizeEtag(value.toUtf8());
+            result.etag = Utility::normalizeEtag(value.toUtf8());
         } else if (property == "id") {
-            file_stat->file_id = value.toUtf8();
+            result.fileId = value.toUtf8();
         } else if (property == "downloadURL") {
-            file_stat->directDownloadUrl = value.toUtf8();
+            qFatal("FIXME: downloadURL and dDC");
+            //file_stat->directDownloadUrl = value.toUtf8();
         } else if (property == "dDC") {
-            file_stat->directDownloadCookies = value.toUtf8();
+            qFatal("FIXME: downloadURL and dDC");
+           // file_stat->directDownloadCookies = value.toUtf8();
         } else if (property == "permissions") {
-            file_stat->remotePerm = RemotePermissions(value);
+            result.remotePerm = RemotePermissions(value);
         } else if (property == "checksums") {
-            file_stat->checksumHeader = findBestChecksum(value.toUtf8());
+            result.checksumHeader = findBestChecksum(value.toUtf8());
         } else if (property == "share-types" && !value.isEmpty()) {
             // Since QMap is sorted, "share-types" is always after "permissions".
-            if (file_stat->remotePerm.isNull()) {
+            if (result.remotePerm.isNull()) {
                 qWarning() << "Server returned a share type, but no permissions?";
             } else {
                 // S means shared with me.
                 // But for our purpose, we want to know if the file is shared. It does not matter
                 // if we are the owner or not.
                 // Piggy back on the persmission field
-                file_stat->remotePerm.setPermission(RemotePermissions::IsShared);
+                result.remotePerm.setPermission(RemotePermissions::IsShared);
             }
         }
     }
@@ -372,48 +296,33 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(QString file, con
             }
         }
     } else {
-        // Remove <webDAV-Url>/folder/ from <webDAV-Url>/folder/subfile.txt
-        file.remove(0, _lsColJob->reply()->request().url().path().length());
-        // remove trailing slash
-        while (file.endsWith('/')) {
-            file.chop(1);
-        }
-        // remove leading slash
-        while (file.startsWith('/')) {
-            file = file.remove(0, 1);
-        }
 
-        std::unique_ptr<csync_file_stat_t> file_stat(new csync_file_stat_t);
-        file_stat->path = file.toUtf8();
-        file_stat->size = -1;
-        propertyMapToFileStat(map, file_stat.get());
-        if (file_stat->type == ItemTypeDirectory)
-            file_stat->size = 0;
-        if (file_stat->remotePerm.hasPermission(RemotePermissions::IsShared) && file_stat->etag.isEmpty()) {
-            /* Handle broken shared file error gracefully instead of stopping sync in the desktop client.
-               DO not set _error */
-            qCWarning(lcDiscovery)
-                << "Missing path to a share :" << file << file_stat->path << file_stat->type << file_stat->size
-                << file_stat->modtime << file_stat->remotePerm.toString()
-                << file_stat->etag << file_stat->file_id;
-        } else if (file_stat->type == ItemTypeSkip
-            || file_stat->size == -1
-            || file_stat->remotePerm.isNull()
-            || file_stat->etag.isEmpty()
-            || file_stat->file_id.isEmpty()) {
+        RemoteInfo result;
+        int slash = file.lastIndexOf('/');
+        result.name = file.mid(slash + 1);
+        result.size = -1;
+        result.modtime = -1;
+        propertyMapToFileStat(map, result);
+        if (result.isDirectory)
+            result.size = 0;
+        if (result.size == -1
+            || result.modtime == -1
+            || result.remotePerm.isNull()
+            || result.etag.isEmpty()
+            || result.fileId.isEmpty()) {
             _error = tr("The server file discovery reply is missing data.");
             qCWarning(lcDiscovery)
-                << "Missing properties:" << file << file_stat->type << file_stat->size
-                << file_stat->modtime << file_stat->remotePerm.toString()
-                << file_stat->etag << file_stat->file_id;
+                << "Missing properties:" << file << result.isDirectory << result.size
+                << result.modtime << result.remotePerm.toString()
+                << result.etag << result.fileId;
         }
 
-        if (_isExternalStorage && file_stat->remotePerm.hasPermission(RemotePermissions::IsMounted)) {
+        if (_isExternalStorage && result.remotePerm.hasPermission(RemotePermissions::IsMounted)) {
             /* All the entries in a external storage have 'M' in their permission. However, for all
                purposes in the desktop client, we only need to know about the mount points.
                So replace the 'M' by a 'm' for every sub entries in an external storage */
-            file_stat->remotePerm.unsetPermission(RemotePermissions::IsMounted);
-            file_stat->remotePerm.setPermission(RemotePermissions::IsMountedSub);
+            result.remotePerm.unsetPermission(RemotePermissions::IsMounted);
+            result.remotePerm.setPermission(RemotePermissions::IsMountedSub);
         }
 
         QStringRef fileRef(&file);
@@ -421,7 +330,7 @@ void DiscoverySingleDirectoryJob::directoryListingIteratedSlot(QString file, con
         if (slashPos > -1) {
             fileRef = file.midRef(slashPos + 1);
         }
-        _results.push_back(std::move(file_stat));
+        _results.push_back(std::move(result));
     }
 
     //This works in concerto with the RequestEtagJob and the Folder object to check if the remote folder changed.
@@ -439,17 +348,17 @@ void DiscoverySingleDirectoryJob::lsJobFinishedWithoutErrorSlot()
     if (!_ignoredFirst) {
         // This is a sanity check, if we haven't _ignoredFirst then it means we never received any directoryListingIteratedSlot
         // which means somehow the server XML was bogus
-        emit finishedWithError(ERRNO_WRONG_CONTENT, QLatin1String("Server error: PROPFIND reply is not XML formatted!"));
+        emit finished({ERRNO_WRONG_CONTENT, tr("Server error: PROPFIND reply is not XML formatted!")});
         deleteLater();
         return;
     } else if (!_error.isEmpty()) {
-        emit finishedWithError(ERRNO_WRONG_CONTENT, _error);
+        emit finished({ERRNO_WRONG_CONTENT, _error});
         deleteLater();
         return;
     }
     emit etag(_firstEtag);
     emit etagConcatenation(_etagConcatenation);
-    emit finishedWithResult();
+    emit finished(_results);
     deleteLater();
 }
 
@@ -459,20 +368,12 @@ void DiscoverySingleDirectoryJob::lsJobFinishedWithErrorSlot(QNetworkReply *r)
     int httpCode = r->attribute(QNetworkRequest::HttpStatusCodeAttribute).toInt();
     QString httpReason = r->attribute(QNetworkRequest::HttpReasonPhraseAttribute).toString();
     QString msg = r->errorString();
-    int errnoCode = EIO; // Something went wrong
     qCWarning(lcDiscovery) << "LSCOL job error" << r->errorString() << httpCode << r->error();
-    if (httpCode != 0 && httpCode != 207) {
-        errnoCode = get_errno_from_http_errcode(httpCode, httpReason);
-    } else if (r->error() != QNetworkReply::NoError) {
-        errnoCode = EIO;
-    } else if (!contentType.contains("application/xml; charset=utf-8")) {
-        msg = QLatin1String("Server error: PROPFIND reply is not XML formatted!");
-        errnoCode = ERRNO_WRONG_CONTENT;
-    } else {
-        // Default keep at EIO, see above
+    if (httpCode == 0 && r->error() == QNetworkReply::NoError
+        && !contentType.contains("application/xml; charset=utf-8")) {
+        msg = tr("Server error: PROPFIND reply is not XML formatted!");
     }
-
-    emit finishedWithError(errnoCode == 0 ? EIO : errnoCode, msg);
+    emit finished({httpCode, msg});
     deleteLater();
 }
 

diff --git a/src/libsync/discoveryphase.h b/src/libsync/discoveryphase.h
index 7cc31de3ff972a9e2fc35d8a029191d316911b72..5cf9550df2c01e4e0fede7168e357b700c3f5b15 100644 (file)
--- a/src/libsync/discoveryphase.h
+++ b/src/libsync/discoveryphase.h
@@ -34,20 +34,33 @@ namespace OCC {
 class Account;
 class SyncJournalDb;
 
-/**
- * The Discovery Phase was once called "update" phase in csync terms.
- * Its goal is to look at the files in one of the remote and check compared to the db
- * if the files are new, or changed.
- */
 
-struct DiscoveryDirectoryResult
+struct RemoteInfo
+{
+    QString name;
+    QByteArray etag;
+    QByteArray fileId;
+    QByteArray checksumHeader;
+    OCC::RemotePermissions remotePerm;
+    time_t modtime = 0;
+    int64_t size = 0;
+    bool isDirectory = false;
+    bool isValid() const { return !name.isNull(); }
+};
+
+struct LocalInfo
 {
-    QString path;
-    QString msg;
-    int code = EIO;
-    std::deque<std::unique_ptr<csync_file_stat_t>> list;
+    QString name;
+    time_t modtime = 0;
+    int64_t size = 0;
+    uint64_t inode = 0;
+    bool isDirectory = false;
+    bool isHidden = false;
+    bool isVirtualFile = false;
+    bool isValid() const { return !name.isNull(); }
 };
 
+
 /**
  * @brief The DiscoverySingleDirectoryJob class
  *
@@ -64,22 +77,20 @@ public:
     void setIsRootPath() { _isRootPath = true; }
     void start();
     void abort();
-    std::deque<std::unique_ptr<csync_file_stat_t>> &&takeResults() { return std::move(_results); }
 
     // This is not actually a network job, it is just a job
 signals:
     void firstDirectoryPermissions(RemotePermissions);
     void etagConcatenation(const QString &);
     void etag(const QString &);
-    void finishedWithResult();
-    void finishedWithError(int csyncErrnoCode, const QString &msg);
+    void finished(const Result<QVector<RemoteInfo>> &result);
 private slots:
     void directoryListingIteratedSlot(QString, const QMap<QString, QString> &);
     void lsJobFinishedWithoutErrorSlot();
     void lsJobFinishedWithErrorSlot(QNetworkReply *);
 
 private:
-    std::deque<std::unique_ptr<csync_file_stat_t>> _results;
+    QVector<RemoteInfo> _results;
     QString _subPath;
     QString _etagConcatenation;
     QString _firstEtag;

diff --git a/test/testremotediscovery.cpp b/test/testremotediscovery.cpp
index 4945bbf4cd373dcbd7112d7e541c4ef704db7cff..4e9953e50fa275d6271e460e5c6223b8d2b8ef59 100644 (file)
--- a/test/testremotediscovery.cpp
+++ b/test/testremotediscovery.cpp
@@ -106,7 +106,7 @@ private slots:
         QScopedValueRollback<int> setHttpTimeout(AbstractNetworkJob::httpTimeout, errorKind == Timeout ? 1 : 10000);
 
         QSignalSpy errorSpy(&fakeFolder.syncEngine(), &SyncEngine::syncError);
-        QCOMPARE(fakeFolder.syncOnce(), false);
+        QCOMPARE(fakeFolder.syncOnce(), syncSucceeds);
         qDebug() << "errorSpy=" << errorSpy;
 
         // The folder B should not have been sync'ed (and in particular not removed)