Cleanup adapter connections

lib/faraday/adapter.rb

@@ -46,6 +46,19 @@ def initialize(_app = nil, opts = {}, &block)
       @config_block = block
     end
 
+    # Yields an adapter's configured connection. Depends on #build_connection
+    # being defined on this adapter.
+    #
+    # @param env [Faraday::Env, Hash] The env object for a faraday request.
+    #
+    # @return An HTTP client connection for this adapter.
+    def connection(env)
+      conn = build_connection(env)
+      return conn unless block_given?
+
+      yield conn
+    end
+
     def call(env)
       env.clear_body if env.needs_body?
       env.response = Response.new

lib/faraday/adapter/excon.rb

@@ -6,12 +6,14 @@ class Adapter
     class Excon < Faraday::Adapter
       dependency 'excon'
 
+      def build_connection(env)
+        opts = opts_from_env(env)
+        ::Excon.new(env[:url].to_s, opts.merge(@connection_options))
+      end
+
       def call(env)
         super
 
-        opts = opts_from_env(env)
-        conn = create_connection(env, opts)
-
         req_opts = {
           method: env[:method].to_s.upcase,
           headers: env[:request_headers],
@@ -26,7 +28,7 @@ def call(env)
           end
         end
 
-        resp = conn.request(req_opts)
+        resp = connection(env) { |http| http.request(req_opts) }
         save_response(env, resp.status.to_i, resp.body, resp.headers,
                       resp.reason_phrase)
 
@@ -41,11 +43,6 @@ def call(env)
         raise Faraday::TimeoutError, e
       end
 
-      # @return [Excon]
-      def create_connection(env, opts)
-        ::Excon.new(env[:url].to_s, opts.merge(@connection_options))
-      end
-
       # TODO: support streaming requests
       def read_body(env)
         env[:body].respond_to?(:read) ? env[:body].read : env[:body]

lib/faraday/adapter/httpclient.rb

@@ -6,51 +6,54 @@ class Adapter
     class HTTPClient < Faraday::Adapter
       dependency 'httpclient'
 
-      # @return [HTTPClient]
-      def client
-        @client ||= ::HTTPClient.new
-      end
-
-      def call(env)
-        super
-
-        # enable compression
-        client.transparent_gzip_decompression = true
+      def build_connection(env)
+        @client ||= ::HTTPClient.new.tap do |cli|
+          # enable compression
+          cli.transparent_gzip_decompression = true
+        end
 
         if (req = env[:request])
           if (proxy = req[:proxy])
-            configure_proxy proxy
+            configure_proxy @client, proxy
           end
 
           if (bind = req[:bind])
-            configure_socket bind
+            configure_socket @client, bind
           end
 
-          configure_timeouts req
+          configure_timeouts @client, req
         end
 
         if env[:url].scheme == 'https' && (ssl = env[:ssl])
-          configure_ssl ssl
+          configure_ssl @client, ssl
         end
 
-        configure_client
+        configure_client @client
+
+        @client
+      end
+
+      def call(env)
+        super
 
         # TODO: Don't stream yet.
         # https://github.com/nahi/httpclient/pull/90
         env[:body] = env[:body].read if env[:body].respond_to? :read
 
-        resp = client.request env[:method], env[:url],
+        connection(env) do |http|
+          resp = http.request env[:method], env[:url],
                               body: env[:body],
                               header: env[:request_headers]
 
-        if (req = env[:request]).stream_response?
-          warn "Streaming downloads for #{self.class.name} " \
-            'are not yet implemented.'
-          req.on_data.call(resp.body, resp.body.bytesize)
-        end
-        save_response env, resp.status, resp.body, resp.headers, resp.reason
+          if (req = env[:request]).stream_response?
+            warn "Streaming downloads for #{self.class.name} " \
+              'are not yet implemented.'
+            req.on_data.call(resp.body, resp.body.bytesize)
+          end
+          save_response env, resp.status, resp.body, resp.headers, resp.reason
 
-        @app.call env
+          @app.call env
+        end
       rescue ::HTTPClient::TimeoutError, Errno::ETIMEDOUT
         raise Faraday::TimeoutError, $ERROR_INFO
       rescue ::HTTPClient::BadResponseError => e
@@ -71,23 +74,23 @@ def call(env)
       end
 
       # @param bind [Hash]
-      def configure_socket(bind)
+      def configure_socket(client, bind)
         client.socket_local.host = bind[:host]
         client.socket_local.port = bind[:port]
       end
 
       # Configure proxy URI and any user credentials.
       #
       # @param proxy [Hash]
-      def configure_proxy(proxy)
+      def configure_proxy(client, proxy)
         client.proxy = proxy[:uri]
         return unless proxy[:user] && proxy[:password]
 
         client.set_proxy_auth(proxy[:user], proxy[:password])
       end
 
       # @param ssl [Hash]
-      def configure_ssl(ssl)
+      def configure_ssl(client, ssl)
         ssl_config = client.ssl_config
         ssl_config.verify_mode = ssl_verify_mode(ssl)
         ssl_config.cert_store = ssl_cert_store(ssl)
@@ -100,23 +103,23 @@ def configure_ssl(ssl)
       end
 
       # @param req [Hash]
-      def configure_timeouts(req)
-        configure_timeout(req) if req[:timeout]
-        configure_open_timeout(req) if req[:open_timeout]
+      def configure_timeouts(client, req)
+        configure_timeout(client, req) if req[:timeout]
+        configure_open_timeout(client, req) if req[:open_timeout]
       end
 
-      def configure_timeout(req)
+      def configure_timeout(client, req)
         client.connect_timeout   = req[:timeout]
         client.receive_timeout   = req[:timeout]
         client.send_timeout      = req[:timeout]
       end
 
-      def configure_open_timeout(req)
+      def configure_open_timeout(client, req)
         client.connect_timeout   = req[:open_timeout]
         client.send_timeout      = req[:open_timeout]
       end
 
-      def configure_client
+      def configure_client(client)
         @config_block&.call(client)
       end
 

lib/faraday/adapter/net_http.rb

@@ -40,30 +40,46 @@ def initialize(app = nil, opts = {}, &block)
         super(app, opts, &block)
       end
 
-      def call(env)
-        super
-        with_net_http_connection(env) do |http|
-          if (env[:url].scheme == 'https') && env[:ssl]
-            configure_ssl(http, env[:ssl])
+      def build_connection(env)
+        net_http_connection(env).tap do |http|
+          if http.respond_to?(:use_ssl=)
+            http.use_ssl = env[:url].scheme == 'https'
           end
+          configure_ssl(http, env[:ssl])
           configure_request(http, env[:request])
+        end
+      end
 
+      def net_http_connection(env)
+        klass = if (proxy = env[:request][:proxy])
+                  Net::HTTP::Proxy(proxy[:uri].hostname, proxy[:uri].port,
+                                   proxy[:user], proxy[:password])
+                else
+                  Net::HTTP
+                end
+        port = env[:url].port || (env[:url].scheme == 'https' ? 443 : 80)
+        klass.new(env[:url].hostname, port)
+      end
+
+      def call(env)
+        super
+        http_response = connection(env) do |http|
           begin
-            http_response = perform_request(http, env)
+            perform_request(http, env)
           rescue *NET_HTTP_EXCEPTIONS => e
             if defined?(OpenSSL) && e.is_a?(OpenSSL::SSL::SSLError)
               raise Faraday::SSLError, e
             end
 
             raise Faraday::ConnectionFailed, e
           end
+        end
 
-          save_response(env, http_response.code.to_i,
-                        http_response.body || '', nil,
-                        http_response.message) do |response_headers|
-            http_response.each_header do |key, value|
-              response_headers[key] = value
-            end
+        save_response(env, http_response.code.to_i,
+                      http_response.body || '', nil,
+                      http_response.message) do |response_headers|
+          http_response.each_header do |key, value|
+            response_headers[key] = value
           end
         end
 
@@ -134,23 +150,9 @@ def request_via_request_method(http, env, &block)
         end
       end
 
-      def with_net_http_connection(env)
-        yield net_http_connection(env)
-      end
-
-      def net_http_connection(env)
-        klass = if (proxy = env[:request][:proxy])
-                  Net::HTTP::Proxy(proxy[:uri].hostname, proxy[:uri].port,
-                                   proxy[:user], proxy[:password])
-                else
-                  Net::HTTP
-                end
-        port = env[:url].port || (env[:url].scheme == 'https' ? 443 : 80)
-        klass.new(env[:url].hostname, port)
-      end
-
       def configure_ssl(http, ssl)
-        http.use_ssl = true
+        return unless ssl
+
         http.verify_mode = ssl_verify_mode(ssl)
         http.cert_store = ssl_cert_store(ssl)
 

lib/faraday/adapter/net_http_persistent.rb

@@ -73,6 +73,8 @@ def perform_request(http, env)
       }.freeze
 
       def configure_ssl(http, ssl)
