Rework of timeout errors

lib/httpi.rb

@@ -86,11 +86,11 @@ module HTTPI
   DEFAULT_LOG_LEVEL = :debug
 
   class Error < StandardError; end
-  class TimeoutError < Error; end
   class NotSupportedError < Error; end
   class NotImplementedError < Error; end
 
   module ConnectionError; end
+  module TimeoutError; end
 
   class SSLError < Error
     def initialize(message = nil, original = $!)

lib/httpi/adapter/curb.rb

@@ -46,6 +46,9 @@ def request(method)
       rescue Curl::Err::ConnectionFailedError  # connection refused
         $!.extend ConnectionError
         raise
+      rescue Curl::Err::TimeoutError
+        $!.extend TimeoutError
+        raise
       end
 
       private
@@ -72,8 +75,9 @@ def setup_client
       def basic_setup
         @client.url = @request.url.to_s
         @client.proxy_url = @request.proxy.to_s if @request.proxy
-        @client.timeout = @request.read_timeout if @request.read_timeout
-        @client.connect_timeout = @request.open_timeout if @request.open_timeout
+        read_or_write_timeout = @request.read_timeout || @request.write_timeout
+        @client.timeout_ms = read_or_write_timeout * 1000 if read_or_write_timeout
+        @client.connect_timeout_ms = @request.open_timeout * 1000 if @request.open_timeout
         @client.headers = @request.headers.to_hash
         @client.verbose = false
         # cURL workaround

lib/httpi/adapter/em_http.rb

@@ -29,6 +29,8 @@ module Adapter
     # are supported by em-httprequest but not HTTPI.
     class EmHttpRequest < Base
 
+      class EmHttpTimeoutError < StandardError; end # Generic error for timeouts
+
       register :em_http, :deps => %w(em-synchrony em-synchrony/em-http em-http)
 
       def initialize(request)
@@ -48,6 +50,9 @@ def cert_directory
       # @see HTTPI.request
       def request(method)
         _request { |options| @client.send method, options }
+      rescue EmHttpTimeoutError
+        $!.extend TimeoutError
+        raise
       end
 
       private
@@ -69,10 +74,11 @@ def _request
       end
 
       def connection_options
-        options = {
-          :connect_timeout    => @request.open_timeout,
-          :inactivity_timeout => @request.read_timeout
-        }
+        options = {}
+
+        read_or_write_timeout = @request.read_timeout || @request.write_timeout
+        options[:inactivity_timeout] = read_or_write_timeout if read_or_write_timeout
+        options[:connect_timeout] = @request.open_timeout if @request.open_timeout
 
         options[:proxy] = proxy_options if @request.proxy
 
@@ -105,7 +111,7 @@ def setup_http_auth(options)
       end
 
       def respond_with(http, start_time)
-        raise TimeoutError, "EM-HTTP-Request connection timed out: #{Time.now - start_time} sec" if http.response_header.status.zero?
+        raise EmHttpTimeoutError, "EM-HTTP-Request connection timed out: #{Time.now - start_time} sec" if http.response_header.status.zero?
 
         Response.new http.response_header.status,
           convert_headers(http.response_header), http.response

lib/httpi/adapter/excon.rb

@@ -31,6 +31,9 @@ def request(method)
           $!.extend ConnectionError
         end
         raise
+      rescue ::Excon::Error::Timeout
+        $!.extend TimeoutError
+        raise
       end
 
       private
@@ -58,6 +61,7 @@ def client_opts
         opts[:user], opts[:password] = *@request.auth.credentials if @request.auth.basic?
         opts[:connect_timeout] = @request.open_timeout if @request.open_timeout
         opts[:read_timeout]    = @request.read_timeout if @request.read_timeout
+        opts[:write_timeout]   = @request.write_timeout if @request.write_timeout
         opts[:response_block]  = @request.on_body if @request.on_body
         opts[:proxy]           = @request.proxy if @request.proxy
 

