Commit 7d4f8377 authored by Owen Min's avatar Owen Min Committed by Commit Bot

Handle request_too_large error for reporting

If request it too large for the server, we move to the next request
without retry. If there is no more request, we notifies the accomplishment
to the caller as they are succeeded.

Bug: 956237
Change-Id: Ie157c8b8d884d345d4212bbc86b8ceaa76428de1
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1639238Reviewed-by: default avatarMarc-André Decoste <mad@chromium.org>
Commit-Queue: Owen Min <zmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#665680}
parent e670f77e
......@@ -52,14 +52,7 @@ void ReportUploader::Upload() {
void ReportUploader::OnRequestFinished(bool status) {
if (status) {
// We don't reset the backoff in case there are multiple requests in a row
// and we don't start from 1 minute again.
backoff_entry_.InformOfRequest(true);
requests_.pop();
if (requests_.size() == 0)
SendResponse(ReportStatus::kSuccess);
else
Upload();
NextRequest();
return;
}
......@@ -74,8 +67,13 @@ void ReportUploader::OnRequestFinished(bool status) {
case policy::DM_STATUS_SERVICE_DEVICE_ID_CONFLICT:
Retry();
break;
case policy::DM_STATUS_REQUEST_TOO_LARGE:
// Treats the REQUEST_TOO_LARGE error as a success upload. It's likely
// a calculation error during request generating and there is nothing
// can be done here.
NextRequest();
break;
default:
// TODO(zmin): Handle error 413.
SendResponse(ReportStatus::kPersistentError);
break;
}
......@@ -101,4 +99,16 @@ void ReportUploader::SendResponse(const ReportStatus status) {
std::move(callback_).Run(status);
}
void ReportUploader::NextRequest() {
// We don't reset the backoff in case there are multiple requests in a row
// and we don't start from 1 minute again.
backoff_entry_.InformOfRequest(true);
requests_.pop();
if (requests_.size() == 0)
SendResponse(ReportStatus::kSuccess);
else
Upload();
return;
}
} // namespace enterprise_reporting
......@@ -42,8 +42,6 @@ class ReportUploader {
// Request upload result.
enum ReportStatus {
kSuccess,
kReportTooLarge, // Report can't be uploaded due to its size. Usually it's
// caused by our calculation error.
kTransientError, // Report can't be uploaded due to transient error like
// network error or server side error.
kPersistentError, // Report can't be uploaded due to persistent error like
......@@ -78,6 +76,9 @@ class ReportUploader {
// Notifies the upload result.
void SendResponse(const ReportStatus status);
// Moves to the next request if exist, or notifies the accomplishments.
void NextRequest();
policy::CloudPolicyClient* client_;
ReportCallback callback_;
std::queue<std::unique_ptr<em::ChromeDesktopReportRequest>> requests_;
......
......@@ -114,6 +114,20 @@ TEST_F(ReportUploaderTest, PersistentError) {
::testing::Mock::VerifyAndClearExpectations(&client_);
}
TEST_F(ReportUploaderTest, RequestTooBigError) {
CreateUploader(/* *retyr_count = */ 2);
EXPECT_CALL(client_, UploadChromeDesktopReportProxy(_, _))
.Times(2)
.WillOnce(WithArgs<1>(policy::ScheduleStatusCallback(false)))
.WillOnce(WithArgs<1>(policy::ScheduleStatusCallback(false)));
client_.SetStatus(policy::DM_STATUS_REQUEST_TOO_LARGE);
UploadReportAndSetExpectation(/*number_of_request=*/2,
ReportUploader::kSuccess);
RunNextTask();
EXPECT_TRUE(has_responded_);
::testing::Mock::VerifyAndClearExpectations(&client_);
}
TEST_F(ReportUploaderTest, RetryAndSuccess) {
EXPECT_CALL(client_, UploadChromeDesktopReportProxy(_, _))
.Times(2)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment