Api request timeout should always be longer than long poll timeout.

Otherwise, the api request will timeout before long polling finishes.
baderouaich · May 1, 2024 · 0a0154e · 0a0154e
1 parent a351657
commit 0a0154e
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 41 deletions.
diff --git a/src/Api.cpp b/src/Api.cpp
@@ -2435,9 +2435,11 @@ const std::string& Api::getUrl() const noexcept {
 }
 
 void Api::setLongPollTimeout(const cpr::Timeout& longPollTimeout) {
-  if (longPollTimeout.ms > m_timeout.ms)
-    throw Exception("Api::setLongPollTimeout: Long poll timeout should always be shorter than api request timeout."
-                    " Otherwise the api request will time out before long polling finishes.");
+  using namespace std::chrono_literals;
+  if (longPollTimeout.ms >= m_timeout.ms)
+    // Api request timeout should always be longer than long poll timeout.
+    // Otherwise, the api request will time out before long polling finishes
+    m_timeout.ms = longPollTimeout.ms + 1s;
   m_longPollTimeout = longPollTimeout;
 }
 cpr::Timeout Api::getLongPollTimeout() const noexcept { return m_longPollTimeout; }
@@ -2448,10 +2450,13 @@ void Api::setConnectTimeout(const cpr::ConnectTimeout& timeout) noexcept {
 cpr::ConnectTimeout Api::getConnectTimeout() const noexcept { return m_connectTimeout; }
 
 void Api::setTimeout(const cpr::Timeout& timeout) {
-  if (timeout.ms <= m_longPollTimeout.ms)
-    throw Exception("Api::setTimeout: Api request timeout should always be longer than long poll timeout."
-                    " Otherwise the api request will time out before long polling finishes.");
-  m_timeout = timeout;
+  using namespace std::chrono_literals;
+  cpr::Timeout newTimeout{timeout.ms};
+  if (newTimeout.ms <= m_longPollTimeout.ms)
+    // Api request timeout should always be longer than long poll timeout.
+    // Otherwise, the api request will time out before long polling finishes.
+    newTimeout.ms = m_longPollTimeout.ms + 1s;
+  m_timeout = newTimeout;
 }
 cpr::Timeout Api::getTimeout() const noexcept { return m_timeout; }
 

diff --git a/tests/src/TestApi.cpp b/tests/src/TestApi.cpp
@@ -1,47 +1,44 @@
 #include <chrono>
 #define CATCH_CONFIG_MAIN
-#include <tgbotxx/tgbotxx.hpp>
 #include <catch2/catch.hpp>
+#include <tgbotxx/tgbotxx.hpp>
 using namespace tgbotxx;
 
 thread_local static Ptr<Api> API(new Api(std::getenv("TESTS_BOT_TOKEN") ?: "BOT_TOKEN"));
 
 
-TEST_CASE("Test Api", "methods")
-{
-    SECTION("getMe") {
-        Ptr<User> me = API->getMe();
-        REQUIRE(me);
-        REQUIRE(me->isBot);
-        REQUIRE(not me->username.empty());
-        REQUIRE(not me->firstName.empty());
-        REQUIRE(me->id != 0);
-    }
-
-    SECTION("deleteWebhook") {
-        REQUIRE(API->deleteWebhook(false));
-        REQUIRE(API->deleteWebhook(true));
-    }
-
-
-    SECTION("timeouts") {
-      cpr::Timeout longPollTimeout = std::chrono::seconds(60);
-      cpr::Timeout timeout = std::chrono::seconds(70); // Must be longer than long polling timeout
-      API->setLongPollTimeout(longPollTimeout);
-      API->setTimeout(timeout);
-
-      // Getters & setters
-      REQUIRE(timeout.ms == API->getTimeout().ms);
-      REQUIRE(longPollTimeout.ms == API->getLongPollTimeout().ms);
+TEST_CASE("Test Api", "methods") {
+  SECTION("getMe") {
+    Ptr<User> me = API->getMe();
+    REQUIRE(me);
+    REQUIRE(me->isBot);
+    REQUIRE(not me->username.empty());
+    REQUIRE(not me->firstName.empty());
+    REQUIRE(me->id != 0);
+  }
 
-      // Try set a timeout that is less than long polling timeout
-      REQUIRE_THROWS(API->setTimeout(std::chrono::seconds(30)));
-      // .. or vice versa
-      REQUIRE_THROWS(API->setLongPollTimeout(std::chrono::seconds(80)));
-      REQUIRE_THROWS_AS(API->setLongPollTimeout(std::chrono::seconds(80)), Exception);
+  SECTION("deleteWebhook") {
+    REQUIRE(API->deleteWebhook(false));
+    REQUIRE(API->deleteWebhook(true));
+  }
 
+  SECTION("timeouts") {
+    std::srand(std::time(nullptr));
+    for (int i = 0; i < 100; i++) {
+      cpr::Timeout longPollTimeout = std::chrono::seconds(std::rand() % 1'000);
+      cpr::Timeout timeout = std::chrono::seconds(std::rand() % 1'000);
+      if (!!(std::rand() % 2)) {
+        API->setLongPollTimeout(longPollTimeout);
+        API->setTimeout(timeout);
+      } else {
+        API->setTimeout(timeout);
+        API->setLongPollTimeout(longPollTimeout);
+      }
+      // Api request timeout should always be longer than long poll timeout.
+      // Otherwise, the api request will time out before long polling finishes.
+      REQUIRE(API->getLongPollTimeout().ms < API->getTimeout().ms);
     }
-
+  }
 }
 
 TEST_CASE("Test Api ErrorCodes", "ErrorCode") {
@@ -50,7 +47,7 @@ TEST_CASE("Test Api ErrorCodes", "ErrorCode") {
       auto invalid_chat_id = 0;
       API->sendMessage(invalid_chat_id, "test msg");
     } catch (const tgbotxx::Exception& e) {
-      std::cerr << e.what() << " (" << (int)e.errorCode() << ") " << std::endl;
+      std::cerr << e.what() << " (" << (int) e.errorCode() << ") " << std::endl;
       REQUIRE(e.errorCode() == ErrorCode::BAD_REQUEST);
       REQUIRE(std::string(e.what()) == "Bad Request: chat not found");
     }