Merge branch '1.0' of https://github.com/lostisland/faraday into feat…

…ure/#762-rspec

lib/faraday/adapter.rb

@@ -40,6 +40,8 @@ def call(env)
       env.clear_body if env.needs_body?
     end
 
+    private
+
     def save_response(env, status, body, headers = nil, reason_phrase = nil)
       env.status = status
       env.body = body

lib/faraday/adapter/net_http.rb

@@ -56,6 +56,8 @@ def call(env)
         raise Faraday::Error::TimeoutError, err
       end
 
+      private
+
       def create_request(env)
         request = Net::HTTPGenericRequest.new \
           env[:method].to_s.upcase,    # request method

lib/faraday/adapter/net_http_persistent.rb

@@ -1,13 +1,23 @@
-# Rely on autoloading instead of explicit require; helps avoid the "already
-# initialized constant" warning on Ruby 1.8.7 when NetHttp is refereced below.
-# require 'faraday/adapter/net_http'
-
 module Faraday
   class Adapter
     class NetHttpPersistent < NetHttp
       dependency 'net/http/persistent'
 
+      private
+
       def net_http_connection(env)
+        proxy_uri = proxy_uri(env)
+
+        cached_connection env[:url], proxy_uri do
+          if Net::HTTP::Persistent.instance_method(:initialize).parameters.first == [:key, :name]
+            Net::HTTP::Persistent.new(name: 'Faraday', proxy: proxy_uri)
+          else
+            Net::HTTP::Persistent.new('Faraday', proxy_uri)
+          end
+        end
+      end
+
+      def proxy_uri(env)
         proxy_uri = nil
         if (proxy = env[:request][:proxy])
           proxy_uri = ::URI::HTTP === proxy[:uri] ? proxy[:uri].dup : ::URI.parse(proxy[:uri].to_s)
@@ -18,12 +28,7 @@ def net_http_connection(env)
             define_method(:password) { proxy[:password] }
           end if proxy[:user]
         end
-
-        if Net::HTTP::Persistent.instance_method(:initialize).parameters.first == [:key, :name]
-          Net::HTTP::Persistent.new(name: 'Faraday', proxy: proxy_uri)
-        else
-          Net::HTTP::Persistent.new('Faraday', proxy_uri)
-        end
+        proxy_uri
       end
 
       def perform_request(http, env)
@@ -49,6 +54,10 @@ def configure_ssl(http, ssl)
         http.ca_file      = ssl[:ca_file]      if ssl[:ca_file]
         http.ssl_version  = ssl[:version]      if ssl[:version]
       end
+
+      def cached_connection(url, proxy_uri)
+        (@cached_connection ||= {})[[url.scheme, url.host, url.port, proxy_uri]] ||= yield
+      end
     end
   end
 end

lib/faraday/error.rb

@@ -55,8 +55,12 @@ def initialize(ex = nil)
   class SSLError < ClientError
   end
 
+  class RetriableResponse < ClientError; end
+
   [:ClientError, :ConnectionFailed, :ResourceNotFound,
-   :ParsingError, :TimeoutError, :SSLError].each do |const|
+   :ParsingError, :TimeoutError, :SSLError, :RetriableResponse].each do |const|
     Error.const_set(const, Faraday.const_get(const))
   end
+
+
 end

lib/faraday/request/retry.rb

@@ -23,7 +23,9 @@ class Request::Retry < Faraday::Middleware
     IDEMPOTENT_METHODS = [:delete, :get, :head, :options, :put]
 
     class Options < Faraday::Options.new(:max, :interval, :max_interval, :interval_randomness,
-                                         :backoff_factor, :exceptions, :methods, :retry_if)
+                                         :backoff_factor, :exceptions, :methods, :retry_if, :retry_block,
+                                         :retry_statuses)
+
       DEFAULT_CHECK = lambda { |env,exception| false }
 
       def self.from(value)
@@ -56,7 +58,8 @@ def backoff_factor
 
       def exceptions
         Array(self[:exceptions] ||= [Errno::ETIMEDOUT, 'Timeout::Error',
-                                     Error::TimeoutError])
+                                     Error::TimeoutError,
+                                     Faraday::Error::RetriableResponse])
       end
 
       def methods
@@ -67,6 +70,13 @@ def retry_if
         self[:retry_if] ||= DEFAULT_CHECK
       end
 
+      def retry_block
+        self[:retry_block] ||= Proc.new {}
+      end
+
+      def retry_statuses
+        Array(self[:retry_statuses] ||= [])
+      end
     end
 
     # Public: Initialize middleware
@@ -94,34 +104,43 @@ def retry_if
     #                       if the exception produced is non-recoverable or if the
     #                       the HTTP method called is not idempotent.
     #                       (defaults to return false)
+    # retry_block         - block that is executed after every retry. Request environment, middleware options,
+    #                       current number of retries and the exception is passed to the block as parameters.
     def initialize(app, options = nil)
       super(app)
       @options = Options.from(options)
       @errmatch = build_exception_matcher(@options.exceptions)
     end
 
