Skip to content

Commit

Permalink
attempt to gracefully return 429 errors when uploading assets (#1118)
Browse files Browse the repository at this point in the history
  • Loading branch information
Jamiras authored Aug 27, 2024
1 parent d1786c0 commit 2781b96
Show file tree
Hide file tree
Showing 4 changed files with 172 additions and 5 deletions.
23 changes: 20 additions & 3 deletions src/api/impl/ConnectedServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,12 @@ _NODISCARD static bool GetJson([[maybe_unused]] _In_ const char* sApiName,
if (pDocument.HasMember("Error"))
{
pResponse.ErrorMessage = pDocument["Error"].GetString();
if (httpResponse.StatusCode() == ra::services::Http::StatusCode::TooManyRequests)
{
pResponse.Result = ApiResult::Incomplete;
return false;
}

if (!pResponse.ErrorMessage.empty())
{
pResponse.Result = ApiResult::Error;
Expand Down Expand Up @@ -382,15 +388,24 @@ static bool ValidateResponse(int nResult, const rc_api_response_t& api_response,
if (api_response.error_message && *api_response.error_message)
{
pResponse.ErrorMessage = api_response.error_message;
pResponse.Result = ApiResult::Error;
RA_LOG_ERR("-- %s Error: %s", sApiName, pResponse.ErrorMessage);

if (nStatusCode == ra::services::Http::StatusCode::TooManyRequests)
{
pResponse.Result = ApiResult::Incomplete;
}
else
{
pResponse.Result = ApiResult::Error;
RA_LOG_ERR("-- %s Error: %s", sApiName, pResponse.ErrorMessage);
}
}
else
{
pResponse.Result = ApiResult::Failed;
RA_LOG_ERR("-- %s Error: Success=false", sApiName, pResponse.ErrorMessage);
}


return false;
}

Expand Down Expand Up @@ -1075,7 +1090,9 @@ UploadBadge::Response ConnectedServer::UploadBadge(const UploadBadge::Request& r
{
if (!document.HasMember("Response"))
{
response.Result = ApiResult::Error;
if (response.Result == ApiResult::None)
response.Result = ApiResult::Error;

if (response.ErrorMessage.empty())
response.ErrorMessage = ra::StringPrintf("%s not found in response", "Response");
}
Expand Down
37 changes: 35 additions & 2 deletions src/ui/viewmodels/AssetUploadViewModel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,13 +183,22 @@ void AssetUploadViewModel::UploadBadge(const std::wstring& sBadge)
ra::api::UploadBadge::Request request;
request.ImageFilePath = sFilename;

std::vector<ra::data::models::AchievementModel*> vAffectedAchievements;
ra::api::UploadBadge::Response response;
{
// only allow one badge upload at a time to prevent a race condition on server that could
// result in non-unique image IDs being returned.
std::lock_guard<std::mutex> pLock(m_pMutex);
const auto response = request.Call();
response = request.Call();
}

if (response.Result == ra::api::ApiResult::Incomplete)
{
Rest();
UploadBadge(sBadge);
return;
}
std::vector<ra::data::models::AchievementModel*> vAffectedAchievements;
{
// if the upload succeeded, update the badge property for each associated achievement and queue the
// achievement update. if the upload failed, set the error message and don't queue the achievement update.
for (auto& pItem : m_vUploadQueue)
Expand Down Expand Up @@ -278,6 +287,12 @@ void AssetUploadViewModel::UploadAchievement(ra::data::models::AchievementModel&
pAchievement.UpdateLocalCheckpoint();
pAchievement.UpdateServerCheckpoint();
}
else if (response.Result == ra::api::ApiResult::Incomplete)
{
Rest();
UploadAchievement(pAchievement);
return;
}

// update the queue
std::lock_guard<std::mutex> pLock(m_pMutex);
Expand Down Expand Up @@ -322,6 +337,12 @@ void AssetUploadViewModel::UploadLeaderboard(ra::data::models::LeaderboardModel&
pLeaderboard.UpdateLocalCheckpoint();
pLeaderboard.UpdateServerCheckpoint();
}
else if (response.Result == ra::api::ApiResult::Incomplete)
{
Rest();
UploadLeaderboard(pLeaderboard);
return;
}

// update the queue
std::lock_guard<std::mutex> pLock(m_pMutex);
Expand Down Expand Up @@ -355,6 +376,12 @@ void AssetUploadViewModel::UploadCodeNote(ra::data::models::CodeNotesModel& pNot
pNotes.SetServerCodeNote(nAddress, L"");
nState = UploadState::Success;
}
else if (response.Result == ra::api::ApiResult::Incomplete)
{
Rest();
UploadCodeNote(pNotes, nAddress);
return;
}

sErrorMessage = response.ErrorMessage;
}
Expand All @@ -372,6 +399,12 @@ void AssetUploadViewModel::UploadCodeNote(ra::data::models::CodeNotesModel& pNot
pNotes.SetServerCodeNote(nAddress, *pNote);
nState = UploadState::Success;
}
else if (response.Result == ra::api::ApiResult::Incomplete)
{
Rest();
UploadCodeNote(pNotes, nAddress);
return;
}

sErrorMessage = response.ErrorMessage;
}
Expand Down
7 changes: 7 additions & 0 deletions src/ui/viewmodels/AssetUploadViewModel.hh
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ public:
protected:
void OnBegin() override;

virtual void Rest() const noexcept
{
// sleep 500-1500ms to allow server to stop throttling us.
// stagger value so we don't slam it in batches.
Sleep(rand() % 1000 + 500);
}

private:
enum class UploadState
{
Expand Down
110 changes: 110 additions & 0 deletions tests/ui/viewmodels/AssetUploadViewModel_Tests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,9 @@ TEST_CLASS(AssetUploadViewModel_Tests)
Assert::IsTrue(bDialogSeen);
}

protected:
void Rest() const noexcept override {}

private:
ra::data::context::GameAssets m_pAssets;
};
Expand Down Expand Up @@ -707,6 +710,68 @@ TEST_CLASS(AssetUploadViewModel_Tests)
vmUpload.AssertSuccess(2);
}

TEST_METHOD(TestMultipleCoreAchievementsWithImages429)
{
AssetUploadViewModelHarness vmUpload;
auto& pAchievement1 = vmUpload.AddAchievement(AssetCategory::Core, 5, L"Title1", L"Desc1", L"local\\12345", "0xH1234=1");
auto& pAchievement2 = vmUpload.AddAchievement(AssetCategory::Core, 5, L"Title2", L"Desc2", L"local\\22222", "0xH1234=1");
Assert::AreEqual(AssetChanges::Unpublished, pAchievement1.GetChanges());
Assert::AreEqual(AssetChanges::Unpublished, pAchievement2.GetChanges());

vmUpload.QueueAsset(pAchievement1);
vmUpload.QueueAsset(pAchievement2);
Assert::AreEqual({ 2U }, vmUpload.TaskCount());

int nImagesUploaded = 0;
vmUpload.mockServer.HandleRequest<ra::api::UploadBadge>([&nImagesUploaded]
(const ra::api::UploadBadge::Request& pRequest, ra::api::UploadBadge::Response& pResponse)
{
++nImagesUploaded;

if (pRequest.ImageFilePath == L"RACache\\Badges\\local\\12345")
pResponse.BadgeId = "76543";
else
pResponse.BadgeId = "55555";

if (nImagesUploaded % 2 == 0)
pResponse.Result = ra::api::ApiResult::Incomplete;
else
pResponse.Result = ra::api::ApiResult::Success;
return true;
});

int nAchievementsUploaded = 0;
vmUpload.mockServer.HandleRequest<ra::api::UpdateAchievement>([&nAchievementsUploaded]
(const ra::api::UpdateAchievement::Request& pRequest, ra::api::UpdateAchievement::Response& pResponse)
{
++nAchievementsUploaded;

if (pRequest.Title == L"Title1")
Assert::AreEqual(std::string("76543"), pRequest.Badge);
else
Assert::AreEqual(std::string("55555"), pRequest.Badge);

pResponse.AchievementId = pRequest.AchievementId;

if (nAchievementsUploaded % 2 == 0)
pResponse.Result = ra::api::ApiResult::Incomplete;
else
pResponse.Result = ra::api::ApiResult::Success;
return true;
});

vmUpload.DoUpload();

Assert::AreEqual(3, nImagesUploaded);
Assert::AreEqual(3, nAchievementsUploaded);
Assert::AreEqual(AssetChanges::None, pAchievement1.GetChanges());
Assert::AreEqual(std::wstring(L"76543"), pAchievement1.GetBadge());
Assert::AreEqual(AssetChanges::None, pAchievement2.GetChanges());
Assert::AreEqual(std::wstring(L"55555"), pAchievement2.GetBadge());

vmUpload.AssertSuccess(2);
}

TEST_METHOD(TestMultipleCoreAchievementsWithSameImage)
{
AssetUploadViewModelHarness vmUpload;
Expand Down Expand Up @@ -1220,6 +1285,51 @@ TEST_CLASS(AssetUploadViewModel_Tests)

vmUpload.AssertSuccess(2);
}

TEST_METHOD(TestMultipleCodeNotes429)
{
AssetUploadViewModelHarness vmUpload;
vmUpload.CodeNotes().SetCodeNote(0x1234, L"This is a note.");
vmUpload.CodeNotes().SetCodeNote(0x1235, L"This is another note.");
Assert::AreEqual(AssetChanges::Unpublished, vmUpload.CodeNotes().GetChanges());

vmUpload.QueueAsset(vmUpload.CodeNotes());
Assert::AreEqual({2U}, vmUpload.TaskCount());

int nApiCount = 0;
vmUpload.mockServer.HandleRequest<ra::api::UpdateCodeNote>(
[&nApiCount](const ra::api::UpdateCodeNote::Request& pRequest,
ra::api::UpdateCodeNote::Response& pResponse) {
nApiCount++;
Assert::AreEqual(AssetUploadViewModelHarness::GameId, pRequest.GameId);
if (pRequest.Address == 0x1234U)
{
Assert::AreEqual(0x1234U, pRequest.Address);
Assert::AreEqual(std::wstring(L"This is a note."), pRequest.Note);
}
else
{
Assert::AreEqual(0x1235U, pRequest.Address);
Assert::AreEqual(std::wstring(L"This is another note."), pRequest.Note);
}

if (nApiCount % 2 == 0)
pResponse.Result = ra::api::ApiResult::Incomplete;
else
pResponse.Result = ra::api::ApiResult::Success;
return true;
});

vmUpload.DoUpload();

// 0 = 1234, success
// 1 = 1235, delayed
// 2 = 1235, success
Assert::AreEqual(3, nApiCount);
Assert::AreEqual(AssetChanges::None, vmUpload.CodeNotes().GetChanges());

vmUpload.AssertSuccess(2);
}
};

} // namespace tests
Expand Down

0 comments on commit 2781b96

Please sign in to comment.