read timeout request option (part 2)

lib/faraday/adapter.rb

@@ -65,5 +65,27 @@ 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)
+      unless TIMEOUT_TYPES.include?(type)
+        msg = "Expected :read, :write, :open. Got #{type.inspect} :("
+        raise ArgumentError, msg
+      end
+
+      options["#{type}_timeout".to_sym] || options[:timeout]
+    end
+
+    TIMEOUT_TYPES = Set.new(%i[read write open])
   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

@@ -90,17 +90,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/net_http.rb

@@ -165,17 +165,17 @@ 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
+        if (sec = http.respond_to?(:write_timeout=) &&
+                  request_timeout(:write, req))
+          http.write_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 = 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/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
@@ -17,5 +18,7 @@ def []=(key, value)
     def stream_response?
       on_data.is_a?(Proc)
     end
+
+    TIMEOUT_TYPES = Set.new(%i[read write open])
   end
 end

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/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') }
 

spec/faraday/adapter_spec.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+RSpec.describe Faraday::Adapter do
+  let(:adapter) { Faraday::Adapter.new }
+  let(:request) { {} }
+
+  context '#request_timeout' do
+    it 'gets :read timeout' do
+      expect(timeout(:read)).to eq(nil)
+
+      request[:timeout] = 5
+      request[:write_timeout] = 1
+
+      expect(timeout(:read)).to eq(5)
+
+      request[:read_timeout] = 2
+
+      expect(timeout(:read)).to eq(2)
+    end
+
+    it 'gets :open timeout' do
+      expect(timeout(:open)).to eq(nil)
+
+      request[:timeout] = 5
+      request[:write_timeout] = 1
+
+      expect(timeout(:open)).to eq(5)
+
+      request[:open_timeout] = 2
+
+      expect(timeout(:open)).to eq(2)
+    end
+
+    it 'gets :write timeout' do
+      expect(timeout(:write)).to eq(nil)
+
+      request[:timeout] = 5
+      request[:read_timeout] = 1
+
+      expect(timeout(:write)).to eq(5)
+
+      request[:write_timeout] = 2
+
+      expect(timeout(:write)).to eq(2)
+    end
+
+    def timeout(type)
+      adapter.send(:request_timeout, type, request)
+    end
+  end
+end

spec/faraday/options/request_options_spec.rb

@@ -1,8 +1,9 @@
 # frozen_string_literal: true
 
 RSpec.describe Faraday::RequestOptions do
+  subject(:options) { Faraday::RequestOptions.new }
+
   it 'allows to set the request proxy' do
-    options = Faraday::RequestOptions.new
     expect(options.proxy).to be_nil
 
     expect { options[:proxy] = { booya: 1 } }.to raise_error(NoMethodError)