Rework of timeout errors (part 2): add support for new write_timeout option to all adapters + fix integration tests duplication

lib/httpi/adapter/curb.rb

@@ -72,8 +72,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

@@ -69,10 +69,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
 

lib/httpi/adapter/excon.rb

@@ -58,6 +58,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

@@ -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

@@ -45,6 +45,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

@@ -155,6 +155,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

@@ -32,6 +32,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)
 

spec/httpi/adapter/excon_spec.rb

@@ -1,124 +1,28 @@
 require "spec_helper"
-require "integration/support/server"
-
-describe HTTPI::Adapter::Excon do
-
-  subject(:adapter) { :excon }
-
-  context "http requests" do
-    before :all do
-      @server = IntegrationServer.run
-    end
-
-    after :all do
-      @server.stop
-    end
-
-    it "sends and receives HTTP headers" do
-      request = HTTPI::Request.new(@server.url + "x-header")
-      request.headers["X-Header"] = "HTTPI"
-
-      response = HTTPI.get(request, adapter)
-      expect(response.body).to include("HTTPI")
-    end
-
-    it "executes GET requests" do
-      response = HTTPI.get(@server.url, adapter)
-      expect(response.body).to eq("get")
-      expect(response.headers["Content-Type"]).to eq("text/plain")
-    end
-
-    it "executes POST requests" do
-      response = HTTPI.post(@server.url, "<some>xml</some>", adapter)
-      expect(response.body).to eq("post")
-      expect(response.headers["Content-Type"]).to eq("text/plain")
-    end
-
-    it "executes HEAD requests" do
-      response = HTTPI.head(@server.url, adapter)
-      expect(response.code).to eq(200)
-      expect(response.headers["Content-Type"]).to eq("text/plain")
-    end
-
-    it "executes PUT requests" do
-      response = HTTPI.put(@server.url, "<some>xml</some>", adapter)
-      expect(response.body).to eq("put")
-      expect(response.headers["Content-Type"]).to eq("text/plain")
-    end
-
-    it "executes DELETE requests" do
-      response = HTTPI.delete(@server.url, adapter)
-      expect(response.body).to eq("delete")
-      expect(response.headers["Content-Type"]).to eq("text/plain")
-    end
-
-    it "supports basic authentication" do
-      request = HTTPI::Request.new(@server.url + "basic-auth")
-      request.auth.basic("admin", "secret")
-
-      response = HTTPI.get(request, adapter)
-      expect(response.body).to eq("basic-auth")
-    end
-
-    it "does not support ntlm authentication" do
-      request = HTTPI::Request.new(@server.url + "ntlm-auth")
-      request.auth.ntlm("tester", "vReqSoafRe5O")
-
-      expect { HTTPI.get(request, adapter) }.
-        to raise_error(HTTPI::NotSupportedError, /does not support NTLM authentication/)
-    end
-
-    it "supports disabling verify mode" do
-      request = HTTPI::Request.new(@server.url)
-      request.auth.ssl.verify_mode = :none
-      adapter_class = HTTPI::Adapter.load(adapter).new(request)
-      expect(adapter_class.client.data[:ssl_verify_peer]).to eq(false)
-    end
-  end
-
-  # it does not support digest auth
-
-  if RUBY_PLATFORM =~ /java/
-    pending "Puma Server complains: SSL not supported on JRuby"
-  else
-    context "https requests" do
-      before :all do
-        @server = IntegrationServer.run(:ssl => true)
-      end
-      after :all do
-        @server.stop
-      end
-
-      # it does not raise when no certificate was set up
-      it "works when no client cert is specified" do
-        request = HTTPI::Request.new(@server.url)
-        request.auth.ssl.ca_cert_file = IntegrationServer.ssl_ca_file
-
-        response = HTTPI.get(request, adapter)
-        expect(response.body).to eq("get")
-      end
-
-      it "works with client cert and key provided as file path" do
-        request = HTTPI::Request.new(@server.url)
-        request.auth.ssl.ca_cert_file = IntegrationServer.ssl_ca_file
-        request.auth.ssl.cert_file = "spec/fixtures/client_cert.pem"
-        request.auth.ssl.cert_key_file = "spec/fixtures/client_key.pem"
-
-        response = HTTPI.get(request, adapter)
-        expect(response.body).to eq("get")
-      end
-
-      it "works with client cert and key set directly" do
-        request = HTTPI::Request.new(@server.url)
-
-        request.auth.ssl.ca_cert_file = IntegrationServer.ssl_ca_file
-        request.auth.ssl.cert = OpenSSL::X509::Certificate.new File.open("spec/fixtures/client_cert.pem").read
-        request.auth.ssl.cert_key = OpenSSL::PKey.read File.open("spec/fixtures/client_key.pem").read
-
-        response = HTTPI.get(request, adapter)
-        expect(response.body).to eq("get")
+require "httpi/adapter/excon"
+require "httpi/request"
+
+begin
+  HTTPI::Adapter.load_adapter(:excon)
+
+  describe HTTPI::Adapter::Excon do
+    let(:adapter) { HTTPI::Adapter::Excon.new(request) }
+    let(:request) { HTTPI::Request.new("http://example.com") }
+
+    describe "settings" do
+      describe "connect_timeout, read_timeout, write_timeout" do
+        it "are passed as connection options" do
+          request.open_timeout = 30
+          request.read_timeout = 40
+          request.write_timeout = 50
+
+          expect(adapter.client.data).to include(
+            connect_timeout: 30,
+            read_timeout: 40,
+            write_timeout: 50
+          )
+        end
       end
     end
   end
-
 end