Retry uploading in UploadJobImpl when error occurs

chrome/browser/chromeos/policy/upload_job_impl.cc

 // Copyright (c) 2015 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "chrome/browser/chromeos/policy/upload_job_impl.h"
 
 #include <stddef.h>
 #include <set>
 #include <utility>
 
+#include "base/location.h"
 #include "base/logging.h"
 #include "base/macros.h"
 #include "base/strings/stringprintf.h"
 #include "google_apis/gaia/gaia_constants.h"
 #include "google_apis/gaia/google_service_auth_error.h"
 #include "net/base/mime_util.h"
 #include "net/http/http_status_code.h"
 #include "net/url_request/url_request_status.h"
 
 namespace policy {
 
 namespace {
 
 // Format for bearer tokens in HTTP requests to access OAuth 2.0 protected
 // resources.
 const char kAuthorizationHeaderFormat[] = "Authorization: Bearer %s";
 
 // Value the "Content-Type" field will be set to in the POST request.
 const char kUploadContentType[] = "multipart/form-data";
 
-// Number of upload retries.
-const int kMaxRetries = 1;
+// Number of upload attempts.
+const int kMaxAttempts = 4;
 
 // Max size of MIME boundary according to RFC 1341, section 7.2.1.
 const size_t kMaxMimeBoundarySize = 70;
 
+// Delay after each unsuccessful upload attempt.
+const long kRetryDelayMs = 25000;
+
 }  // namespace
 
 UploadJobImpl::Delegate::~Delegate() {
 }
 
 UploadJobImpl::MimeBoundaryGenerator::~MimeBoundaryGenerator() {
 }
 
 UploadJobImpl::RandomMimeBoundaryGenerator::~RandomMimeBoundaryGenerator() {
 }
@@ skipping 73 matching lines @@
     const {
   return net::GenerateMimeMultipartBoundary();
 }
 
 UploadJobImpl::UploadJobImpl(
     const GURL& upload_url,
     const std::string& account_id,
     OAuth2TokenService* token_service,
     scoped_refptr<net::URLRequestContextGetter> url_context_getter,
     Delegate* delegate,
-    std::unique_ptr<MimeBoundaryGenerator> boundary_generator)
+    std::unique_ptr<MimeBoundaryGenerator> boundary_generator,
+    scoped_refptr<base::SequencedTaskRunner> task_runner)
     : OAuth2TokenService::Consumer("cros_upload_job"),
       upload_url_(upload_url),
       account_id_(account_id),
       token_service_(token_service),
       url_context_getter_(url_context_getter),
       delegate_(delegate),
       boundary_generator_(std::move(boundary_generator)),
       state_(IDLE),
