diff --git a/lib/http/chainable.rb b/lib/http/chainable.rb index 7283e5bc..00188d27 100644 --- a/lib/http/chainable.rb +++ b/lib/http/chainable.rb @@ -92,23 +92,35 @@ def build_request(*args) # @param [Numeric] global_timeout def timeout(options) klass, options = case options - when Numeric then [HTTP::Timeout::Global, {global_timeout: options}] + when Numeric then [HTTP::Timeout::Global, {:global_timeout => options}] when Hash - options = options.dup - %i[read write connect].each do |k| - next unless options.key? k + options = options.dup + %i[read write connect].each do |k| + next unless options.key? k - options["#{k}_timeout".to_sym] = options.delete k - end + if options.key?("#{k}_timeout".to_sym) + raise ArgumentError, "can't pass both #{k} and #{"#{k}_timeout".to_sym}" + end - [HTTP::Timeout::PerOperation, options.dup] - when :null then [HTTP::Timeout::Null, {}] + options["#{k}_timeout".to_sym] = options.delete k + end + + options.each do |key, value| + unless HTTP::Timeout::PerOperation::SETTINGS.member?(key) && value.is_a?(Numeric) + raise ArgumentError, "invalid option #{key.inspect}, must be numeric " \ + "`.timeout(connect: x, write: y, read: z)`." + end + end + + raise ArgumentError, "at least one option" if options.empty? + + [HTTP::Timeout::PerOperation, options.dup] + when :null then [HTTP::Timeout::Null, {}] else raise ArgumentError, "Use `.timeout(:null)`, " \ "`.timeout(global_timeout_in_seconds)` or " \ "`.timeout(connect: x, write: y, read: z)`." end - branch default_options.merge( :timeout_class => klass, :timeout_options => options diff --git a/lib/http/timeout/per_operation.rb b/lib/http/timeout/per_operation.rb index 9274325e..f80d751f 100644 --- a/lib/http/timeout/per_operation.rb +++ b/lib/http/timeout/per_operation.rb @@ -11,6 +11,7 @@ class PerOperation < Null WRITE_TIMEOUT = 0.25 READ_TIMEOUT = 0.25 + SETTINGS = Set.new(%i[read_timeout write_timeout connect_timeout]) def initialize(*args) super diff --git a/spec/lib/http_spec.rb b/spec/lib/http_spec.rb index f7a1bb21..dc50597c 100644 --- a/spec/lib/http_spec.rb +++ b/spec/lib/http_spec.rb @@ -304,8 +304,49 @@ end it "sets given timeout options" do + client = HTTP.timeout :read => 125 + expect(client.default_options.timeout_options). - to eq :read_timeout => 123 + to eq :read_timeout => 125 + end + + it "sets given timeout options" do + client = HTTP.timeout :read_timeout => 321 + + expect(client.default_options.timeout_options). + to eq :read_timeout => 321 + end + + it "sets all available options" do + client = HTTP.timeout :read => 123, :write => 12, :connect => 1 + + expect(client.default_options.timeout_options). + to eq(:connect_timeout => 1, :write_timeout => 12, :read_timeout => 123) + end + + it "raises an error is empty hash is passed" do + expect { HTTP.timeout({}) } + .to raise_error(ArgumentError) + end + + it "raises if an invalid key is passed" do + expect { HTTP.timeout({:timeout => 2}) } + .to raise_error(ArgumentError) + end + + it "raises if both read and read_timeout is passed" do + expect { HTTP.timeout({:read => 2, :read_timeout => 2}) } + .to raise_error(ArgumentError) + end + + it "raises if a string is passed as read timeout" do + expect { HTTP.timeout({:connect => 1, :read => "2"}) } + .to raise_error(ArgumentError) + end + + it "don't accept string keys" do + expect { HTTP.timeout({:connect => 1, "read" => 2}) } + .to raise_error(ArgumentError) end end @@ -330,6 +371,18 @@ expect(client.default_options.timeout_options). to eq :global_timeout => 123 end + + it "accepts floats argument" do + client = HTTP.timeout 2.5 + + expect(client.default_options.timeout_options). + to eq(:global_timeout => 2.5) + end + + it "raises expect if a string is passed" do + expect { HTTP.timeout "1" } + .to raise_error(ArgumentError) + end end end