Be stricter of allowed values for timeout

Author: stoivo

lib/http/chainable.rb

@@ -90,25 +90,38 @@ def build_request(*args)
     # @overload timeout(global_timeout)
     #   Adds a global timeout to the full request
     #   @param [Numeric] global_timeout
+    PER_OPERATION_KEYS = Set.new(%i[read write connect])
     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

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
 

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