Merge pull request #4430 from nextcloud/bugfix/betterReconnectLogic

increase time between connection tries

src/gui/accountstate.cpp

@@ -21,17 +21,19 @@
 #include "logger.h"
 #include "configfile.h"
 #include "ocsnavigationappsjob.h"
+#include "pushnotifications.h"
 
 #include <QSettings>
 #include <QTimer>
-#include <qfontmetrics.h>
-
+#include <QFontMetrics>
 #include <QJsonDocument>
 #include <QJsonObject>
 #include <QJsonArray>
 #include <QNetworkRequest>
 #include <QBuffer>
 
+#include <cmath>
+
 namespace OCC {
 
 Q_LOGGING_CATEGORY(lcAccountState, "nextcloud.gui.account.state", QtInfoMsg)
@@ -54,6 +56,8 @@ AccountState::AccountState(AccountPtr account)
         this, &AccountState::slotCredentialsFetched);
     connect(account.data(), &Account::credentialsAsked,
         this, &AccountState::slotCredentialsAsked);
+    connect(account.data(), &Account::pushNotificationsReady,
+            this, &AccountState::slotPushNotificationsReady);
 
     connect(this, &AccountState::isConnectedChanged, [=]{
         // Get the Apps available on the server if we're now connected.
@@ -61,6 +65,12 @@ AccountState::AccountState(AccountPtr account)
             fetchNavigationApps();
         }
     });
+
+    connect(&_checkConnectionTimer, &QTimer::timeout, this, &AccountState::slotCheckConnection);
+    _checkConnectionTimer.setInterval(ConnectionValidator::DefaultCallingIntervalMsec);
+    _checkConnectionTimer.start();
+
+    QTimer::singleShot(0, this, &AccountState::slotCheckConnection);
 }
 
 AccountState::~AccountState() = default;
@@ -120,6 +130,9 @@ void AccountState::setState(State state)
         if (oldState == Connected || _state == Connected) {
             emit isConnectedChanged();
         }
+        if (_state == Connected) {
+            resetRetryCount();
+        }
     }
 
     // might not have changed but the underlying _connectionErrors might have
@@ -149,6 +162,16 @@ QString AccountState::stateString(State state)
     return tr("Unknown account state");
 }
 
+int AccountState::retryCount() const
+{
+    return _retryCount;
+}
+
+void AccountState::increaseRetryCount()
+{
+    ++_retryCount;
+}
+
 bool AccountState::isSignedOut() const
 {
     return _state == SignedOut;
@@ -234,8 +257,15 @@ void AccountState::trySignIn()
     }
 }
 
