Merge pull request #1022 from lostisland/springerigor-read-timeout-re…

…quest-option

read timeout request option (part 2)

lib/faraday/adapter.rb

@@ -65,5 +65,30 @@ def save_response(env, status, body, headers = nil, reason_phrase = nil)
       env.response.finish(env) unless env.parallel?
       env.response
     end
+
+    # Fetches either a read, write, or open timeout setting. Defaults to the
+    # :timeout value if a more specific one is not given.
+    #
+    # @param type [Symbol] Describes which timeout setting to get: :read,
+    #                      :write, or :open.
+    # @param options [Hash] Hash containing Symbol keys like :timeout,
+    #                       :read_timeout, :write_timeout, :open_timeout, or
+    #                       :timeout
+    #
+    # @return [Integer, nil] Timeout duration in seconds, or nil if no timeout
+    #                        has been set.
+    def request_timeout(type, options)
+      key = TIMEOUT_KEYS.fetch(type) do
+        msg = "Expected :read, :write, :open. Got #{type.inspect} :("
+        raise ArgumentError, msg
+      end
+      options[key] || options[:timeout]
+    end
+
+    TIMEOUT_KEYS = {
+      read: :read_timeout,
+      open: :open_timeout,
+      write: :write_timeout
+    }.freeze
   end
 end

lib/faraday/adapter/em_http.rb

@@ -70,10 +70,9 @@ def configure_ssl(options, env)
 
         # Reads out timeout settings from env into options
         def configure_timeout(options, env)
-          timeout, open_timeout = request_options(env)
-                                  .values_at(:timeout, :open_timeout)
-          options[:connect_timeout] = options[:inactivity_timeout] = timeout
-          options[:connect_timeout] = open_timeout if open_timeout
+          req = request_options(env)
+          options[:inactivity_timeout] = request_timeout(:read, req)
+          options[:connect_timeout] = request_timeout(:open, req)
         end
 
         # Reads out compression header settings from env into options

lib/faraday/adapter/excon.rb

@@ -95,17 +95,17 @@ def amend_opts_with_ssl!(opts, ssl)
       end
 
       def amend_opts_with_timeouts!(opts, req)
-        timeout = req[:timeout]
-        return unless timeout
+        if (sec = request_timeout(:read, req))
+          opts[:read_timeout] = sec
+        end
 
-        opts[:read_timeout] = timeout
-        opts[:connect_timeout] = timeout
-        opts[:write_timeout] = timeout
+        if (sec = request_timeout(:write, req))
+          opts[:write_timeout] = sec
+        end
 
-        open_timeout = req[:open_timeout]
-        return unless open_timeout
+        return unless (sec = request_timeout(:open, req))
 
-        opts[:connect_timeout] = open_timeout
+        opts[:connect_timeout] = sec
       end
 
       def amend_opts_with_proxy_settings!(opts, req)

lib/faraday/adapter/httpclient.rb

@@ -101,19 +101,17 @@ def configure_ssl(ssl)
 
       # @param req [Hash]
       def configure_timeouts(req)
-        configure_timeout(req) if req[:timeout]
-        configure_open_timeout(req) if req[:open_timeout]
-      end
+        if (sec = request_timeout(:open, req))
+          client.connect_timeout = sec
+        end
 
-      def configure_timeout(req)
-        client.connect_timeout   = req[:timeout]
-        client.receive_timeout   = req[:timeout]
-        client.send_timeout      = req[:timeout]
-      end
+        if (sec = request_timeout(:write, req))
+          client.send_timeout = sec
+        end
+
+        return unless (sec = request_timeout(:read, req))
 
-      def configure_open_timeout(req)
-        client.connect_timeout   = req[:open_timeout]
-        client.send_timeout      = req[:open_timeout]
+        client.receive_timeout = sec
       end
 
       def configure_client

lib/faraday/adapter/net_http.rb

@@ -165,17 +165,19 @@ def configure_ssl(http, ssl)
       end
 
       def configure_request(http, req)
-        if req[:timeout]
-          http.read_timeout = req[:timeout]
-          http.open_timeout = req[:timeout]
-          if http.respond_to?(:write_timeout=)
-            http.write_timeout = req[:timeout]
-          end
+        if (sec = request_timeout(:read, req))
+          http.read_timeout = sec
         end
-        http.open_timeout = req[:open_timeout] if req[:open_timeout]
-        if req[:write_timeout] && http.respond_to?(:write_timeout=)
-          http.write_timeout = req[:write_timeout]
+
+        if (sec = http.respond_to?(:write_timeout=) &&
+                  request_timeout(:write, req))
+          http.write_timeout = sec
         end
