diff --git a/lib/faraday/adapter.rb b/lib/faraday/adapter.rb index 6a3bb7d5c..959a98817 100644 --- a/lib/faraday/adapter.rb +++ b/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 diff --git a/lib/faraday/adapter/em_http.rb b/lib/faraday/adapter/em_http.rb index 897a8ef75..acadc35ff 100644 --- a/lib/faraday/adapter/em_http.rb +++ b/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 diff --git a/lib/faraday/adapter/excon.rb b/lib/faraday/adapter/excon.rb index 7610b53f9..8f8d86c8b 100644 --- a/lib/faraday/adapter/excon.rb +++ b/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) diff --git a/lib/faraday/adapter/httpclient.rb b/lib/faraday/adapter/httpclient.rb index e0ac23c7e..45e0d585a 100644 --- a/lib/faraday/adapter/httpclient.rb +++ b/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 diff --git a/lib/faraday/adapter/net_http.rb b/lib/faraday/adapter/net_http.rb index c1cf1eccb..5c46f6048 100644 --- a/lib/faraday/adapter/net_http.rb +++ b/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=) diff --git a/lib/faraday/adapter/patron.rb b/lib/faraday/adapter/patron.rb index 3020dcf4b..a009c50f6 100644 --- a/lib/faraday/adapter/patron.rb +++ b/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 = [ diff --git a/lib/faraday/adapter/rack.rb b/lib/faraday/adapter/rack.rb index c866792b9..6a237ee13 100644 --- a/lib/faraday/adapter/rack.rb +++ b/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) diff --git a/lib/faraday/options/request_options.rb b/lib/faraday/options/request_options.rb index cca000f8b..1a96fb8f9 100644 --- a/lib/faraday/options/request_options.rb +++ b/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 diff --git a/spec/faraday/adapter/em_http_spec.rb b/spec/faraday/adapter/em_http_spec.rb index efe61348d..74a287853 100644 --- a/spec/faraday/adapter/em_http_spec.rb +++ b/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 diff --git a/spec/faraday/adapter/excon_spec.rb b/spec/faraday/adapter/excon_spec.rb index fee13d052..d71ef8c44 100644 --- a/spec/faraday/adapter/excon_spec.rb +++ b/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 diff --git a/spec/faraday/adapter/httpclient_spec.rb b/spec/faraday/adapter/httpclient_spec.rb index d3fb5f0e5..cbc715562 100644 --- a/spec/faraday/adapter/httpclient_spec.rb +++ b/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 diff --git a/spec/faraday/adapter/net_http_spec.rb b/spec/faraday/adapter/net_http_spec.rb index 2197ab180..175f806a6 100644 --- a/spec/faraday/adapter/net_http_spec.rb +++ b/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') } diff --git a/spec/faraday/adapter_spec.rb b/spec/faraday/adapter_spec.rb new file mode 100644 index 000000000..22ef1d149 --- /dev/null +++ b/spec/faraday/adapter_spec.rb @@ -0,0 +1,55 @@ +# 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 + + it 'attempts unknown timeout type' do + expect { timeout(:unknown) }.to raise_error(ArgumentError) + end + + def timeout(type) + adapter.send(:request_timeout, type, request) + end + end +end diff --git a/spec/faraday/options/request_options_spec.rb b/spec/faraday/options/request_options_spec.rb index d73c7a89a..8c1bb9921 100644 --- a/spec/faraday/options/request_options_spec.rb +++ b/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)