-    def sleep_amount(retries)
-      retry_index = @options.max - retries
-      current_interval = @options.interval * (@options.backoff_factor ** retry_index)
-      current_interval = [current_interval, @options.max_interval].min
-      random_interval  = rand * @options.interval_randomness.to_f * @options.interval
-      current_interval + random_interval
+    def calculate_sleep_amount(retries, env)
+      retry_after     = calculate_retry_after(env)
+      retry_interval  = calculate_retry_interval(retries)
+
+      return if retry_after && retry_after > @options.max_interval
+
+      retry_after && retry_after >= retry_interval ? retry_after : retry_interval
     end
 
     def call(env)
       retries = @options.max
       request_body = env[:body]
       begin
         env[:body] = request_body # after failure env[:body] is set to the response body
-        @app.call(env)
+        @app.call(env).tap do |resp|
+          raise Faraday::Error::RetriableResponse.new(nil, resp) if @options.retry_statuses.include?(resp.status)
+        end
       rescue @errmatch => exception
         if retries > 0 && retry_request?(env, exception)
           retries -= 1
           rewind_files(request_body)
-          sleep sleep_amount(retries + 1)
-          retry
+          @options.retry_block.call(env, @options, retries, exception)
+          if (sleep_amount = calculate_sleep_amount(retries + 1, env))
+            sleep sleep_amount
+            retry
+          end
         end
-        raise
+
+        raise unless exception.is_a?(Faraday::Error::RetriableResponse)
       end
     end
 
@@ -160,5 +179,29 @@ def rewind_files(body)
       end
     end
 
+    # MDN spec for Retry-After header: https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Retry-After
+    def calculate_retry_after(env)
+      response_headers = env[:response_headers]
+      return unless response_headers
+
+      retry_after_value = env[:response_headers]["Retry-After"]
+
+      # Try to parse date from the header value
+      begin
+        datetime = DateTime.rfc2822(retry_after_value)
+        datetime.to_time - Time.now.utc
+      rescue ArgumentError
+        retry_after_value.to_f
+      end
+    end
+
+    def calculate_retry_interval(retries)
+      retry_index = @options.max - retries
+      current_interval = @options.interval * (@options.backoff_factor ** retry_index)
+      current_interval = [current_interval, @options.max_interval].min
+      random_interval  = rand * @options.interval_randomness.to_f * @options.interval
+
+      current_interval + random_interval
+    end
   end
 end

test/adapters/net_http_persistent_test.rb

@@ -26,10 +26,24 @@ def test_custom_adapter_config
         http.idle_timeout = 123
       end
 
-      http = adapter.net_http_connection(:url => url, :request => {})
-      adapter.configure_request(http, {})
+      http = adapter.send(:net_http_connection, :url => url, :request => {})
+      adapter.send(:configure_request, http, {})
 
       assert_equal 123, http.idle_timeout
     end
+
+    def test_caches_connections
+      adapter = Faraday::Adapter::NetHttpPersistent.new
+      a = adapter.send(:net_http_connection, :url => URI('https://example.com:1234/foo'), :request => {})
+      b = adapter.send(:net_http_connection, :url => URI('https://example.com:1234/bar'), :request => {})
+      assert_equal a.object_id, b.object_id
+    end
+
+    def test_does_not_cache_connections_for_different_hosts
+      adapter = Faraday::Adapter::NetHttpPersistent.new
+      a = adapter.send(:net_http_connection, :url => URI('https://example.com:1234/foo'), :request => {})
+      b = adapter.send(:net_http_connection, :url => URI('https://example2.com:1234/bar'), :request => {})
+      refute_equal a.object_id, b.object_id
+    end
   end
 end

test/adapters/net_http_test.rb

@@ -16,7 +16,7 @@ def test_no_explicit_http_port_number
       url.port = nil
 
       adapter = Faraday::Adapter::NetHttp.new
-      http = adapter.net_http_connection(:url => url, :request => {})
+      http = adapter.send(:net_http_connection, :url => url, :request => {})
 
       assert_equal 80, http.port
     end
@@ -26,7 +26,7 @@ def test_no_explicit_https_port_number
       url.port = nil
 
       adapter = Faraday::Adapter::NetHttp.new
-      http = adapter.net_http_connection(:url => url, :request => {})
+      http = adapter.send(:net_http_connection, :url => url, :request => {})
 
       assert_equal 443, http.port
     end
@@ -35,7 +35,7 @@ def test_explicit_port_number
       url = URI('https://example.com:1234')
 
       adapter = Faraday::Adapter::NetHttp.new
-      http = adapter.net_http_connection(:url => url, :request => {})
+      http = adapter.send(:net_http_connection, :url => url, :request => {})
 
       assert_equal 1234, http.port
     end
@@ -47,8 +47,8 @@ def test_custom_adapter_config
         http.continue_timeout = 123
       end
 
-      http = adapter.net_http_connection(:url => url, :request => {})
-      adapter.configure_request(http, {})
+      http = adapter.send(:net_http_connection, :url => url, :request => {})
+      adapter.send(:configure_request, http, {})
 
       assert_equal 123, http.continue_timeout
     end