-      retry_(0) {
+      retry_(0),
+      task_runner_(task_runner),
+      weak_factory_(this) {
   DCHECK(token_service_);
   DCHECK(url_context_getter_);
   DCHECK(delegate_);
   if (!upload_url_.is_valid()) {
     state_ = ERROR;
     NOTREACHED() << upload_url_ << " is not a valid URL.";
   }
 }
 
 UploadJobImpl::~UploadJobImpl() {
 }
 
 void UploadJobImpl::AddDataSegment(
     const std::string& name,
     const std::string& filename,
     const std::map<std::string, std::string>& header_entries,
     std::unique_ptr<std::string> data) {
   // Cannot add data to busy or failed instance.
   DCHECK_EQ(IDLE, state_);
   if (state_ != IDLE)
     return;
+  DCHECK(thread_checker_.CalledOnValidThread());
 
   std::unique_ptr<DataSegment> data_segment(
       new DataSegment(name, filename, std::move(data), header_entries));
   data_segments_.push_back(std::move(data_segment));
 }
 
 void UploadJobImpl::Start() {
   // Cannot start an upload on a busy or failed instance.
   DCHECK_EQ(IDLE, state_);
   if (state_ != IDLE)
     return;
+  DCHECK_EQ(0, retry_);
+  DCHECK(thread_checker_.CalledOnValidThread());

[Andrew T Wilson (Slow), 2016/05/10 22:00:42]

Move call to CalledOnValidThread() to the top of the function, in every place
you use it in this file. It should be the first thing done in every function you
call it from.

[Marton Hunyady, 2016/05/11 12:55:30]

Done.

+
   RequestAccessToken();
 }
 
 void UploadJobImpl::RequestAccessToken() {
+  DCHECK(!access_token_request_);
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   state_ = ACQUIRING_TOKEN;
 
   OAuth2TokenService::ScopeSet scope_set;
   scope_set.insert(GaiaConstants::kDeviceManagementServiceOAuth);
   access_token_request_ =
       token_service_->StartRequest(account_id_, scope_set, this);
 }
 
 bool UploadJobImpl::SetUpMultipart() {
   DCHECK_EQ(ACQUIRING_TOKEN, state_);
@@ skipping 71 matching lines @@
 
   upload_fetcher_ =
       net::URLFetcher::Create(upload_url_, net::URLFetcher::POST, this);
   upload_fetcher_->SetRequestContext(url_context_getter_.get());
   upload_fetcher_->SetUploadData(content_type, *post_data_);
   upload_fetcher_->AddExtraRequestHeader(
       base::StringPrintf(kAuthorizationHeaderFormat, access_token.c_str()));
   upload_fetcher_->Start();
 }
 
-void UploadJobImpl::StartUpload(const std::string& access_token) {
+void UploadJobImpl::StartUpload() {
+  DCHECK(thread_checker_.CalledOnValidThread());
+
   if (!SetUpMultipart()) {
     LOG(ERROR) << "Multipart message assembly failed.";
     state_ = ERROR;
     return;
   }
-  CreateAndStartURLFetcher(access_token);
+  CreateAndStartURLFetcher(access_token_);
   state_ = UPLOADING;
 }
 
 void UploadJobImpl::OnGetTokenSuccess(
     const OAuth2TokenService::Request* request,
     const std::string& access_token,
     const base::Time& expiration_time) {
   DCHECK_EQ(ACQUIRING_TOKEN, state_);
   DCHECK_EQ(access_token_request_.get(), request);
   access_token_request_.reset();
 
   // Also cache the token locally, so that we can revoke it later if necessary.
   access_token_ = access_token;
-  StartUpload(access_token);
+  StartUpload();
 }
 
 void UploadJobImpl::OnGetTokenFailure(
     const OAuth2TokenService::Request* request,
     const GoogleServiceAuthError& error) {
   DCHECK_EQ(ACQUIRING_TOKEN, state_);
   DCHECK_EQ(access_token_request_.get(), request);
   access_token_request_.reset();
   LOG(ERROR) << "Token request failed: " << error.ToString();
-  state_ = ERROR;
-  delegate_->OnFailure(AUTHENTICATION_ERROR);
+  HandleError(AUTHENTICATION_ERROR);
+}
+
+void UploadJobImpl::HandleError(ErrorCode errorCode) {

[Andrew T Wilson (Slow), 2016/05/10 22:00:42]

errorCode -> error_code

[Marton Hunyady, 2016/05/11 12:55:30]

Done.

+  retry_++;
+  upload_fetcher_.reset();
+
+  std::string errorString;

[Andrew T Wilson (Slow), 2016/05/10 22:00:42]

errorString->error_string

[Marton Hunyady, 2016/05/11 12:55:30]

Done.

+  switch (errorCode) {
+    case NETWORK_ERROR:
+      errorString = "NETWORK_ERROR";
+      break;
+    case AUTHENTICATION_ERROR:
+      errorString = "AUTHENTICATION_ERROR";
+      break;
+    case SERVER_ERROR:
+      errorString = "SERVER_ERROR";
+      break;
+  }
+  LOG(ERROR) << "Upload failed: " << errorString;

[Andrew T Wilson (Slow), 2016/05/10 22:00:42]

This is OK, but I think it's superior to just log the errorCode itself.

[Marton Hunyady, 2016/05/11 12:55:30]

Done.

+
+  if (retry_ >= kMaxAttempts) {
+    // Maximum number of attempts reached, failure.
+    LOG(ERROR) << "Maximum number of attempts reached.";
+    access_token_.clear();
+    post_data_.reset();
+    state_ = ERROR;
+    delegate_->OnFailure(errorCode);
+  } else {
+    if (errorCode == AUTHENTICATION_ERROR) {
+      LOG(ERROR) << "Retrying upload with a new token.";
+      // Request new token and retry.
+      OAuth2TokenService::ScopeSet scope_set;
+      scope_set.insert(GaiaConstants::kDeviceManagementServiceOAuth);
+      token_service_->InvalidateAccessToken(account_id_, scope_set,
+                                            access_token_);
+      access_token_.clear();
+      task_runner_->PostDelayedTask(
+          FROM_HERE, base::Bind(&UploadJobImpl::RequestAccessToken,
+                                weak_factory_.GetWeakPtr()),
+          base::TimeDelta::FromMilliseconds(kRetryDelayMs));
+    } else {
+      // Retry without a new token.
+      state_ = ACQUIRING_TOKEN;
+      LOG(WARNING) << "Retrying upload with the same token.";
+      task_runner_->PostDelayedTask(
+          FROM_HERE,
+          base::Bind(&UploadJobImpl::StartUpload, weak_factory_.GetWeakPtr()),
+          base::TimeDelta::FromMilliseconds(kRetryDelayMs));
+    }
+  }
 }
 
 void UploadJobImpl::OnURLFetchComplete(const net::URLFetcher* source) {
   DCHECK_EQ(upload_fetcher_.get(), source);
+  DCHECK_EQ(UPLOADING, state_);
   const net::URLRequestStatus& status = source->GetStatus();
   if (!status.is_success()) {
     LOG(ERROR) << "URLRequestStatus error " << status.error();
-    upload_fetcher_.reset();
-    state_ = ERROR;
-    post_data_.reset();
-    delegate_->OnFailure(NETWORK_ERROR);
-    return;
-  }
-
-  const int response_code = source->GetResponseCode();
-  const bool success = response_code == net::HTTP_OK;
-  if (!success)
-    LOG(ERROR) << "POST request failed with HTTP status code " << response_code;
-
-  if (response_code == net::HTTP_UNAUTHORIZED) {
-    if (retry_ >= kMaxRetries) {
+    HandleError(NETWORK_ERROR);
+  } else {
+    const int response_code = source->GetResponseCode();
+    if (response_code == net::HTTP_OK) {
+      // Successful upload
       upload_fetcher_.reset();
+      access_token_.clear();
+      post_data_.reset();
+      state_ = SUCCESS;
+      delegate_->OnSuccess();
+    } else if (response_code == net::HTTP_UNAUTHORIZED) {
       LOG(ERROR) << "Unauthorized request.";
-      state_ = ERROR;
-      post_data_.reset();
-      delegate_->OnFailure(AUTHENTICATION_ERROR);
-      return;
+      HandleError(AUTHENTICATION_ERROR);
+    } else {
+      LOG(ERROR) << "POST request failed with HTTP status code "
+                 << response_code << ".";
+      HandleError(SERVER_ERROR);
     }
-    retry_++;
-    upload_fetcher_.reset();
-    OAuth2TokenService::ScopeSet scope_set;
-    scope_set.insert(GaiaConstants::kDeviceManagementServiceOAuth);
-    token_service_->InvalidateAccessToken(account_id_, scope_set,
-                                          access_token_);
-    access_token_.clear();
-    RequestAccessToken();
-    return;
-  }
-
-  upload_fetcher_.reset();
-  access_token_.clear();
-  upload_fetcher_.reset();
-  post_data_.reset();
-  if (success) {
-    state_ = SUCCESS;
-    delegate_->OnSuccess();
-  } else {
-    state_ = ERROR;
-    delegate_->OnFailure(SERVER_ERROR);
   }
 }
 
 }  // namespace policy