[network/ruqola/ddp-reconnect-robustness] src/core: Make DDP reconnection robust against authentication cleanup
Till Adam <[email protected]>
| Newsgroups | gmane.comp.kde.cvs |
|---|---|
| Message-ID | <[email protected]> |
Git commit 20677e4a8233bda054a0a628c45f143cfb6b963a by Till Adam.
Committed on 25/07/2026 at 09:20.
Pushed by tilladam into branch 'ddp-reconnect-robustness'.
Make DDP reconnection robust against authentication cleanup
When the DDP websocket closed, authentication cleanup
(setLoginStatus(LoggedOutAndCleanedUp)) synchronously disconnected the
DDP client's signals from the account before the reconnect notification
could be delivered, so a server-initiated close could leave the client
disconnected without ever reconnecting.
- Emit disconnectedByServer/wsClosedSocketError before changing the
authentication state in DDPClient::onWSclosed, and queue an
account-owned reconnect callback (guarded by a QPointer) so it
survives the signal disconnect during cleanup.
- Recreate the DDP client on reconnect (rather than re-logging in a
dead one), with an exponential backoff capped at 60s and a generation
counter to invalidate superseded reconnect timers.
- Give the DDP-only and full-server reconnect paths their own backoff
state instead of sharing mDelayReconnect, and route both through a
single capped nextReconnectDelay() helper (the full-server path was
previously uncapped and could grow without bound).
- Reset the DDP backoff only once authentication reaches LoggedIn, so a
server that accepts the handshake but drops during login can't cause a
tight reconnect loop.
- Add DDPClientTest covering that the close notification survives
authentication cleanup for both normal and error closes.
Claude-Session: https://claude.ai/code/session_019S3mh6FrcakoNjfBKMBG4A
M +1 -0 src/core/autotests/CMakeLists.txt
A +142 -0 src/core/autotests/ddpclienttest.cpp [License: LGPL(v2.0+)]
A +17 -0 src/core/autotests/ddpclienttest.h [License: LGPL(v2.0+)]
M +6 -4 src/core/ddpapi/ddpclient.cpp
M +3 -0 src/core/ddpapi/ddpclient.h
M +77 -14 src/core/rocketchataccount.cpp
M +4 -0 src/core/rocketchataccount.h
https://invent.kde.org/network/ruqola/-/commit/20677e4a8233bda054a0a628c45f143cfb6b963a
diff --git a/src/core/autotests/CMakeLists.txt b/src/core/autotests/CMakeLists.txt
index 6e7d6a3541..9943665105 100644
--- a/src/core/autotests/CMakeLists.txt
+++ b/src/core/autotests/CMakeLists.txt
@@ -149,6 +149,7 @@ add_ruqola_test(commandstest.cpp)
add_ruqola_test(lrucachetest.cpp)
add_ruqola_test(notifierjobtest.cpp)
add_ruqola_test(ddpauthenticationmanagertest.cpp)
+add_ruqola_test(ddpclienttest.cpp)
add_ruqola_test(restauthenticationmanagertest.cpp)
add_ruqola_test(downloadappslanguagesparsertest.cpp)
add_ruqola_test(downloadappslanguagesinfotest.cpp)
diff --git a/src/core/autotests/ddpclienttest.cpp b/src/core/autotests/ddpclienttest.cpp
new file mode 100644
index 0000000000..03a88be158
--- /dev/null
+++ b/src/core/autotests/ddpclienttest.cpp
@@ -0,0 +1,142 @@
+/*
+ SPDX-FileCopyrightText: 2026 KDE Developers
+
+ SPDX-License-Identifier: LGPL-2.0-or-later
+*/
+
+#include "ddpclienttest.h"
+
+#include <QJsonObject>
+
+#include "abstractwebsocket.h"
+#include "authenticationmanager/ddpauthenticationmanager.h"
+#include "ddpapi/ddpclient.h"
+
+#include <QTest>
+
+class FakeWebSocket final : public AbstractWebSocket
+{
+public:
+ using AbstractWebSocket::AbstractWebSocket;
+
+ void openUrl(const QUrl &url) override
+ {
+ mRequestUrl = url;
+ }
+
+ [[nodiscard]] qint64 sendTextMessage(const QString &message) override
+ {
+ return message.size();
+ }
+
+ [[nodiscard]] bool isValid() const override
+ {
+ return false;
+ }
+
+ void flush() override
+ {
+ }
+
+ void close() override
+ {
+ }
+
+ [[nodiscard]] QAbstractSocket::SocketError error() const override
+ {
+ return QAbstractSocket::UnknownSocketError;
+ }
+
+ [[nodiscard]] QString errorString() const override
+ {
+ return {};
+ }
+
+ [[nodiscard]] QUrl requestUrl() const override
+ {
+ return mRequestUrl;
+ }
+
+ [[nodiscard]] QWebSocketProtocol::CloseCode closeCode() const override
+ {
+ return mCloseCode;
+ }
+
+ [[nodiscard]] QString closeReason() const override
+ {
+ return {};
+ }
+
+ void ignoreSslErrors() override
+ {
+ }
+
+ [[nodiscard]] QWebSocketProtocol::Version version() const override
+ {
+ return QWebSocketProtocol::VersionLatest;
+ }
+
+ void setCloseCode(QWebSocketProtocol::CloseCode closeCode)
+ {
+ mCloseCode = closeCode;
+ }
+
+private:
+ QUrl mRequestUrl;
+ QWebSocketProtocol::CloseCode mCloseCode = QWebSocketProtocol::CloseCodeNormal;
+};
+
+QTEST_GUILESS_MAIN(DDPClientTest)
+
+void DDPClientTest::closeNotificationSurvivesAuthenticationCleanup_data()
+{
+ QTest::addColumn<int>("closeCode");
+ QTest::addColumn<bool>("normalClose");
+
+ QTest::addRow("normal") << static_cast<int>(QWebSocketProtocol::CloseCodeNormal) << true;
+ QTest::addRow("unexpected") << static_cast<int>(QWebSocketProtocol::CloseCodeAbnormalDisconnection) << false;
+}
+
+void DDPClientTest::closeNotificationSurvivesAuthenticationCleanup()
+{
+ QFETCH(int, closeCode);
+ QFETCH(bool, normalClose);
+
+ DDPClient client;
+ client.setDDPClientAccountParameter(new DDPClient::DDPClientAccountParameter);
+ auto webSocket = new FakeWebSocket;
+ client.mWebSocket = webSocket;
+
+ QObject account;
+ bool reconnectScheduled = false;
+ bool authenticationCleanupRan = false;
+ const auto queueReconnect = [&account, &reconnectScheduled]() {
+ QMetaObject::invokeMethod(
+ &account,
+ [&reconnectScheduled]() {
+ reconnectScheduled = true;
+ },
+ Qt::QueuedConnection);
+ };
+ if (normalClose) {
+ connect(&client, &DDPClient::disconnectedByServer, &account, queueReconnect);
+ } else {
+ connect(&client, &DDPClient::wsClosedSocketError, &account, queueReconnect);
+ }
+
+ connect(client.authenticationManager(),
+ &DDPAuthenticationManager::loginStatusChanged,
+ &account,
+ [&client, &account, &authenticationCleanupRan]() {
+ authenticationCleanupRan = true;
+ disconnect(&client, nullptr, &account, nullptr);
+ });
+
+ webSocket->setCloseCode(static_cast<QWebSocketProtocol::CloseCode>(closeCode));
+ client.onWSclosed();
+
+ QVERIFY(authenticationCleanupRan);
+ QTRY_VERIFY(reconnectScheduled);
+}
+
+#include "moc_ddpclienttest.cpp"
diff --git a/src/core/autotests/ddpclienttest.h b/src/core/autotests/ddpclienttest.h
new file mode 100644
index 0000000000..5bdb3b9a2a
--- /dev/null
+++ b/src/core/autotests/ddpclienttest.h
@@ -0,0 +1,17 @@
+/*
+ SPDX-FileCopyrightText: 2026 KDE Developers
+
+ SPDX-License-Identifier: LGPL-2.0-or-later
+*/
+
+#pragma once
+
+#include <QObject>
+
+class DDPClientTest : public QObject
+{
+ Q_OBJECT
+private Q_SLOTS:
+ void closeNotificationSurvivesAuthenticationCleanup_data();
+ void closeNotificationSurvivesAuthenticationCleanup();
+};
diff --git a/src/core/ddpapi/ddpclient.cpp b/src/core/ddpapi/ddpclient.cpp
index 6b3c357ca9..787ae14523 100644
--- a/src/core/ddpapi/ddpclient.cpp
+++ b/src/core/ddpapi/ddpclient.cpp
@@ -573,20 +573,22 @@ void DDPClient::onSslErrors(const QList<QSslError> &errors)
void DDPClient::onWSclosed()
{
+ mConnected = false;
+ // Notify before changing the authentication state. Account cleanup disconnects
+ // this client's signals as soon as the state becomes LoggedOutAndCleanedUp.
const bool normalClose = mWebSocket->closeCode() == QWebSocketProtocol::CloseCodeNormal;
if (normalClose) {
- qCDebug(RUQOLA_RECONNECT_LOG) << "DDP: Normal close, set status to LoggedOutAndCleanedUp, emit disconnectedByServer";
- authenticationManager()->setLoginStatus(AuthenticationManager::LoggedOutAndCleanedUp);
+ qCDebug(RUQOLA_RECONNECT_LOG) << "DDP: Normal close, emit disconnectedByServer and set status to LoggedOutAndCleanedUp";
Q_EMIT disconnectedByServer();
+ authenticationManager()->setLoginStatus(AuthenticationManager::LoggedOutAndCleanedUp);
} else {
qCWarning(RUQOLA_DDPAPI_LOG) << "WebSocket CLOSED reason:" << mWebSocket->closeReason() << " error: " << mWebSocket->error()
<< " close code : " << mWebSocket->closeCode() << " error string " << mWebSocket->errorString() << "Protocol version"
<< mWebSocket->version();
- authenticationManager()->setLoginStatus(AuthenticationManager::GenericError);
Q_EMIT wsClosedSocketError();
+ authenticationManager()->setLoginStatus(AuthenticationManager::GenericError);
}
- mConnected = false;
Q_EMIT connectedChanged(false);
}
diff --git a/src/core/ddpapi/ddpclient.h b/src/core/ddpapi/ddpclient.h
index 970e0d8615..d6bee152a7 100644
--- a/src/core/ddpapi/ddpclient.h
+++ b/src/core/ddpapi/ddpclient.h
@@ -24,6 +24,7 @@ class RocketChatAccountSettings;
class DDPManager;
class MessageQueue;
class PluginAuthenticationInterface;
+class DDPClientTest;
class LIBRUQOLACORE_EXPORT DDPClient : public QObject
{
Q_OBJECT
@@ -274,4 +275,6 @@ private:
std::unique_ptr<DDPClientAccountParameter> mDDPClientAccountParameter;
QList<qint64> mSubscribeIdentifiers;
bool mLoginEnqueued = false;
+
+ friend class DDPClientTest;
};
diff --git a/src/core/rocketchataccount.cpp b/src/core/rocketchataccount.cpp
index b1b009db15..585f87af72 100644
--- a/src/core/rocketchataccount.cpp
+++ b/src/core/rocketchataccount.cpp
@@ -99,6 +99,7 @@
#include "custom/customuserstatuslistjob.h"
#include <KLocalizedString>
#include <QJsonArray>
+#include <QPointer>
#include <QTimer>
#include <TextEmoticonsCore/EmojiModel>
#include <TextEmoticonsCore/EmojiModelManager>
@@ -672,8 +673,31 @@ DDPClient *RocketChatAccount::ddp()
connect(mDdp.get(), &DDPClient::added, mRocketChatBackend, &RocketChatBackend::slotAdded);
connect(mDdp.get(), &DDPClient::removed, mRocketChatBackend, &RocketChatBackend::slotRemoved);
connect(mDdp.get(), &DDPClient::socketError, this, &RocketChatAccount::socketError);
- connect(mDdp.get(), &DDPClient::disconnectedByServer, this, &RocketChatAccount::slotReconnectToDdpServer);
+ const QPointer<DDPClient> ddpClient(mDdp.get());
+ const auto reconnectDdp = [this, ddpClient]() {
+ // Authentication cleanup disconnects the DDP client from the account, so queue
+ // an account-owned callback before that cleanup starts.
+ QMetaObject::invokeMethod(
+ this,
+ [this, ddpClient]() {
+ // Ignore a delayed notification from a DDP client which has already been replaced.
+ if (!mDdp || mDdp.get() == ddpClient.data()) {
+ slotReconnectToDdpServer();
+ }
+ },
+ Qt::QueuedConnection);
+ };
+ connect(mDdp.get(), &DDPClient::disconnectedByServer, this, reconnectDdp);
+ connect(mDdp.get(), &DDPClient::wsClosedSocketError, this, reconnectDdp);
connect(mDdp.get(), &DDPClient::methodRequested, this, &RocketChatAccount::parseMethodRequested);
+ connect(mDdp.get(), &DDPClient::connectedChanged, this, [this](bool connected) {
+ if (connected) {
+ // A client is connected: cancel any pending reconnect timer. The backoff itself is
+ // only reset once DDP authentication succeeds (see slotDDpLoginStatusChanged).
+ mDdpReconnectScheduled = false;
+ ++mDdpReconnectGeneration;
+ }
+ });
if (mSettings) {
mDdp->setServerUrl(mSettings->serverUrl());
@@ -2313,13 +2337,50 @@ void RocketChatAccount::slotUsersPresenceDone(const QJsonObject &obj)
}
}
-void RocketChatAccount::slotReconnectToDdpServer() // connected to DDPClient::disconnectedByServer
+// Advance an exponential reconnect backoff: 100ms for the first retry, then
+// 1s, doubling up to a 60s ceiling. Shared by the DDP-only and full-server
+// reconnect paths (which keep their own delay state).
+static int nextReconnectDelay(int currentDelay)
+{
+ if (currentDelay == 100) {
+ return 1000;
+ }
+ return qMin(currentDelay * 2, 60000);
+}
+
+void RocketChatAccount::resetDdp()
+{
+ if (mDdp) {
+ disconnect(mDdp.get(), nullptr, this, nullptr);
+ mDdp.release()->deleteLater();
+ }
+}
+
+void RocketChatAccount::slotReconnectToDdpServer()
{
mRoomModel->clear();
- if (mRestApi && mRestApi->authenticationManager()->isLoggedIn()) {
- qCDebug(RUQOLA_RECONNECT_LOG) << debugCategoryAccountName() << "Reconnect only ddpclient";
- ddp()->enqueueLogin();
+ resetDdp();
+
+ if (!mRestApi || !mRestApi->authenticationManager()->isLoggedIn() || mDdpReconnectScheduled) {
+ return;
}
+
+ const int reconnectDelay = mDdpDelayReconnect;
+ mDdpReconnectScheduled = true;
+ const quint64 reconnectGeneration = ++mDdpReconnectGeneration;
+ qCDebug(RUQOLA_RECONNECT_LOG) << debugCategoryAccountName() << "Reconnect DDP client in" << reconnectDelay << "ms";
+ QTimer::singleShot(reconnectDelay, this, [this, reconnectGeneration]() {
+ if (reconnectGeneration != mDdpReconnectGeneration) {
+ return;
+ }
+ mDdpReconnectScheduled = false;
+ if (!mDdp && mRestApi && mRestApi->authenticationManager()->isLoggedIn()) {
+ qCDebug(RUQOLA_RECONNECT_LOG) << debugCategoryAccountName() << "Recreating DDP client";
+ ddp();
+ }
+ });
+
+ mDdpDelayReconnect = nextReconnectDelay(mDdpDelayReconnect);
}
E2eKeyManager *RocketChatAccount::e2eKeyManager() const
@@ -2349,11 +2410,7 @@ void RocketChatAccount::autoReconnectDelayed()
// Let's try connecting in again
QTimer::singleShot(mDelayReconnect, this, [this]() {
qCDebug(RUQOLA_RECONNECT_LOG) << debugCategoryAccountName() << "Attempting to reconnect after the server disconnected us: " << accountName();
- if (mDelayReconnect == 100) {
- mDelayReconnect = 1000;
- } else {
- mDelayReconnect *= 2;
- }
+ mDelayReconnect = nextReconnectDelay(mDelayReconnect);
Q_EMIT displayReconnectWidget(mDelayReconnect / 1000);
reconnectToServer();
});
@@ -2542,10 +2599,16 @@ void RocketChatAccount::slotListCommandDone(const QJsonObject &obj)
void RocketChatAccount::slotDDpLoginStatusChanged()
{
- if (mDdp && mDdp->authenticationManager()->loginStatus() == AuthenticationManager::LoggedOutAndCleanedUp) {
- qCDebug(RUQOLA_RECONNECT_LOG) << debugCategoryAccountName() << "Logged out from DDP, resetting mDdp" << accountName();
- disconnect(mDdp.get(), nullptr, this, nullptr);
- mDdp.release()->deleteLater();
+ if (mDdp) {
+ const auto ddpLoginStatus = mDdp->authenticationManager()->loginStatus();
+ if (ddpLoginStatus == AuthenticationManager::LoggedOutAndCleanedUp) {
+ qCDebug(RUQOLA_RECONNECT_LOG) << debugCategoryAccountName() << "Logged out from DDP, resetting mDdp" << accountName();
+ resetDdp();
+ } else if (ddpLoginStatus == AuthenticationManager::LoggedIn) {
+ // Only reset the backoff once authentication actually succeeds: a server that accepts the
+ // handshake but drops during login would otherwise cause a tight 100ms reconnect loop.
+ mDdpDelayReconnect = 100;
+ }
}
Q_EMIT ddpLoginStatusChanged();
if (!mRestApi && !mDdp) {
diff --git a/src/core/rocketchataccount.h b/src/core/rocketchataccount.h
index 21f6b208fe..9712ef98ae 100644
--- a/src/core/rocketchataccount.h
+++ b/src/core/rocketchataccount.h
@@ -593,6 +593,7 @@ private:
LIBRUQOLACORE_NO_EXPORT void licenseGetModules();
LIBRUQOLACORE_NO_EXPORT void loadSoundFiles();
LIBRUQOLACORE_NO_EXPORT void slotReconnectToDdpServer();
+ LIBRUQOLACORE_NO_EXPORT void resetDdp();
LIBRUQOLACORE_NO_EXPORT void slotVerifyKeysDone();
LIBRUQOLACORE_NO_EXPORT void slotDDpLoginStatusChanged();
LIBRUQOLACORE_NO_EXPORT void slotRESTLoginStatusChanged();
@@ -700,7 +701,10 @@ private:
MemoryManager *const mMemoryManager;
ActionButtonsManager *const mActionButtonsManager;
int mDelayReconnect = 100;
+ int mDdpDelayReconnect = 100;
qint64 mAccountTimeStamp = -1;
+ quint64 mDdpReconnectGeneration = 0;
+ bool mDdpReconnectScheduled = false;
bool mMarkUnreadThreadsAsReadOnNextReply = false;
bool mE2EPasswordMustBeSave = false;
bool mE2EPasswordMustBeDecrypt = false;