HTTP retry support.

net/http/http_network_transaction.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/http/http_network_transaction.h"
 
 #include <set>
 #include <vector>
 
 #include "base/bind.h"
@@ skipping 57 matching lines @@
 #endif
 
 
 using base::Time;
 using base::TimeDelta;
 
 namespace net {
 
 namespace {
 
+const int kNumRetries = 3;
+
 void ProcessAlternateProtocol(
     HttpNetworkSession* session,
     const HttpResponseHeaders& headers,
     const HostPortPair& http_host_port_pair) {
   if (!headers.HasHeader(kAlternateProtocolHeader))
     return;
 
   std::vector<std::string> alternate_protocol_values;
   void* iter = NULL;
   std::string alternate_protocol_str;
@@ skipping 47 matching lines @@
                               base::Unretained(this))),
       session_(session),
       request_(NULL),
       priority_(priority),
       headers_valid_(false),
       logged_response_time_(false),
       fallback_error_code_(ERR_SSL_INAPPROPRIATE_FALLBACK),
       request_headers_(),
       read_buf_len_(0),
       total_received_bytes_(0),
+      retry_attempt_(0),
+      offset_(0),
+      previous_content_length_(0),
       next_state_(STATE_NONE),
       establishing_tunnel_(false),
       websocket_handshake_stream_base_create_helper_(NULL) {
   session->ssl_config_service()->GetSSLConfig(&server_ssl_config_);
   session->GetNextProtos(&server_ssl_config_.next_protos);
   proxy_ssl_config_ = server_ssl_config_;
 }
 
 HttpNetworkTransaction::~HttpNetworkTransaction() {
   if (stream_.get()) {
@@ skipping 616 matching lines @@
 }
 
 int HttpNetworkTransaction::DoInitStream() {
   DCHECK(stream_.get());
   next_state_ = STATE_INIT_STREAM_COMPLETE;
   return stream_->InitializeStream(request_, priority_, net_log_, io_callback_);
 }
 
 int HttpNetworkTransaction::DoInitStreamComplete(int result) {
   if (result == OK) {
+    if (offset_)
+      stream_->SetRestartInfo(offset_, hash_, sizeof(hash_));
     next_state_ = STATE_GENERATE_PROXY_AUTH_TOKEN;
   } else {
     if (result < 0)
       result = HandleIOError(result);
 
     // The stream initialization failed, so this stream will never be useful.
     if (stream_)
         total_received_bytes_ += stream_->GetTotalReceivedBytes();
     stream_.reset();
   }
@@ skipping 202 matching lines @@
   // bizarre for SPDY. Assuming this logic is useful at all.
   // TODO(davidben): Bubble the error code up so we do not cache?
   if (result == ERR_CONNECTION_CLOSED && response_.headers.get())
     result = OK;
 
   if (result < 0)
     return HandleIOError(result);
 
   DCHECK(response_.headers.get());
 
+  if ((response_.headers->response_code() == 200 ||
+       response_.headers->response_code() == 206) && offset_) {
+    std::string etag, last_modified;
+    response_.headers->EnumerateHeader(NULL, "last-modified", &last_modified);
+    if (response_.headers->GetHttpVersion() >= HttpVersion(1, 1))
+       response_.headers->EnumerateHeader(NULL, "etag", &etag);
+
+    // Return an error if content was updated on a retry.
+    if ((previous_content_length_ != response_.headers->GetContentLength() &&
+         response_.headers->response_code() == 200) ||
+        previous_etag_ != etag || previous_last_modified_ != last_modified) {
+      return HandleIOError(ERR_RETRY_CONTENT_UPDATED);
+    }
+  }
+
   // On a 408 response from the server ("Request Timeout") on a stale socket,
   // retry the request.
   // Headers can be NULL because of http://crbug.com/384554.
   if (response_.headers.get() && response_.headers->response_code() == 408 &&
       stream_->IsConnectionReused()) {
     net_log_.AddEventWithNetErrorCode(
         NetLog::TYPE_HTTP_TRANSACTION_RESTART_AFTER_ERROR,
         response_.headers->response_code());
     // This will close the socket - it would be weird to try and reuse it, even
     // if the server doesn't actually close it.
@@ skipping 41 matching lines @@
 
   int rv = HandleAuthChallenge();
   if (rv != OK)
     return rv;
 
   if (is_https_request())
     stream_->GetSSLInfo(&response_.ssl_info);
 
   headers_valid_ = true;
 
+  if (retry_attempt_ && read_buf_.get())
+    next_state_ = STATE_READ_BODY;
+
   if (session_->huffman_aggregator()) {
     session_->huffman_aggregator()->AggregateTransactionCharacterCounts(
         *request_,
         request_headers_,
         proxy_info_.proxy_server(),
         *response_.headers);
   }
   return OK;
 }
 
 int HttpNetworkTransaction::DoReadBody() {
   DCHECK(read_buf_.get());
   DCHECK_GT(read_buf_len_, 0);
   DCHECK(stream_ != NULL);
 
   next_state_ = STATE_READ_BODY_COMPLETE;
   return stream_->ReadResponseBody(
       read_buf_.get(), read_buf_len_, io_callback_);
 }
 
 int HttpNetworkTransaction::DoReadBodyComplete(int result) {
   // We are done with the Read call.
   bool done = false;
   if (result <= 0) {
     DCHECK_NE(ERR_IO_PENDING, result);
+    if (result < 0)
+      return HandleIOError(result);
     done = true;
   }
 
   bool keep_alive = false;
   if (stream_->IsResponseBodyComplete()) {
     // Note: Just because IsResponseBodyComplete is true, we're not
     // necessarily "done".  We're only "done" when it is the last
     // read on this HttpNetworkTransaction, which will be signified
     // by a zero-length read.
     // TODO(mbelshe): The keepalive property is really a property of
@@ skipping 121 matching lines @@
                              base::TimeDelta::FromMinutes(10), 100);
 
   if (!stream_->IsConnectionReused()) {
     UMA_HISTOGRAM_CUSTOM_TIMES(
         "Net.Transaction_Latency_Total_New_Connection",
         total_duration, base::TimeDelta::FromMilliseconds(1),
         base::TimeDelta::FromMinutes(10), 100);
   }
 }
 
+void HttpNetworkTransaction::LogTransactionRetryMetrics(int error) const {
+  UMA_HISTOGRAM_CUSTOM_COUNTS("Net.HttpRetry.Count", retry_attempt_, 1, 5, 5);
+  if (request_->load_flags & LOAD_MAIN_FRAME) {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+    "Net.HttpRetry.Cause.MainFrame",
+    -error,
+    GetAllErrorCodesForUma());
+  } else {
+    UMA_HISTOGRAM_CUSTOM_ENUMERATION(
+    "Net.HttpRetry.Cause.Subresource",
+    -error,
+    GetAllErrorCodesForUma());
+  }
+}
+
 int HttpNetworkTransaction::HandleCertificateRequest(int error) {
   // There are two paths through which the server can request a certificate
   // from us.  The first is during the initial handshake, the second is
   // during SSL renegotiation.
   //
   // In both cases, we want to close the connection before proceeding.
   // We do this for two reasons:
   //   First, we don't want to keep the connection to the server hung for a
   //   long time while the user selects a certificate.
   //   Second, even if we did keep the connection open, NSS has a bug where
@@ skipping 174 matching lines @@
     // likely happen when trying to retrieve its IP address.
     // See http://crbug.com/105824 for more details.
     case ERR_SOCKET_NOT_CONNECTED:
     // If a socket is closed on its initial request, HttpStreamParser returns
     // ERR_EMPTY_RESPONSE. This may still be close/reuse race if the socket was
     // preconnected but failed to be used before the server timed it out.
     case ERR_EMPTY_RESPONSE:
       if (ShouldResendRequest()) {
         net_log_.AddEventWithNetErrorCode(
             NetLog::TYPE_HTTP_TRANSACTION_RESTART_AFTER_ERROR, error);
+        retry_attempt_++;
+        LogTransactionRetryMetrics(error);
         ResetConnectionAndRequestForResend();
         error = OK;
       }
       break;
     case ERR_SPDY_PING_FAILED:
     case ERR_SPDY_SERVER_REFUSED_STREAM:
     case ERR_QUIC_HANDSHAKE_FAILED:
       net_log_.AddEventWithNetErrorCode(
           NetLog::TYPE_HTTP_TRANSACTION_RESTART_AFTER_ERROR, error);
       ResetConnectionAndRequestForResend();
@@ skipping 21 matching lines @@
   request_headers_.Clear();
   response_ = HttpResponseInfo();
   establishing_tunnel_ = false;
 }
 
 HttpResponseHeaders* HttpNetworkTransaction::GetResponseHeaders() const {
   return response_.headers.get();
 }
 
 bool HttpNetworkTransaction::ShouldResendRequest() const {
-  bool connection_is_proven = stream_->IsConnectionReused();
-  bool has_received_headers = GetResponseHeaders() != NULL;
+  if (retry_attempt_ > kNumRetries)
+    return false;
 
-  // NOTE: we resend a request only if we reused a keep-alive connection.
-  // This automatically prevents an infinite resend loop because we'll run
-  // out of the cached keep-alive connections eventually.
-  if (connection_is_proven && !has_received_headers)
-    return true;
-  return false;
+  if (headers_valid_) {

[mmenke, 2014/07/23 20:17:42]

If there a reason the old GetResponseHeaders() != NULL check doesn't work?

[jonnyr, 2014/07/25 13:41:59]

Done.

+    // Do not attempt a retry on anything but GET request if headers are valid.

[rvargas (doing something else), 2014/07/24 19:09:47]

How do we know that nothing actually reached the server when headers_valid_ is
false?

[jonnyr, 2014/07/25 13:41:59]

Right, this will have to be removed.

+    if (request_->method != "GET")
+      return false;
+
+    TimeDelta time_delta = base::TimeTicks::Now() - send_start_time_;
+    if (time_delta.InSeconds() > 30)
+      return true;
+
+    // Do not retry automatically if resource is larger than 1 Mb until byte
+    // range support is enabled. This is temporary.
+    if (response_.headers->GetContentLength() > 1000000)

[rvargas (doing something else), 2014/07/24 19:09:47]

Seems like this should go before the 30 seconds rule.

[jonnyr, 2014/07/25 13:41:59]

Done.

+      return false;
+
+    // If etag or last-modified is set we can always retry since the response
+    // can be verified.
+    if (response_.headers->HasHeader("etag") ||

[rvargas (doing something else), 2014/07/24 19:09:47]

use HasStrongValidators()

[jonnyr, 2014/07/25 13:41:59]

Done.

+        response_.headers->HasHeader("Last-Modified")) {
+      return true;
+    }
+
+    if (response_.headers->HasHeaderValue("cache-control", "no-cache") ||

[rvargas (doing something else), 2014/07/24 19:09:47]

why fail if no-cache or must-revalidate?

[jonnyr, 2014/07/25 13:41:59]

What I am trying to avoid is that we glue together two responses 
that have been modified between retry attempts. I am using the 
cache headers as an indication that the resource could be modified 
in between two requests in an attempt to be as careful as possible.

no-cache is used to force intermediate caches to obtain a new copy 
from the origin server, and must-revalidate to force the client to 
revalidate with server if stale. I am thinking that developers use 
this to get the client to check with original content servers often 
since content is changed all the time.

An alternative to being this strict is to cross-check content, and 
save and match x number of bytes in the response before glueing 
together the result. Currently everything is hashed, but the 
intention is to remove the hashing once we see that we do not get 
too many ERR_RETRY_HASH_MISMATCH errors.

[rvargas (doing something else), 2014/07/26 00:30:33]

hmm... I don't think it is safe to remove the hash unless we have zero
ERR_RETRY_HASH_MISMATCH, at which point something is probably broken an not
reporting that error :)

Even if statistically speaking the error rate is insignificant, the fact that we
would be mixing responses seems unacceptable to me.

And if there is a hash in place, the only reason I could think of for being
extra conservative would be to try to avoid extra latency before actually
failing a request, but that should probably come out of an experiment (I mean,
we are assuming that the user is better served by waiting more and actually
seeing the page than a fast failure)

I think there's a lot of no-cache content that doesn't really change that often
but people still feel the need to force revalidations to avoid showing stale
content occasionally. (That is to say, choosing an expiration date is not an
easy thing).

+        response_.headers->HasHeaderValue("cache-control", "no-store") ||
+        response_.headers->HasHeaderValue("cache-control", "must-revalidate") ||
+        response_.headers->HasHeaderValue("pragma", "no-cache")) {
+      return false;
+    }
+
+    if (response_.headers->GetMaxAgeValue(&time_delta))
+      return time_delta.InSeconds() > 60;
+
+    // If there is no Date header, then assume that the server response was
+    // generated at the time when we received the response, hence do not retry.

[rvargas (doing something else), 2014/07/24 19:09:48]

why not? If it actually differs, we'll catch that by the hash.

[jonnyr, 2014/07/25 13:41:59]

The intention is to remove the hash.

+    Time date_value;
+    if (!response_.headers->GetDateValue(&date_value))
+      return false;
+
+    Time expires_value;
+    if (response_.headers->GetExpiresValue(&expires_value)) {
+      time_delta = expires_value - date_value;
+      if (time_delta.InSeconds() > 60)
+        return true;
+    }

[rvargas (doing something else), 2014/07/24 19:09:47]

Most of this logic should probably be moved to a method of HttpResponseHeaders.

[jonnyr, 2014/07/25 13:41:59]

That I can do. Do you think the overall strategy as mentioned above makes sense?

[rvargas (doing something else), 2014/07/26 00:30:33]

Yeah, I think that avoid merging by using a hash makes sense, and what I've seen
seems appropriate.

+    return false;
+  }
+  return true;

[mmenke, 2014/07/23 20:17:42]

Out of paranoia, think we should keep the only retry non-GETs (And maybe
non-HEADs) if the connection was also reused.

[jonnyr, 2014/07/25 13:41:59]

Agreed.

 }
 
 void HttpNetworkTransaction::ResetConnectionAndRequestForResend() {
+  // We need to clear request_headers_ because it contains the real request
+  // headers, but we may need to resend the CONNECT request first to recreate
+  // the SSL tunnel.
+  request_headers_.Clear();
+  headers_valid_ = false;
+
+  if (stream_) {
+    total_received_bytes_ += stream_->GetTotalReceivedBytes();
+    if (stream_->GetReceivedBodyLength() > offset_) {
+      offset_ = stream_->GetReceivedBodyLength();
+      stream_->GetHash(hash_, sizeof(hash_));
+    }
+  }
+
+  HttpResponseHeaders* headers = GetResponseHeaders();
+  if (offset_ && headers) {
+    previous_content_length_ = headers->GetContentLength();
+    headers->EnumerateHeader(NULL, "last-modified",
+                             &previous_last_modified_);
+    if (headers->GetHttpVersion() >= HttpVersion(1, 1))
+      headers->EnumerateHeader(NULL, "etag", &previous_etag_);
+
+    // Disable until we have statistics that show that retrying does not
+    // result in too many ERR_RETRY_HASH_MISMATCH.
+    if (0 && previous_content_length_ &&
+        headers->HasHeaderValue("Accept-Ranges", "bytes")) {
+      // Try resending using a range request if supported.
+      request_headers_.SetHeader(HttpRequestHeaders::kRange,
+          "bytes=" + base::Uint64ToString(offset_) + "-");
+      if (!previous_last_modified_.empty())
+        request_headers_.SetHeader("If-Unmodified-Since",
+            previous_last_modified_);
+      if (!previous_etag_.empty())

[rvargas (doing something else), 2014/07/24 19:09:47]

I suggest checking this one before last-modified and not adding both headers if
this one is present.

[jonnyr, 2014/07/25 13:41:59]

Done.

+        request_headers_.SetHeader("If-Match", previous_etag_);
+    }
+  }
+
+  response_ = HttpResponseInfo();
+  establishing_tunnel_ = false;
   if (stream_.get()) {
     stream_->Close(true);
     stream_.reset();
   }
-
-  // We need to clear request_headers_ because it contains the real request
-  // headers, but we may need to resend the CONNECT request first to recreate
-  // the SSL tunnel.
-  request_headers_.Clear();
   next_state_ = STATE_CREATE_STREAM;  // Resend the request.
 }
 
 bool HttpNetworkTransaction::ShouldApplyProxyAuth() const {
   return !is_https_request() &&
       (proxy_info_.is_https() || proxy_info_.is_http());
 }
 
 bool HttpNetworkTransaction::ShouldApplyServerAuth() const {
   return !(request_->load_flags & LOAD_DO_NOT_SEND_AUTH_DATA);
@@ skipping 88 matching lines @@
       description = base::StringPrintf("Unknown state 0x%08X (%u)", state,
                                        state);
       break;
   }
   return description;
 }
 
 #undef STATE_CASE
 
 }  // namespace net