+        return unless ssl
+
         http_set(http, :verify_mode, ssl_verify_mode(ssl))
         http_set(http, :cert_store, ssl_cert_store(ssl))
 

lib/faraday/adapter/patron.rb

@@ -6,11 +6,7 @@ class Adapter
     class Patron < Faraday::Adapter
       dependency 'patron'
 
-      def call(env)
-        super
-        # TODO: support streaming requests
-        env[:body] = env[:body].read if env[:body].respond_to? :read
-
+      def build_connection(env)
         session = ::Patron::Session.new
         @config_block&.call(session)
         if (env[:url].scheme == 'https') && env[:ssl]
@@ -33,12 +29,22 @@ def call(env)
           end
         end
 
-        response = begin
-          data = env[:body] ? env[:body].to_s : nil
-          session.request(env[:method], env[:url].to_s,
-                          env[:request_headers], data: data)
-                   rescue Errno::ECONNREFUSED, ::Patron::ConnectionFailed
-                     raise Faraday::ConnectionFailed, $ERROR_INFO
+        session
+      end
+
+      def call(env)
+        super
+        # TODO: support streaming requests
+        env[:body] = env[:body].read if env[:body].respond_to? :read
+
+        response = connection(env) do |session|
+          begin
+            data = env[:body] ? env[:body].to_s : nil
+            session.request(env[:method], env[:url].to_s,
+                            env[:request_headers], data: data)
+          rescue Errno::ECONNREFUSED, ::Patron::ConnectionFailed
+            raise Faraday::ConnectionFailed, $ERROR_INFO
+          end
         end
 
         if (req = env[:request]).stream_response?