+
+        if (sec = request_timeout(:open, req))
+          http.open_timeout = sec
+        end
+
         # Only set if Net::Http supports it, since Ruby 2.5.
         http.max_retries = 0 if http.respond_to?(:max_retries=)
 

lib/faraday/adapter/patron.rb

@@ -18,19 +18,8 @@ def call(env)
         end
 
         if (req = env[:request])
-          if req[:timeout]
-            session.timeout = session.connect_timeout = req[:timeout]
-          end
-          session.connect_timeout = req[:open_timeout] if req[:open_timeout]
-
-          if (proxy = req[:proxy])
-            proxy_uri = proxy[:uri].dup
-            proxy_uri.user = proxy[:user] &&
-                             Utils.escape(proxy[:user]).gsub('+', '%20')
-            proxy_uri.password = proxy[:password] &&
-                                 Utils.escape(proxy[:password]).gsub('+', '%20')
-            session.proxy = proxy_uri.to_s
-          end
+          configure_timeouts(session, req)
+          configure_proxy(session, req[:proxy])
         end
 
         response = begin
@@ -91,6 +80,29 @@ def configure_ssl(session, ssl)
         end
       end
 
+      def configure_timeouts(session, req)
+        return unless req
+
+        if (sec = request_timeout(:read, req))
+          session.timeout = sec
+        end
+
+        return unless (sec = request_timeout(:open, req))
+
+        session.connect_timeout = sec
+      end
+
+      def configure_proxy(session, proxy)
+        return unless proxy
+
+        proxy_uri = proxy[:uri].dup
+        proxy_uri.user = proxy[:user] &&
+                         Utils.escape(proxy[:user]).gsub('+', '%20')
+        proxy_uri.password = proxy[:password] &&
+                             Utils.escape(proxy[:password]).gsub('+', '%20')
+        session.proxy = proxy_uri.to_s
+      end
+
       private
 
       CURL_TIMEOUT_MESSAGES = [

lib/faraday/adapter/rack.rb

@@ -37,7 +37,8 @@ def call(env)
           rack_env[name] = value
         end
 
-        timeout  = env[:request][:timeout] || env[:request][:open_timeout]
+        timeout = request_timeout(:open, env[:request])
+        timeout ||= request_timeout(:read, env[:request])
         response = if timeout
                      Timer.timeout(timeout, Faraday::TimeoutError) do
                        execute_request(env, rack_env)

lib/faraday/options/request_options.rb

@@ -3,8 +3,9 @@
 module Faraday
   # RequestOptions contains the configurable properties for a Faraday request.
   class RequestOptions < Options.new(:params_encoder, :proxy, :bind,
-                                     :timeout, :open_timeout, :write_timeout,
-                                     :boundary, :oauth, :context, :on_data)
+                                     :timeout, :open_timeout, :read_timeout,
+                                     :write_timeout, :boundary, :oauth,
+                                     :context, :on_data)
 
     def []=(key, value)
       if key && key.to_sym == :proxy

spec/faraday/adapter/em_http_spec.rb

@@ -13,4 +13,35 @@
 
     expect(req.connopts.inactivity_timeout).to eq(20)
   end
+
+  context 'Options' do
+    let(:request) { Faraday::RequestOptions.new }
+    let(:env) { { request: request } }
+    let(:options) { {} }
+    let(:adapter) { Faraday::Adapter::EMHttp.new }
+
+    it 'configures timeout' do
+      request.timeout = 5
+      adapter.configure_timeout(options, env)
+      expect(options[:inactivity_timeout]).to eq(5)
+      expect(options[:connect_timeout]).to eq(5)
+    end
+
+    it 'configures timeout and open_timeout' do
+      request.timeout = 5
+      request.open_timeout = 1
+      adapter.configure_timeout(options, env)
+      expect(options[:inactivity_timeout]).to eq(5)
+      expect(options[:connect_timeout]).to eq(1)
+    end
+
+    it 'configures all timeout settings' do
+      request.timeout = 5
+      request.read_timeout = 3
+      request.open_timeout = 1
+      adapter.configure_timeout(options, env)
+      expect(options[:inactivity_timeout]).to eq(3)
+      expect(options[:connect_timeout]).to eq(1)
+    end
+  end
 end

spec/faraday/adapter/excon_spec.rb

@@ -14,4 +14,36 @@
 
     expect(conn.data[:debug_request]).to be_truthy
   end
