Repository: RetroAchievements/RAIntegration
Pull request #994 (2023-04-10 14:59:51)

better error message for 429 error
https://discord.com/channels/310192285306454017/310195377993416714/1094895782223675434

Should now report "HTTP error code: 429 (Too Many Requests)" instead of "JSON Parse Error: Invalid JSON".


Patch:
diff --git a/src/api/impl/ConnectedServer.cpp b/src/api/impl/ConnectedServer.cpp
index 07d42bbf..60e3b1b3 100644
--- a/src/api/impl/ConnectedServer.cpp
+++ b/src/api/impl/ConnectedServer.cpp
@@ -279,7 +279,11 @@ static bool DoRequestWithoutLog(const rc_api_request_t& api_request, _UNUSED con
     switch (pHttpResponse.Content().at(0))
     {
         case '{': // JSON, expected
+            break;
+
         case '<': // HTML, not expected
+            if (HandleHttpError(pHttpResponse.StatusCode(), pResponse))
+                return false;
             break;
 
         default:
diff --git a/src/services/Http.hh b/src/services/Http.hh
index 2c30de74..c05b84ec 100644
--- a/src/services/Http.hh
+++ b/src/services/Http.hh
@@ -16,6 +16,7 @@ public:
         Unauthorized = 401,
         Forbidden = 403,
         NotFound = 404,
+        TooManyRequests = 429,
     };
 
     class Response
diff --git a/src/services/impl/WindowsHttpRequester.cpp b/src/services/impl/WindowsHttpRequester.cpp
index 43fb495c..ba2544ed 100644
--- a/src/services/impl/WindowsHttpRequester.cpp
+++ b/src/services/impl/WindowsHttpRequester.cpp
@@ -348,6 +348,7 @@ std::string WindowsHttpRequester::GetStatusCodeText(unsigned int nStatusCode) co
             case HTTP_STATUS_NOT_SUPPORTED: message = "Not Implemented"; break;
             case HTTP_STATUS_BAD_GATEWAY: message = "Bad Gateway"; break;
             case HTTP_STATUS_SERVICE_UNAVAIL: message = "Service Unavailable"; break;
+            case 429: message = "Too Many Requests"; break;
         }
     }
 


Test patch:
diff --git a/tests/api/ConnectedServer_Tests.cpp b/tests/api/ConnectedServer_Tests.cpp
index 95dcd1d5..d388fbd4 100644
--- a/tests/api/ConnectedServer_Tests.cpp
+++ b/tests/api/ConnectedServer_Tests.cpp
@@ -188,7 +188,6 @@ public:
 
         auto response = server.Login(request);
 
-        // "Success:false" without error message results in Failed call, not Error call
         Assert::AreEqual(ApiResult::Error, response.Result);
         Assert::AreEqual(std::string("HTTP error code: 401 (err401)"), response.ErrorMessage);
         Assert::AreEqual(std::string(""), response.Username);
@@ -239,7 +238,6 @@ public:
 
         auto response = server.AwardAchievement(request);
 
-        // "Success:false" without error message results in Failed call, not Error call
         Assert::AreEqual(ApiResult::Error, response.Result);
         Assert::AreEqual(std::string("HTTP error code: 403 (err403)"), response.ErrorMessage);
     }
@@ -263,11 +261,40 @@ public:
 
         auto response = server.AwardAchievement(request);
 
-        // "Success:false" without error message results in Failed call, not Error call
         Assert::AreEqual(ApiResult::Error, response.Result);
         Assert::AreEqual(std::string("Invalid API token"), response.ErrorMessage);
     }
 
+    TEST_METHOD(TestAwardAchievementFailed429)
+    {
+        MockUserContext mockUserContext;
+        mockUserContext.Initialize("Username", "ApiToken");
+
+        MockHttpRequester mockHttp([]([[maybe_unused]] const Http::Request& /*request*/)
+        {
+            return Http::Response(Http::StatusCode::TooManyRequests,
+                "<html>\n"
+                "<head><title>429 Too Many Requests</title></head>\n"
+                "<body>\n"
+                "<center><h1>429 Too Many Requests</h1></center>\n"
+                "<hr><center>nginx</center>\n"
+                "</body>\n"
+                "</html>");
+        });
+
+        ConnectedServer server("host.com");
+
+        AwardAchievement::Request request;
+        request.GameHash = "HASH";
+        request.AchievementId = 1234U;
+        request.Hardcore = true;
+
+        auto response = server.AwardAchievement(request);
+
+        Assert::AreEqual(ApiResult::Error, response.Result);
+        Assert::AreEqual(std::string("HTTP error code: 429 (err429)"), response.ErrorMessage);
+    }
+
     TEST_METHOD(TestLoginUnknownServer)
     {
         MockHttpRequester mockHttp([]([[maybe_unused]] const Http::Request& /*request*/)