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

Unified Diff: net/url_request/url_request_http_job.cc

Issue 10872044: Retry failed network requests. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src/
Patch Set: Fix bug Created 8 years, 4 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
« no previous file with comments | « net/url_request/url_request_http_job.h ('k') | net/url_request/url_request_job_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: net/url_request/url_request_http_job.cc
===================================================================
--- net/url_request/url_request_http_job.cc (revision 152798)
+++ net/url_request/url_request_http_job.cc (working copy)
@@ -49,6 +49,17 @@
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);
@@ -244,7 +255,9 @@
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());
@@ -757,6 +770,14 @@
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) {
+ DCHECK(has_retried_);
+ RecordRetryResult(result);
+ error_before_retry_ = OK;
+ }
+
if (result == ERR_SSL_PINNED_KEY_NOT_IN_CERT_CHAIN &&
transaction_->GetResponseInfo() != NULL) {
FraudulentCertificateReporter* reporter =
@@ -813,6 +834,8 @@
} 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));
}
@@ -864,6 +887,78 @@
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;
@@ -1317,6 +1412,38 @@
request_creation_time_ = base::Time::Now();
}
+void URLRequestHttpJob::RecordRetryResult(int result) const {
+ if (request_info_.load_flags & LOAD_MAIN_FRAME) {
+ if (result != OK) {
+ 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 != OK) {
+ 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;
« no previous file with comments | « net/url_request/url_request_http_job.h ('k') | net/url_request/url_request_job_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698