Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(126)

Unified Diff: sync/internal_api/attachments/attachment_downloader_impl.cc

Issue 554743004: Update AttachmentServiceImpl to retry attachment uploads. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Merge with master. Created 6 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: sync/internal_api/attachments/attachment_downloader_impl.cc
diff --git a/sync/internal_api/attachments/attachment_downloader_impl.cc b/sync/internal_api/attachments/attachment_downloader_impl.cc
index d43b3ea1cc276620738d6cd538fd718436580af9..a7a43b42b342d164da2f872453de8d6917920197 100644
--- a/sync/internal_api/attachments/attachment_downloader_impl.cc
+++ b/sync/internal_api/attachments/attachment_downloader_impl.cc
@@ -115,7 +115,7 @@ void AttachmentDownloaderImpl::OnGetTokenFailure(
DownloadState* download_state = *iter;
scoped_refptr<base::RefCountedString> null_attachment_data;
ReportResult(
- *download_state, DOWNLOAD_UNSPECIFIED_ERROR, null_attachment_data);
+ *download_state, DOWNLOAD_TRANSIENT_ERROR, null_attachment_data);
DCHECK(state_map_.find(download_state->attachment_url) != state_map_.end());
state_map_.erase(download_state->attachment_url);
}
@@ -133,22 +133,28 @@ void AttachmentDownloaderImpl::OnURLFetchComplete(
const DownloadState& download_state = *iter->second;
DCHECK(source == download_state.url_fetcher.get());
- DownloadResult result = DOWNLOAD_UNSPECIFIED_ERROR;
+ DownloadResult result = DOWNLOAD_TRANSIENT_ERROR;
scoped_refptr<base::RefCountedString> attachment_data;
- if (source->GetResponseCode() == net::HTTP_OK) {
+ const int response_code = source->GetResponseCode();
+ if (response_code == net::HTTP_OK) {
result = DOWNLOAD_SUCCESS;
std::string data_as_string;
source->GetResponseAsString(&data_as_string);
attachment_data = base::RefCountedString::TakeString(&data_as_string);
- } else if (source->GetResponseCode() == net::HTTP_UNAUTHORIZED) {
+ } else if (response_code == net::HTTP_UNAUTHORIZED) {
+ // Server tells us we've got a bad token so invalidate it.
OAuth2TokenServiceRequest::InvalidateToken(token_service_provider_.get(),
account_id_,
oauth2_scopes_,
download_state.access_token);
- // TODO(pavely): crbug/380437. This is transient error. Request new access
- // token for this DownloadState. The only trick is to do it with exponential
- // backoff.
+ // Fail the request, but indicate that it may be successful if retried.
+ result = DOWNLOAD_TRANSIENT_ERROR;
+ } else if (response_code == net::HTTP_FORBIDDEN) {
+ // User is not allowed to use attachments. Retrying won't help.
+ result = DOWNLOAD_UNSPECIFIED_ERROR;
+ } else if (response_code == net::URLFetcher::RESPONSE_CODE_INVALID) {
+ result = DOWNLOAD_TRANSIENT_ERROR;
}
ReportResult(download_state, result, attachment_data);
state_map_.erase(iter);
@@ -159,6 +165,7 @@ scoped_ptr<net::URLFetcher> AttachmentDownloaderImpl::CreateFetcher(
const std::string& access_token) {
scoped_ptr<net::URLFetcher> url_fetcher(
net::URLFetcher::Create(GURL(url), net::URLFetcher::GET, this));
+ url_fetcher->SetAutomaticallyRetryOn5xx(false);
const std::string auth_header("Authorization: Bearer " + access_token);
url_fetcher->AddExtraRequestHeader(auth_header);
url_fetcher->SetRequestContext(url_request_context_getter_.get());

Powered by Google App Engine
This is Rietveld 408576698