diff --git a/CHANGELOG.md b/CHANGELOG.md index 587e24f7868..574f3ba6288 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ - Bugfix: Fixed global badges not showing in anonymous mode. (#5599) - Bugfix: Fixed grammar in the user highlight page. (#5602) - Bugfix: Fixed incorrect message being disabled in some cases upon approving or denying an automod caught message. (#5611) +- Bugfix: Fixed network requests timing out despite them not being in flight for that long, for Qt 6.3+ where we have the technology. (#5729) - Bugfix: Fixed double-click selection not working when clicking outside a message. (#5617) - Bugfix: Fixed a potential rare crash that could occur on Windows if a toast was about to fire just as we were shutting down. (#5728) - Bugfix: Fixed emotes starting with ":" not tab-completing. (#5603) diff --git a/src/common/network/NetworkTask.cpp b/src/common/network/NetworkTask.cpp index bebe3405123..7bc1330c6b3 100644 --- a/src/common/network/NetworkTask.cpp +++ b/src/common/network/NetworkTask.cpp @@ -30,22 +30,35 @@ NetworkTask::~NetworkTask() void NetworkTask::run() { + this->reply_ = this->createReply(); + if (!this->reply_) + { + this->deleteLater(); + return; + } + const auto &timeout = this->data_->timeout; if (timeout.has_value()) { +#if QT_VERSION >= QT_VERSION_CHECK(6, 3, 0) + QObject::connect(this->reply_, &QNetworkReply::requestSent, this, + [this]() { + const auto &timeout = this->data_->timeout; + this->timer_ = new QTimer(this); + this->timer_->setSingleShot(true); + this->timer_->start(timeout.value()); + QObject::connect(this->timer_, &QTimer::timeout, + this, &NetworkTask::timeout); + }); +#else this->timer_ = new QTimer(this); this->timer_->setSingleShot(true); this->timer_->start(timeout.value()); QObject::connect(this->timer_, &QTimer::timeout, this, &NetworkTask::timeout); +#endif } - this->reply_ = this->createReply(); - if (!this->reply_) - { - this->deleteLater(); - return; - } QObject::connect(this->reply_, &QNetworkReply::finished, this, &NetworkTask::finished); } diff --git a/tests/src/NetworkHelpers.hpp b/tests/src/NetworkHelpers.hpp index f55355e4a54..7125deb7da3 100644 --- a/tests/src/NetworkHelpers.hpp +++ b/tests/src/NetworkHelpers.hpp @@ -1,7 +1,15 @@ #pragma once + #include "Test.hpp" #include +#include +#include + +#include +#include +#include + namespace chatterino { #ifdef CHATTERINO_TEST_USE_PUBLIC_HTTPBIN @@ -52,4 +60,5 @@ class RequestWaiter std::condition_variable condition_; bool requestDone_ = false; }; + } // namespace chatterino diff --git a/tests/src/NetworkRequest.cpp b/tests/src/NetworkRequest.cpp index 44b7504c7d3..a81059e4841 100644 --- a/tests/src/NetworkRequest.cpp +++ b/tests/src/NetworkRequest.cpp @@ -231,3 +231,49 @@ TEST(NetworkRequest, FinallyCallbackOnTimeout) EXPECT_FALSE(onSuccessCalled); EXPECT_TRUE(NetworkManager::workerThread->isRunning()); } + +/// Ensure timeouts don't expire early just because their request took a bit longer to actually fire +/// +/// We need to ensure all requests are "executed" before we start waiting for them +TEST(NetworkRequest, BatchedTimeouts) +{ +#if QT_VERSION >= QT_VERSION_CHECK(6, 3, 0) + // roughly num network manager worker threads * 2 + static const auto numRequests = 10; + + struct RequestState { + RequestWaiter waiter; + bool errored = false; + }; + + EXPECT_TRUE(NetworkManager::workerThread->isRunning()); + + std::vector> states; + + for (auto i = 1; i <= numRequests; ++i) + { + auto state = std::make_shared(); + + auto url = getDelayURL(1); + + NetworkRequest(url) + .timeout(1500) + .onError([=](const NetworkResult &result) { + (void)result; + state->errored = true; + }) + .finally([=] { + state->waiter.requestDone(); + }) + .execute(); + + states.emplace_back(state); + } + + for (const auto &state : states) + { + state->waiter.waitForRequest(); + EXPECT_FALSE(state->errored); + } +#endif +}