lib/httpi/adapter/http.rb

@@ -32,13 +32,13 @@ def request(method)
         unless ::HTTP::Request::METHODS.include? method
           raise NotSupportedError, "http.rb does not support custom HTTP methods"
         end
-        response = begin
-          @client.send(method, @request.url, :body => @request.body)
-        rescue OpenSSL::SSL::SSLError
-          raise SSLError
-        end
-
+        response = @client.send(method, @request.url, :body => @request.body)
         Response.new(response.code, response.headers.to_h, response.body.to_s)
+      rescue OpenSSL::SSL::SSLError
+        raise SSLError
+      rescue ::HTTP::TimeoutError
+        $!.extend TimeoutError
+        raise
       end
 
       private
@@ -73,6 +73,12 @@ def create_client
           client = client.via(@request.proxy.host, @request.proxy.port, @request.proxy.user, @request.proxy.password)
         end
 
+        timeouts = {}
+        timeouts[:connect] = @request.open_timeout if @request.open_timeout
+        timeouts[:read] = @request.read_timeout if @request.read_timeout
+        timeouts[:write] = @request.write_timeout if @request.write_timeout
+        client = client.timeout(timeouts) if timeouts.any?
+
         client.headers(@request.headers)
       end
     end

lib/httpi/adapter/httpclient.rb

@@ -29,6 +29,9 @@ def request(method)
       rescue Errno::ECONNREFUSED   # connection refused
         $!.extend ConnectionError
         raise
+      rescue ::HTTPClient::TimeoutError
+        $!.extend TimeoutError
+        raise
       end
 
       private
@@ -45,6 +48,7 @@ def basic_setup
         @client.proxy = @request.proxy if @request.proxy
         @client.connect_timeout = @request.open_timeout if @request.open_timeout
         @client.receive_timeout = @request.read_timeout if @request.read_timeout
+        @client.send_timeout = @request.write_timeout if @request.write_timeout
       end
 
       def setup_auth

lib/httpi/adapter/net_http.rb

@@ -52,6 +52,9 @@ def request(method)
       rescue Errno::ECONNREFUSED   # connection refused
         $!.extend ConnectionError
         raise
+      rescue ::Timeout::Error
+        $!.extend TimeoutError
+        raise
       end
 
       private
@@ -155,6 +158,13 @@ def setup_client
         @client.use_ssl = @request.ssl?
         @client.open_timeout = @request.open_timeout if @request.open_timeout
         @client.read_timeout = @request.read_timeout if @request.read_timeout
+        if @request.write_timeout
+          if @client.respond_to?(:write_timeout=) # Expected to appear in Ruby 2.6
+            @client.write_timeout = @request.write_timeout
+          else
+            raise NotSupportedError, "Net::HTTP supports write_timeout starting from Ruby 2.6"
+          end
+        end
       end
 
       def setup_ssl_auth

lib/httpi/adapter/net_http_persistent.rb

@@ -9,6 +9,17 @@ class NetHTTPPersistent < NetHTTP
 
       register :net_http_persistent, :deps => %w(net/http/persistent)
 
+      # Executes arbitrary HTTP requests.
+      # @see HTTPI.request
+      def request(method)
+        super
+      rescue Net::HTTP::Persistent::Error => e
+        if !e.message.nil? && e.message =~ /Timeout/
+          $!.extend TimeoutError
+        end
+        raise
+      end
+
       private
 
       def create_client
@@ -32,6 +43,7 @@ def setup_client
 
         @client.open_timeout = @request.open_timeout if @request.open_timeout
         @client.read_timeout = @request.read_timeout if @request.read_timeout
+        raise NotSupportedError, "Net::HTTP::Persistent does not support write_timeout" if @request.write_timeout
       end
 
       def thread_key

lib/httpi/request.rb

@@ -11,7 +11,7 @@ module HTTPI
   class Request
 
     # Available attribute writers.
