diff --git a/CHANGELOG.md b/CHANGELOG.md index baafb17d..7d1f4518 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ - Major: Newly uploaded Twitch emotes are once again present in emote picker and can be autocompleted with Tab as well. (#2992) - Major: Deprecated `/(un)follow` commands and (un)following in the usercards as Twitch has removed this feature for 3rd party applications. (#3076, #3078) - Major: Added the ability to add nicknames for users. (#137, #2981) -- Major: Work on rate-limiting JOINs and PARTs. (#3112) +- Major: Fixed constant disconnections with more than 20 channels by rate-limiting outgoing JOIN messages. (#3112, #3115) - Minor: Added autocompletion in /whispers for Twitch emotes, Global Bttv/Ffz emotes and emojis. (#2999, #3033) - Minor: Received Twitch messages now use the exact same timestamp (obtained from Twitch's server) for every Chatterino user instead of assuming message timestamp on client's side. (#3021) - Minor: Received IRC messages use `time` message tag for timestamp if it's available. (#3021) diff --git a/chatterino.pro b/chatterino.pro index 63352862..cb5104bb 100644 --- a/chatterino.pro +++ b/chatterino.pro @@ -252,6 +252,7 @@ SOURCES += \ src/util/LayoutHelper.cpp \ src/util/NuulsUploader.cpp \ src/util/RapidjsonHelpers.cpp \ + src/util/RatelimitBucket.cpp \ src/util/SplitCommand.cpp \ src/util/StreamerMode.cpp \ src/util/StreamLink.cpp \ @@ -509,6 +510,7 @@ HEADERS += \ src/util/rangealgorithm.hpp \ src/util/RapidjsonHelpers.hpp \ src/util/RapidJsonSerializeQString.hpp \ + src/util/RatelimitBucket.hpp \ src/util/RemoveScrollAreaBackground.hpp \ src/util/SampleCheerMessages.hpp \ src/util/SampleLinks.hpp \ @@ -583,6 +585,7 @@ HEADERS += \ src/widgets/settingspages/IgnoresPage.hpp \ src/widgets/settingspages/KeyboardSettingsPage.hpp \ src/widgets/settingspages/ModerationPage.hpp \ + src/widgets/settingspages/NicknamesPage.hpp \ src/widgets/settingspages/NotificationPage.hpp \ src/widgets/settingspages/SettingsPage.hpp \ src/widgets/splits/ClosedSplits.hpp \ diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 31d7a1a9..024e6ff6 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -292,6 +292,8 @@ set(SOURCE_FILES util/NuulsUploader.hpp util/RapidjsonHelpers.cpp util/RapidjsonHelpers.hpp + util/RatelimitBucket.cpp + util/RatelimitBucket.hpp util/SplitCommand.cpp util/SplitCommand.hpp util/StreamLink.cpp diff --git a/src/providers/irc/AbstractIrcServer.cpp b/src/providers/irc/AbstractIrcServer.cpp index 3dfb657c..0ccebdec 100644 --- a/src/providers/irc/AbstractIrcServer.cpp +++ b/src/providers/irc/AbstractIrcServer.cpp @@ -15,6 +15,10 @@ const int RECONNECT_BASE_INTERVAL = 2000; // 60 falloff counter means it will try to reconnect at most every 60*2 seconds const int MAX_FALLOFF_COUNTER = 60; +// Ratelimits for joinBucket_ +const int JOIN_RATELIMIT_BUDGET = 18; +const int JOIN_RATELIMIT_COOLDOWN = 10500; + AbstractIrcServer::AbstractIrcServer() { // Initialize the connections @@ -23,6 +27,17 @@ AbstractIrcServer::AbstractIrcServer() this->writeConnection_->moveToThread( QCoreApplication::instance()->thread()); + // Apply a leaky bucket rate limiting to JOIN messages + auto actuallyJoin = [&](QString message) { + if (!this->channels.contains(message)) + { + return; + } + this->readConnection_->sendRaw("JOIN #" + message); + }; + this->joinBucket_.reset(new RatelimitBucket( + JOIN_RATELIMIT_BUDGET, JOIN_RATELIMIT_COOLDOWN, actuallyJoin, this)); + QObject::connect(this->writeConnection_.get(), &Communi::IrcConnection::messageReceived, this, [this](auto msg) { @@ -224,7 +239,7 @@ ChannelPtr AbstractIrcServer::getOrAddChannel(const QString &dirtyChannelName) { if (this->readConnection_->isConnected()) { - this->readConnection_->sendRaw("JOIN #" + channelName); + this->joinBucket_->send(channelName); } } } @@ -284,7 +299,7 @@ void AbstractIrcServer::onReadConnected(IrcConnection *connection) { if (auto channel = weak.lock()) { - connection->sendRaw("JOIN #" + channel->getName()); + this->joinBucket_->send(channel->getName()); } } diff --git a/src/providers/irc/AbstractIrcServer.hpp b/src/providers/irc/AbstractIrcServer.hpp index 7cfc38f5..40da3ba2 100644 --- a/src/providers/irc/AbstractIrcServer.hpp +++ b/src/providers/irc/AbstractIrcServer.hpp @@ -8,6 +8,7 @@ #include "common/Common.hpp" #include "providers/irc/IrcConnection2.hpp" +#include "util/RatelimitBucket.hpp" namespace chatterino { @@ -88,6 +89,10 @@ private: QObjectPtr writeConnection_ = nullptr; QObjectPtr readConnection_ = nullptr; + // Our rate limiting bucket for the Twitch join rate limits + // https://dev.twitch.tv/docs/irc/guide#rate-limits + QObjectPtr joinBucket_; + QTimer reconnectTimer_; int falloffCounter_ = 1; diff --git a/src/util/RatelimitBucket.cpp b/src/util/RatelimitBucket.cpp new file mode 100644 index 00000000..c33f3a30 --- /dev/null +++ b/src/util/RatelimitBucket.cpp @@ -0,0 +1,45 @@ +#include "RatelimitBucket.hpp" + +#include + +namespace chatterino { + +RatelimitBucket::RatelimitBucket(int budget, int cooldown, + std::function callback, + QObject *parent) + : QObject(parent) + , budget_(budget) + , cooldown_(cooldown) + , callback_(callback) +{ +} + +void RatelimitBucket::send(QString channel) +{ + this->queue_.append(channel); + + if (this->budget_ > 0) + { + this->handleOne(); + } +} + +void RatelimitBucket::handleOne() +{ + if (queue_.isEmpty()) + { + return; + } + + auto item = queue_.takeFirst(); + + this->budget_--; + callback_(item); + + QTimer::singleShot(cooldown_, this, [this] { + this->budget_++; + this->handleOne(); + }); +} + +} // namespace chatterino diff --git a/src/util/RatelimitBucket.hpp b/src/util/RatelimitBucket.hpp new file mode 100644 index 00000000..89ecdc57 --- /dev/null +++ b/src/util/RatelimitBucket.hpp @@ -0,0 +1,40 @@ +#pragma once + +#include +#include +#include + +namespace chatterino { + +class RatelimitBucket : public QObject +{ +public: + RatelimitBucket(int budget, int cooldown, + std::function callback, QObject *parent); + + void send(QString channel); + +private: + /** + * @brief budget_ denotes the amount of calls that can be handled before we need to wait for the cooldown + **/ + int budget_; + + /** + * @brief This is the amount of time in milliseconds it takes for one used up budget to be put back into the bucket for use elsewhere + **/ + const int cooldown_; + + std::function callback_; + QList queue_; + + /** + * @brief Run the callback on one entry in the queue. + * + * This will start a timer that runs after cooldown_ milliseconds that + * gives back one "token" to the bucket and calls handleOne again. + **/ + void handleOne(); +}; + +} // namespace chatterino diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index e7e2c96b..74474c2e 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -11,6 +11,7 @@ set(test_SOURCES ${CMAKE_CURRENT_LIST_DIR}/src/ExponentialBackoff.cpp ${CMAKE_CURRENT_LIST_DIR}/src/TwitchAccount.cpp ${CMAKE_CURRENT_LIST_DIR}/src/Helpers.cpp + ${CMAKE_CURRENT_LIST_DIR}/src/RatelimitBucket.cpp ) add_executable(${PROJECT_NAME} ${test_SOURCES}) diff --git a/tests/src/RatelimitBucket.cpp b/tests/src/RatelimitBucket.cpp new file mode 100644 index 00000000..c92a4223 --- /dev/null +++ b/tests/src/RatelimitBucket.cpp @@ -0,0 +1,46 @@ +#include "util/RatelimitBucket.hpp" + +#include +#include +#include +#include + +#include +#include + +using namespace chatterino; + +TEST(RatelimitBucket, BatchTwoParts) +{ + const int cooldown = 100; + int n = 0; + auto cb = [&n](QString msg) { + qDebug() << msg; + ++n; + }; + auto bucket = std::make_unique(5, cooldown, cb, nullptr); + bucket->send("1"); + EXPECT_EQ(n, 1); + + bucket->send("2"); + EXPECT_EQ(n, 2); + + bucket->send("3"); + EXPECT_EQ(n, 3); + + bucket->send("4"); + EXPECT_EQ(n, 4); + + bucket->send("5"); + EXPECT_EQ(n, 5); + + bucket->send("6"); + // Rate limit reached, n will not have changed yet. If we wait for the cooldown to run, n should have changed + EXPECT_EQ(n, 5); + + QCoreApplication::processEvents(); + std::this_thread::sleep_for(std::chrono::milliseconds{cooldown}); + QCoreApplication::processEvents(); + + EXPECT_EQ(n, 6); +}