Automatically retry failed network requests.

net/url_request/url_request_http_job.cc

 // Copyright (c) 2012 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 "net/url_request/url_request_http_job.h"
 
 #include "base/base_switches.h"
 #include "base/bind.h"
 #include "base/bind_helpers.h"
 #include "base/command_line.h"
@@ skipping 31 matching lines @@
 #include "net/url_request/url_request_context.h"
 #include "net/url_request/url_request_error_job.h"
 #include "net/url_request/url_request_redirect_job.h"
 #include "net/url_request/url_request_throttler_header_adapter.h"
 #include "net/url_request/url_request_throttler_manager.h"
 
 static const char kAvailDictionaryHeader[] = "Avail-Dictionary";
 
 namespace net {
 
+namespace {
+
+// Array of all network error codes.  Needed for histograms.
+const int kAllNetErrorCodes[] = {
+#define NET_ERROR(label, value) -(value),
+#include "net/base/net_error_list.h"
+#undef NET_ERROR
+};
+
+}  // namespace
+
 class URLRequestHttpJob::HttpFilterContext : public FilterContext {
  public:
   explicit HttpFilterContext(URLRequestHttpJob* job);
   virtual ~HttpFilterContext();
 
   // FilterContext implementation.
   virtual bool GetMimeType(std::string* mime_type) const;
   virtual bool GetURL(GURL* gurl) const;
   virtual base::Time GetRequestTime() const;
   virtual bool IsCachedContent() const;
@@ skipping 147 matching lines @@
       bytes_observed_in_packets_(0),
       request_time_snapshot_(),
       final_packet_time_(),
       ALLOW_THIS_IN_INITIALIZER_LIST(
           filter_context_(new HttpFilterContext(this))),
       ALLOW_THIS_IN_INITIALIZER_LIST(weak_factory_(this)),
       ALLOW_THIS_IN_INITIALIZER_LIST(on_headers_received_callback_(
           base::Bind(&URLRequestHttpJob::OnHeadersReceivedCallback,
                      base::Unretained(this)))),
       awaiting_callback_(false),
-      http_transaction_delegate_(new HttpTransactionDelegateImpl(request)) {
+      http_transaction_delegate_(new HttpTransactionDelegateImpl(request)),
+      has_retried_(false),
+      error_before_retry_(OK) {
   URLRequestThrottlerManager* manager = request->context()->throttler_manager();
   if (manager)
     throttling_entry_ = manager->RegisterRequestUrl(request->url());
 
   ResetTimer();
 }
 
 void URLRequestHttpJob::NotifyHeadersComplete() {
   DCHECK(!response_info_);
 
@@ skipping 492 matching lines @@
   // If the transaction was destroyed, then the job was cancelled, and
   // we can just ignore this notification.
   if (!transaction_.get())
     return;
 
   // Clear the IO_PENDING status
   SetStatus(URLRequestStatus());
 
   const URLRequestContext* context = request_->context();
 
+  // If this was a retry, record whether or not we successfully connected to
+  // the server on the second try.
+  if (error_before_retry_ != OK) {

[willchan no longer on Chromium, 2012/08/21 21:01:10]

Why aren't we checking has_retried_? And if true, then assert
error_before_retry_ < ERR_IO_PENDING.

[mmenke, 2012/08/21 21:25:29]

See response below.  I've added a DCHECK(has_retried_);

+    RecordRetryResult(result);
+    error_before_retry_ = OK;

[willchan no longer on Chromium, 2012/08/21 21:01:10]

Why is this needed?

[mmenke, 2012/08/21 21:25:29]

There are a bunch of ways to restart (ContinueWithCertificate,
ContinueDespiteLastError, RestartTransactionWithAuth).  If we retry a request as
a result of an error, and then retry for another reason, we don't want to add
two entries to the retry histogram.

+  }
+
   if (result == ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN &&
       transaction_->GetResponseInfo() != NULL) {
     FraudulentCertificateReporter* reporter =
       context->fraudulent_certificate_reporter();
     if (reporter != NULL) {
       const SSLInfo& ssl_info = transaction_->GetResponseInfo()->ssl_info;
       bool sni_available = SSLConfigService::IsSNIAvailable(
           context->ssl_config_service());
       const std::string& host = request_->url().host();
 
@@ skipping 36 matching lines @@
     const bool fatal =
         context->transport_security_state() &&
         context->transport_security_state()->GetDomainState(
             request_info_.url.host(),
             SSLConfigService::IsSNIAvailable(context->ssl_config_service()),
             &domain_state);
     NotifySSLCertificateError(transaction_->GetResponseInfo()->ssl_info, fatal);
   } else if (result == ERR_SSL_CLIENT_AUTH_CERT_NEEDED) {
     NotifyCertificateRequested(
         transaction_->GetResponseInfo()->cert_request_info);
+  } else if (ShouldRetryRequest(result)) {
+    RetryRequestOnError(result);
   } else {
     NotifyStartError(URLRequestStatus(URLRequestStatus::FAILED, result));
   }
 }
 
 void URLRequestHttpJob::OnHeadersReceivedCallback(int result) {
   request_->net_log().EndEvent(NetLog::TYPE_URL_REQUEST_BLOCKED_ON_DELEGATE);
   awaiting_callback_ = false;
 
   // Check that there are no callbacks to already canceled requests.
@@ skipping 31 matching lines @@
   ResetTimer();
 
   // Update the cookies, since the cookie store may have been updated from the
   // headers in the 401/407. Since cookies were already appended to
   // extra_headers, we need to strip them out before adding them again.
   request_info_.extra_headers.RemoveHeader(HttpRequestHeaders::kCookie);
 
   AddCookieHeaderAndStart();
 }
 
+void URLRequestHttpJob::RetryRequestOnError(int result) {
+  // Should have a transaction that returned the result, but should not have
+  // received headers yet.
+  DCHECK(transaction_.get());
+  DCHECK(!response_info_);
+  DCHECK(result != OK && result != ERR_IO_PENDING);
+
+  error_before_retry_ = result;
+  has_retried_ = true;
+  ResetTimer();
+  transaction_.reset();
+
+  SetStatus(URLRequestStatus(URLRequestStatus::IO_PENDING, 0));
+  StartTransactionInternal();
+}
+
+bool URLRequestHttpJob::ShouldRetryRequest(int result) const {
+  // Request must not have been cancelled, and no response info may have been
+  // received yet.
+  DCHECK(transaction_.get());
+  DCHECK(request_);
+  DCHECK(!response_info_);
+
+  // Don't retry if the request has already been retried or it's not a GET.
+  if (has_retried_ || request_->method() != "GET") {
+    return false;
+  }
+
+
+  // If the HTTP job has already taken too long, do not retry the request.
+  base::TimeDelta request_duration = base::TimeTicks::Now() - start_time_;
+  if (request_duration >= base::TimeDelta::FromSeconds(10))
+    return false;
+
+  // TODO(mmenke):  Look at the metrics and figure out which, if any, of these
+  //                error codes it's worth retrying on.
+  switch (result) {
+    // These four error codes will also result in a retry in NetworkTransaction,
+    // in the case of a reused socket.  It's unclear how much correlation there
+    // is between receiving one of these errors on a reused socket and on a
+    // fresh socket on retry.
+    case ERR_CONNECTION_RESET:
+    case ERR_CONNECTION_CLOSED:
+    case ERR_CONNECTION_ABORTED:
+    case ERR_SOCKET_NOT_CONNECTED:
+
+    // System errors.
+    case ERR_FAILED:
+    case ERR_UNEXPECTED:
+
+    // Connection errors.
+    case ERR_CONNECTION_REFUSED:
+    case ERR_CONNECTION_FAILED:
+    case ERR_NAME_NOT_RESOLVED:
+    case ERR_INTERNET_DISCONNECTED:
+    case ERR_ADDRESS_UNREACHABLE:
+    case ERR_NETWORK_ACCESS_DENIED:
+    case ERR_ADDRESS_IN_USE:
+
+    // Content errors.
+    case ERR_EMPTY_RESPONSE:
+
+    // Cache errors.
+    case ERR_CACHE_MISS:
+    case ERR_CACHE_READ_FAILURE:
+      return true;
+
+    default:
+      return false;
+  }
+}
+
 void URLRequestHttpJob::SetUpload(UploadData* upload) {
   DCHECK(!transaction_.get()) << "cannot change once started";
   request_info_.upload_data = upload;
 }
 
 void URLRequestHttpJob::SetExtraRequestHeaders(
     const HttpRequestHeaders& headers) {
   DCHECK(!transaction_.get()) << "cannot change once started";
   request_info_.extra_headers.CopyFrom(headers);
 }
@@ skipping 431 matching lines @@
 
 void URLRequestHttpJob::ResetTimer() {
   if (!request_creation_time_.is_null()) {
     NOTREACHED()
         << "The timer was reset before it was recorded.";
     return;
   }
   request_creation_time_ = base::Time::Now();
 }
 
+void URLRequestHttpJob::RecordRetryResult(int result) const {
+  if (request_info_.load_flags & LOAD_MAIN_FRAME) {

[willchan no longer on Chromium, 2012/08/21 21:01:10]

Note that long-term this is definitely unacceptable. I don't want more
proliferation of LOAD_MAIN_FRAME since those don't belong at the net/ level.

[mmenke, 2012/08/21 21:25:29]

Yea, I think you're right about that.

You think the way to do this, if it ends up being useful, is with a
NetworkDelegate function?  ShouldRetryRequestOnFailure?  Could also do the
entire retry Chrome-side, but messing much with ResourceDispatcherHost just
seems a bad idea.

+    if (result >= 0) {

[willchan no longer on Chromium, 2012/08/21 21:01:10]

Why is this >= 0? Shouldn't it be OK? Do we expect any other values?

[mmenke, 2012/08/21 21:25:29]

No, we don't, I was just being paranoid.  Done.

+      UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+          "Net.NetworkErrorsRecovered.MainFrame",
+          -error_before_retry_,
+          base::CustomHistogram::ArrayToCustomRanges(
+              kAllNetErrorCodes, arraysize(kAllNetErrorCodes)));
+    } else {
+      UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+          "Net.NetworkErrorsUnrecovered.MainFrame",
+          -error_before_retry_,
+          base::CustomHistogram::ArrayToCustomRanges(
+              kAllNetErrorCodes, arraysize(kAllNetErrorCodes)));
+    }
+  } else {
+    if (result >= 0) {
+      UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+          "Net.NetworkErrorsRecovered.Subresource",
+          -error_before_retry_,
+          base::CustomHistogram::ArrayToCustomRanges(
+              kAllNetErrorCodes, arraysize(kAllNetErrorCodes)));
+    } else {
+      UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+          "Net.NetworkErrorsUnrecovered.Subresource",
+          -error_before_retry_,
+          base::CustomHistogram::ArrayToCustomRanges(
+              kAllNetErrorCodes, arraysize(kAllNetErrorCodes)));
+    }
+  }
+}
+
 void URLRequestHttpJob::UpdatePacketReadTimes() {
   if (!packet_timing_enabled_)
     return;
 
   if (filter_input_byte_count() <= bytes_observed_in_packets_) {
     DCHECK_EQ(filter_input_byte_count(), bytes_observed_in_packets_);
     return;  // No new bytes have arrived.
   }
 
   final_packet_time_ = base::Time::Now();
@@ skipping 167 matching lines @@
 
 void URLRequestHttpJob::NotifyURLRequestDestroyed() {
   awaiting_callback_ = false;
 }
 
 void URLRequestHttpJob::OnDetachRequest() {
   http_transaction_delegate_->OnDetachRequest();
 }
 
 }  // namespace net