+
+  context 'config' do
+    let(:adapter) { Faraday::Adapter::Excon.new }
+    let(:request) { Faraday::RequestOptions.new }
+    let(:uri) { URI.parse('https://example.com') }
+    let(:env) { { request: request, url: uri } }
+
+    it 'sets timeout' do
+      request.timeout = 5
+      options = adapter.send(:opts_from_env, env)
+      expect(options[:read_timeout]).to eq(5)
+      expect(options[:write_timeout]).to eq(5)
+      expect(options[:connect_timeout]).to eq(5)
+    end
+
+    it 'sets timeout and open_timeout' do
+      request.timeout = 5
+      request.open_timeout = 3
+      options = adapter.send(:opts_from_env, env)
+      expect(options[:read_timeout]).to eq(5)
+      expect(options[:write_timeout]).to eq(5)
+      expect(options[:connect_timeout]).to eq(3)
+    end
+
+    it 'sets open_timeout' do
+      request.open_timeout = 3
+      options = adapter.send(:opts_from_env, env)
+      expect(options[:read_timeout]).to eq(nil)
+      expect(options[:write_timeout]).to eq(nil)
+      expect(options[:connect_timeout]).to eq(3)
+    end
+  end
 end

spec/faraday/adapter/httpclient_spec.rb

@@ -1,6 +1,11 @@
 # frozen_string_literal: true
 
 RSpec.describe Faraday::Adapter::HTTPClient do
+  # ruby gem defaults for testing purposes
+  HTTPCLIENT_OPEN = 60
+  HTTPCLIENT_READ = 60
+  HTTPCLIENT_WRITE = 120
+
   features :request_body_on_query_methods, :reason_phrase_parse, :compression,
            :trace_method, :connect_method, :local_socket_binding
 
@@ -18,4 +23,53 @@
     expect(client.keep_alive_timeout).to eq(20)
     expect(client.ssl_config.timeout).to eq(25)
   end
+
+  context 'Options' do
+    let(:request) { Faraday::RequestOptions.new }
+    let(:env) { { request: request } }
+    let(:options) { {} }
+    let(:adapter) { Faraday::Adapter::HTTPClient.new }
+    let(:client) { adapter.client }
+
+    it 'configures timeout' do
+      assert_default_timeouts!
+
+      request.timeout = 5
+      adapter.configure_timeouts(request)
+
+      expect(client.connect_timeout).to eq(5)
+      expect(client.send_timeout).to eq(5)
+      expect(client.receive_timeout).to eq(5)
+    end
+
+    it 'configures open timeout' do
+      assert_default_timeouts!
+
+      request.open_timeout = 1
+      adapter.configure_timeouts(request)
+
+      expect(client.connect_timeout).to eq(1)
+      expect(client.send_timeout).to eq(HTTPCLIENT_WRITE)
+      expect(client.receive_timeout).to eq(HTTPCLIENT_READ)
+    end
+
+    it 'configures multiple timeouts' do
+      assert_default_timeouts!
+
+      request.open_timeout = 1
+      request.write_timeout = 10
+      request.read_timeout = 5
+      adapter.configure_timeouts(request)
+
+      expect(client.connect_timeout).to eq(1)
+      expect(client.send_timeout).to eq(10)
+      expect(client.receive_timeout).to eq(5)
+    end
+
+    def assert_default_timeouts!
+      expect(client.connect_timeout).to eq(HTTPCLIENT_OPEN)
+      expect(client.send_timeout).to eq(HTTPCLIENT_WRITE)
+      expect(client.receive_timeout).to eq(HTTPCLIENT_READ)
+    end
+  end
 end

spec/faraday/adapter/net_http_spec.rb

@@ -11,17 +11,31 @@
     let(:http) { adapter.send(:net_http_connection, url: url, request: {}) }
 
     it { expect(http.port).to eq(80) }
+
     it 'sets max_retries to 0' do
       adapter.send(:configure_request, http, {})
 
       expect(http.max_retries).to eq(0) if http.respond_to?(:max_retries=)
     end
+
     it 'supports write_timeout' do
       adapter.send(:configure_request, http, write_timeout: 10)
 
       expect(http.write_timeout).to eq(10) if http.respond_to?(:write_timeout=)
     end
 
+    it 'supports open_timeout' do
+      adapter.send(:configure_request, http, open_timeout: 10)
+
+      expect(http.open_timeout).to eq(10)
+    end
+
+    it 'supports read_timeout' do
+      adapter.send(:configure_request, http, read_timeout: 10)
+
+      expect(http.read_timeout).to eq(10)
+    end
+
     context 'with https url' do
       let(:url) { URI('https://example.com') }