spec/faraday/adapter/excon_spec.rb

@@ -10,7 +10,7 @@
 
     adapter = described_class.new(nil, debug_request: true)
 
-    conn = adapter.create_connection({ url: url }, {})
+    conn = adapter.build_connection(url: url)
 
     expect(conn.data[:debug_request]).to be_truthy
   end

spec/faraday/adapter/httpclient_spec.rb

@@ -12,9 +12,7 @@
       client.ssl_config.timeout = 25
     end
 
-    client = adapter.client
-    adapter.configure_client
-
+    client = adapter.build_connection(url: URI.parse('https://example.com'))
     expect(client.keep_alive_timeout).to eq(20)
     expect(client.ssl_config.timeout).to eq(25)
   end

spec/faraday/adapter/net_http_persistent_spec.rb

@@ -12,7 +12,7 @@
       http.idle_timeout = 123
     end
 
-    http = adapter.send(:net_http_connection, url: url, request: {})
+    http = adapter.send(:connection, url: url, request: {})
     adapter.send(:configure_request, http, {})
 
     expect(http.idle_timeout).to eq(123)
@@ -23,7 +23,7 @@
 
     adapter = described_class.new
 
-    http = adapter.send(:net_http_connection, url: url, request: {})
+    http = adapter.send(:connection, url: url, request: {})
     adapter.send(:configure_request, http, {})
 
     # `max_retries=` is only present in Ruby 2.5
@@ -35,7 +35,7 @@
 
     adapter = described_class.new(nil, pool_size: 5)
 
-    http = adapter.send(:net_http_connection, url: url, request: {})
+    http = adapter.send(:connection, url: url, request: {})
 
     # `pool` is only present in net_http_persistent >= 3.0
     expect(http.pool.size).to eq(5) if http.respond_to?(:pool)
@@ -47,7 +47,7 @@
 
       adapter = described_class.new(nil)
 
-      http = adapter.send(:net_http_connection, url: url, request: {})
+      http = adapter.send(:connection, url: url, request: {})
       adapter.send(:configure_ssl, http, min_version: :TLS1_2)
 
       # `min_version` is only present in net_http_persistent >= 3.1 (UNRELEASED)