+void AccountState::systemOnlineConfigurationChanged()
+{
+    QMetaObject::invokeMethod(this, "slotCheckConnection", Qt::QueuedConnection);
+}
+
 void AccountState::checkConnectivity()
 {
+    qCInfo(lcAccountState()) << "check connectivity";
+
     if (isSignedOut() || _waitingForNewCredentials) {
         return;
     }
@@ -293,6 +323,20 @@ void AccountState::checkConnectivity()
 
 void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status status, const QStringList &errors)
 {
+    const auto updateRetryCount = [this]() {
+        increaseRetryCount();
+        qCInfo(lcAccountState()) << "connection retry count" << retryCount();
+        _lastCheckConnectionTimer.invalidate();
+        _lastCheckConnectionTimer.start();
+    };
+
+    const auto resetRetryConnection = [this]() {
+        qCInfo(lcAccountState) << "reset retry count";
+        resetRetryCount();
+        _lastCheckConnectionTimer.invalidate();
+        _lastCheckConnectionTimer.start();
+    };
+
     if (isSignedOut()) {
         qCWarning(lcAccountState) << "Signed out, ignoring" << status << _account->url().toString();
         return;
@@ -329,6 +373,7 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta
     case ConnectionValidator::Connected:
         if (_state != Connected) {
             setState(Connected);
+            resetRetryConnection();
 
             // Get the Apps available on the server.
             fetchNavigationApps();
@@ -340,6 +385,7 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta
     case ConnectionValidator::Undefined:
     case ConnectionValidator::NotConfigured:
         setState(Disconnected);
+        updateRetryCount();
         break;
     case ConnectionValidator::ServerVersionMismatch:
         setState(ConfigurationError);
@@ -349,6 +395,7 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta
         // or because we are having network issues. The latter one is
         // much more likely, so keep trying to connect.
         setState(NetworkError);
+        updateRetryCount();
         break;
     case ConnectionValidator::CredentialsWrong:
     case ConnectionValidator::CredentialsNotReady:
@@ -367,6 +414,7 @@ void AccountState::slotConnectionValidatorResult(ConnectionValidator::Status sta
         break;
     case ConnectionValidator::Timeout:
         setState(NetworkError);
+        updateRetryCount();
         break;
     }
 }
@@ -456,6 +504,11 @@ void AccountState::fetchNavigationApps(){
     job->getNavigationApps();
 }
 
+void AccountState::resetRetryCount()
+{
+    _retryCount = 0;
+}
+
 void AccountState::slotEtagResponseHeaderReceived(const QByteArray &value, int statusCode){
     if(statusCode == 200){
         qCDebug(lcAccountState) << "New navigation apps ETag Response Header received " << value;
@@ -468,6 +521,43 @@ void AccountState::slotOcsError(int statusCode, const QString &message)
     qCDebug(lcAccountState) << "Error " << statusCode << " while fetching new navigation apps: " << message;
 }
 
+void AccountState::slotCheckConnection()
+{
+    if (_lastCheckConnectionTimer.isValid()) {
+        static constexpr auto DefaultCallingIntervalMaxMsec = static_cast<int>(ConnectionValidator::DefaultCallingIntervalMsec) * 8;
+
+        const auto minDelay = std::max(retryCount() * ConnectionValidator::DefaultCallingIntervalMsec,
+                                       static_cast<int>(ConnectionValidator::DefaultCallingIntervalMsec));
+        const auto currentDelay = std::min(minDelay, DefaultCallingIntervalMaxMsec);
+
+        if (!_lastCheckConnectionTimer.hasExpired(currentDelay - 1)) {
+            qCInfo(lcAccountState()) << "timer has not expired: do not check now" << _lastCheckConnectionTimer.elapsed() << currentDelay;
+            return;
+        }
+    }
+
+    const auto currentState = state();
+
+    // Don't check if we're manually signed out or
+    // when the error is permanent.
+    const auto pushNotifications = account()->pushNotifications();
+    const auto pushNotificationsAvailable = (pushNotifications && pushNotifications->isReady());
+    if (currentState != AccountState::SignedOut && currentState != AccountState::ConfigurationError
+        && currentState != AccountState::AskingCredentials && !pushNotificationsAvailable) {
+        checkConnectivity();
+    } else if (currentState == AccountState::SignedOut && lastConnectionStatus() == AccountState::ConnectionStatus::SslError) {
+        qCWarning(lcAccountState()) << "Account is signed out due to SSL Handshake error. Going to perform a sign-in attempt...";
+        trySignIn();
+    }
+}
+
+void AccountState::slotPushNotificationsReady()
+{
+    if (state() != AccountState::State::Connected) {
+        setState(AccountState::State::Connected);
+    }
+}
+
 void AccountState::slotNavigationAppsFetched(const QJsonDocument &reply, int statusCode)
 {
     if(_account){

src/gui/accountstate.h

@@ -16,11 +16,13 @@
 #ifndef ACCOUNTINFO_H
 #define ACCOUNTINFO_H
 
+#include "connectionvalidator.h"
+#include "creds/abstractcredentials.h"
+
 #include <QByteArray>
 #include <QElapsedTimer>
 #include <QPointer>
-#include "connectionvalidator.h"
-#include "creds/abstractcredentials.h"
+#include <QTimer>
 
 #include <memory>
 
@@ -175,6 +177,8 @@ public:
     
     void trySignIn();
 
+    void systemOnlineConfigurationChanged();
+
 public slots:
     /// Triggers a ping to the server to update state and
     /// connection status and errors.
@@ -184,6 +188,10 @@ private:
     void setState(State state);
     void fetchNavigationApps();
 
+    int retryCount() const;
+    void increaseRetryCount();
+    void resetRetryCount();
+
 signals:
     void stateChanged(State state);
     void isConnectedChanged();
@@ -205,6 +213,11 @@ protected Q_SLOTS:
     void slotEtagResponseHeaderReceived(const QByteArray &value, int statusCode);
     void slotOcsError(int statusCode, const QString &message);
 
+private Q_SLOTS:
+
+    void slotCheckConnection();
+    void slotPushNotificationsReady();
+
 private:
     AccountPtr _account;
     State _state;
@@ -241,6 +254,12 @@ private:
     AccountAppList _apps;
 
     bool _isDesktopNotificationsAllowed;
+
+    int _retryCount = 0;
+
+    QTimer _checkConnectionTimer;
+    QElapsedTimer _lastCheckConnectionTimer;
+
 };
 
 class AccountApp : public QObject

src/gui/application.cpp

@@ -470,30 +470,16 @@ void Application::slotCleanup()
 void Application::slotSystemOnlineConfigurationChanged(QNetworkConfiguration cnf)
 {
     if (cnf.state() & QNetworkConfiguration::Active) {
-        QMetaObject::invokeMethod(this, "slotCheckConnection", Qt::QueuedConnection);
+        const auto list = AccountManager::instance()->accounts();
+        for (const auto &accountState : list) {
+            accountState->systemOnlineConfigurationChanged();
+        }
     }
 }
 
 void Application::slotCheckConnection()
 {
-    const auto list = AccountManager::instance()->accounts();
-    for (const auto &accountState : list) {
-        AccountState::State state = accountState->state();
-
-        // Don't check if we're manually signed out or
-        // when the error is permanent.
-        const auto pushNotifications = accountState->account()->pushNotifications();
-        const auto pushNotificationsAvailable = (pushNotifications && pushNotifications->isReady());
-        if (state != AccountState::SignedOut && state != AccountState::ConfigurationError
-            && state != AccountState::AskingCredentials && !pushNotificationsAvailable) {
-            accountState->checkConnectivity();
-        } else if (state == AccountState::SignedOut && accountState->lastConnectionStatus() == AccountState::ConnectionStatus::SslError) {
-            qCWarning(lcApplication) << "Account is signed out due to SSL Handshake error. Going to perform a sign-in attempt...";
-            accountState->trySignIn();
-        }
-    }
-
-    if (list.isEmpty()) {
+    if (AccountManager::instance()->accounts().isEmpty()) {
         // let gui open the setup wizard
         _gui->slotOpenSettingsDialog();