-    ATTRIBUTES = [:url, :proxy, :headers, :body, :open_timeout, :read_timeout, :follow_redirect, :redirect_limit, :query]
+    ATTRIBUTES = [:url, :proxy, :headers, :body, :open_timeout, :read_timeout, :write_timeout, :follow_redirect, :redirect_limit, :query]
 
     # Accepts a Hash of +args+ to mass assign attributes and authentication credentials.
     def initialize(args = {})
@@ -90,7 +90,7 @@ def set_cookies(object_or_array)
       headers["Cookie"] = cookies if cookies
     end
 
-    attr_accessor :open_timeout, :read_timeout
+    attr_accessor :open_timeout, :read_timeout, :write_timeout
     attr_reader :body
 
     # Sets a body request given a String or a Hash.

spec/httpi/adapter/curb_spec.rb

@@ -146,29 +146,36 @@
         end
       end
 
-      describe "timeout" do
+      describe "timeout_ms" do
         it "is not set unless it's specified" do
-          curb.expects(:timeout=).never
+          curb.expects(:timeout_ms=).never
           adapter.request(:get)
         end
 
-        it "is set if specified" do
+        it "is set if specified read_timeout" do
           request.read_timeout = 30
-          curb.expects(:timeout=).with(request.read_timeout)
+          curb.expects(:timeout_ms=).with(30_000)
+
+          adapter.request(:get)
+        end
+
+        it "is set if specified write_timeout" do
+          request.write_timeout = 30
+          curb.expects(:timeout_ms=).with(30_000)
 
           adapter.request(:get)
         end
       end
 
-      describe "connect_timeout" do
+      describe "connect_timeout_ms" do
         it "is not set unless it's specified" do
-          curb.expects(:connect_timeout=).never
+          curb.expects(:connect_timeout_ms=).never
           adapter.request(:get)
         end
 
         it "is set if specified" do
           request.open_timeout = 30
-          curb.expects(:connect_timeout=).with(30)
+          curb.expects(:connect_timeout_ms=).with(30_000)
 
           adapter.request(:get)
         end

spec/httpi/adapter/em_http_spec.rb

@@ -89,15 +89,12 @@
         end
 
         it "sets host, port and authorization" do
-          url = 'http://example.com:80'
-
+          url = "http://example.com:80"
           connection_options = {
-            :connect_timeout    => nil,
-            :inactivity_timeout => nil,
-            :proxy              => {
-              :host          => 'proxy-host.com',
-              :port          => 443,
-              :authorization => ['username', 'password']
+            :proxy => {
+              :host => "proxy-host.com",
+              :port => 443,
+              :authorization => ["username", "password"]
             }
           }
 
@@ -111,8 +108,8 @@
         it "is passed as a connection option" do
           request.open_timeout = 30
 
-          url = 'http://example.com:80'
-          connection_options = { :connect_timeout => 30, :inactivity_timeout => nil }
+          url = "http://example.com:80"
+          connection_options = { connect_timeout: 30 }
 
           EventMachine::HttpRequest.expects(:new).with(url, connection_options)
 
@@ -121,11 +118,22 @@
       end
 
       describe "receive_timeout" do
-        it "is passed as a connection option" do
+        it "is passed as a connection option (when read_timeout specified)" do
           request.read_timeout = 60
 
-          url = 'http://example.com:80'
-          connection_options = { :connect_timeout => nil, :inactivity_timeout => 60 }
+          url = "http://example.com:80"
+          connection_options = { inactivity_timeout: 60 }
+
+          EventMachine::HttpRequest.expects(:new).with(url, connection_options)
+
+          adapter
+        end
+
+        it "is passed as a connection option (when write_timeout specified)" do
+          request.write_timeout = 60
+
+          url = "http://example.com:80"
+          connection_options = { inactivity_timeout: 60 }
 
           EventMachine::HttpRequest.expects(:new).with(url, connection_options)