Merge branch 'master' into rubocop-style-classvars

README.md

@@ -73,6 +73,23 @@ conn = Faraday.new(:url => 'http://sushi.com/api_key=s3cr3t') do |faraday|
   end
   faraday.adapter  Faraday.default_adapter  # make requests with Net::HTTP
 end
+
+# Override the log formatting on demand
+ 
+class MyFormatter < Faraday::Response::Logger::Formatter
+  def request(env)
+    info('Request', env)  
+  end
+  
+  def request(env)
+    info('Response', env)
+  end
+end
+
+conn = Faraday.new(:url => 'http://sushi.com/api_key=s3cr3t') do |faraday|
+  faraday.response :logger, StructLogger.new(STDOUT), formatter: MyFormatter
+end
+
 ```
 
 Once you have the connection object, use it to make HTTP requests. You can pass parameters to it in a few different ways:

lib/faraday/adapter/em_http.rb

@@ -166,17 +166,17 @@ module Faraday
       end
 
       def raise_error(msg)
-        errklass = Faraday::ClientError
-        if msg == Errno::ETIMEDOUT
-          errklass = Faraday::TimeoutError
+        error_class = Faraday::ClientError
+        if msg == Errno::ETIMEDOUT || (msg.is_a?(String) && msg.include?('timeout error'))
+          error_class = Faraday::TimeoutError
           msg = 'request timed out'
         elsif msg == Errno::ECONNREFUSED
-          errklass = Faraday::ConnectionFailed
+          error_class = Faraday::ConnectionFailed
           msg = 'connection refused'
         elsif msg == 'connection closed by server'
-          errklass = Faraday::ConnectionFailed
+          error_class = Faraday::ConnectionFailed
         end
-        raise errklass, msg
+        raise error_class, msg
       end
 
       # @return [Boolean]

lib/faraday/logging/formatter.rb

@@ -0,0 +1,76 @@
+# frozen_string_literal: true
+
+require 'pp'
+module Faraday
+  module Logging
+    # Serves as an integration point to customize logging
+    class Formatter
+      extend Forwardable
+
+      DEFAULT_OPTIONS = { headers: true, bodies: false }.freeze
+
+      def initialize(logger:, options:)
+        @logger = logger
+        @filter = []
+        @options = DEFAULT_OPTIONS.merge(options)
+      end
+
+      def_delegators :@logger, :debug, :info, :warn, :error, :fatal
+
+      def request(env)
+        info('request')  { "#{env.method.upcase} #{apply_filters(env.url.to_s)}" }
+        debug('request') { apply_filters(dump_headers(env.request_headers)) } if log_headers?(:request)
+        debug('request') { apply_filters(dump_body(env[:body])) } if env[:body] && log_body?(:request)
+      end
+
+      def response(env)
+        info('response')  { "Status #{env.status}" }
+        debug('response') { apply_filters(dump_headers(env.response_headers)) } if log_headers?(:response)
+        debug('response') { apply_filters(dump_body(env[:body])) } if env[:body] && log_body?(:response)
+      end
+
+      def filter(filter_word, filter_replacement)
+        @filter.push([filter_word, filter_replacement])
+      end
+
+      private
+
+      def dump_headers(headers)
+        headers.map { |k, v| "#{k}: #{v.inspect}" }.join("\n")
+      end
+
+      def dump_body(body)
+        if body.respond_to?(:to_str)
+          body.to_str
+        else
+          pretty_inspect(body)
+        end
+      end
+
+      def pretty_inspect(body)
+        body.pretty_inspect
+      end
+
+      def log_headers?(type)
+        case @options[:headers]
+        when Hash then @options[:headers][type]
+        else @options[:headers]
+        end
+      end
+
+      def log_body?(type)
+        case @options[:bodies]
+        when Hash then @options[:bodies][type]
+        else @options[:bodies]
+        end
+      end
+
+      def apply_filters(output)
+        @filter.each do |pattern, replacement|
+          output = output.to_s.gsub(pattern, replacement)
+        end
+        output
+      end
+    end
+  end
+end

lib/faraday/response/logger.rb

@@ -1,82 +1,29 @@
 # frozen_string_literal: true
 
 require 'forwardable'
+require 'faraday/logging/formatter'
 
 module Faraday
   class Response
     class Logger < Middleware
-      extend Forwardable
-
-      DEFAULT_OPTIONS = { headers: true, bodies: false }.freeze
-
       def initialize(app, logger = nil, options = {})
         super(app)
-        @logger = logger || begin
+        logger ||= begin
           require 'logger'
           ::Logger.new($stdout)
         end
-        @filter = []
-        @options = DEFAULT_OPTIONS.merge(options)
-        yield self if block_given?
+        formatter_class = options.delete(:formatter) || Logging::Formatter
+        @formatter = formatter_class.new(logger: logger, options: options)
+        yield @formatter if block_given?
       end
 
-      def_delegators :@logger, :debug, :info, :warn, :error, :fatal
-
       def call(env)
-        info('request')  { "#{env.method.upcase} #{apply_filters(env.url.to_s)}" }
-        debug('request') { apply_filters(dump_headers(env.request_headers)) } if log_headers?(:request)
-        debug('request') { apply_filters(dump_body(env[:body])) } if env[:body] && log_body?(:request)
+        @formatter.request(env)
         super
       end
 
       def on_complete(env)
-        info('response')  { "Status #{env.status}" }
-        debug('response') { apply_filters(dump_headers(env.response_headers)) } if log_headers?(:response)
-        debug('response') { apply_filters(dump_body(env[:body])) } if env[:body] && log_body?(:response)
-      end
-
-      def filter(filter_word, filter_replacement)
-        @filter.push([filter_word, filter_replacement])
-      end
-
-      private
-
-      def dump_headers(headers)
-        headers.map { |k, v| "#{k}: #{v.inspect}" }.join("\n")
-      end
-
-      def dump_body(body)
-        if body.respond_to?(:to_str)
-          body.to_str
-        else
-          pretty_inspect(body)
-        end
-      end
-
-      def pretty_inspect(body)
-        require 'pp' unless body.respond_to?(:pretty_inspect)
-        body.pretty_inspect
-      end
-
-      def log_headers?(type)
-        case @options[:headers]
-        when Hash then @options[:headers][type]
-        else @options[:headers]
-        end
-      end
-
-      def log_body?(type)
-        case @options[:bodies]
-        when Hash then @options[:bodies][type]
-        else @options[:bodies]
-        end
-      end
-
-      def apply_filters(output)
-        @filter.each do |pattern, replacement|
-          output = output.to_s.gsub(pattern, replacement)
-        end
-        output
+        @formatter.response(env)
       end
     end
   end

spec/faraday/adapter/em_http_spec.rb

@@ -0,0 +1,16 @@
+# frozen_string_literal: true
+
+RSpec.describe Faraday::Adapter::EMHttp do
+  features :request_body_on_query_methods, :reason_phrase_parse, :trace_method, :connect_method,
+           :skip_response_body_on_head, :parallel
+
+  it_behaves_like 'an adapter'
+
+  it 'allows to provide adapter specific configs' do
+    url = URI('https://example.com:1234')
+    adapter = Faraday::Adapter::EMHttp.new nil, inactivity_timeout: 20
+    req = adapter.create_request(url: url, request: {})
+
+    expect(req.connopts.inactivity_timeout).to eq(20)
+  end
+end

spec/faraday/response/logger_spec.rb

@@ -38,6 +38,61 @@ RSpec.describe Faraday::Response::Logger do
     expect(resp.body).to eq('hello')
   end
 
+  context 'without configuration' do
+    let(:conn) do
+      Faraday.new do |b|
+        b.response :logger
+        b.adapter :test do |stubs|
+          stubs.get('/hello') { [200, { 'Content-Type' => 'text/html' }, 'hello'] }
+        end
+      end
+    end
+
+    it 'defaults to stdout' do
+      expect(Logger).to receive(:new).with($stdout).and_return(Logger.new(nil))
+      conn.get('/hello')
+    end
+  end
+
+  context 'with default formatter' do
+    let(:formatter) { instance_double(Faraday::Logging::Formatter, request: true, response: true, filter: []) }
+
+    before { allow(Faraday::Logging::Formatter).to receive(:new).and_return(formatter) }
+
+    it 'delegates logging to the formatter' do
+      expect(formatter).to receive(:request).with(an_instance_of(Faraday::Env))
+      expect(formatter).to receive(:response).with(an_instance_of(Faraday::Env))
+      conn.get '/hello'
+    end
+  end
+
+  context 'with custom formatter' do
+    let(:formatter_class) do
+      Class.new(Faraday::Logging::Formatter) do
+        def initialize(*args)
+          super
+        end
+
+        def request(_env)
+          info 'Custom log formatter request'
+        end
+
+        def response(_env)
+          info 'Custom log formatter response'
+        end
+      end
+    end
+
+    let(:logger_options) { { formatter: formatter_class } }
+
+    it 'logs with custom formatter' do
+      conn.get '/hello'
+
+      expect(string_io.string).to match('Custom log formatter request')
+      expect(string_io.string).to match('Custom log formatter response')
+    end
+  end
+
   it 'logs method and url' do
     conn.get '/hello', nil, accept: 'text/html'
     expect(string_io.string).to match('GET http:/hello')

spec/spec_helper.rb

@@ -25,10 +25,14 @@ SimpleCov.formatters = [SimpleCov::Formatter::HTMLFormatter, Coveralls::SimpleCo
 
 SimpleCov.start do
   add_filter '/spec/'
-  minimum_coverage 90
-  minimum_coverage_by_file 70
+  minimum_coverage 84
+  minimum_coverage_by_file 26
 end
 
+# Ensure all /lib files are loaded
+# so they will be included in the test coverage report.
+Dir['./lib/**/*.rb'].sort.each { |file| require file }
+
 require 'faraday'
 require 'pry'
 

spec/support/helper_methods.rb

@@ -11,11 +11,15 @@ module Faraday
         @features = features
       end
 
-      def on_feature(name, &block)
+      def on_feature(name)
+        yield if block_given? && feature?(name)
+      end
+
+      def feature?(name)
         if @features.nil?
-          superclass.on_feature(name, &block) if superclass.respond_to?(:on_feature)
-        elsif block_given? && @features.include?(name)
-          yield
+          superclass.feature?(name) if superclass.respond_to?(:feature?)
+        elsif @features.include?(name)
+          true
         end
       end
 
@@ -54,11 +58,7 @@ module Faraday
         yield
       ensure
         old_env.each do |key, value|
-          if value == false
-            ENV.delete key
-          else
-            ENV[key] = value
-          end
+          value == false ? ENV.delete(key) : ENV[key] = value
         end
       end
     end

spec/support/shared_examples/request_method.rb

@@ -4,10 +4,12 @@ shared_examples 'a request method' do |http_method|
   let(:query_or_body) { method_with_body?(http_method) ? :body : :query }
   let(:response) { conn.public_send(http_method, '/') }
 
-  it 'retrieves the response body' do
-    res_body = 'test'
-    request_stub.to_return(body: res_body)
-    expect(conn.public_send(http_method, '/').body).to eq(res_body)
+  unless http_method == :head && feature?(:skip_response_body_on_head)
+    it 'retrieves the response body' do
+      res_body = 'test'
+      request_stub.to_return(body: res_body)
+      expect(conn.public_send(http_method, '/').body).to eq(res_body)
+    end
   end
 
   it 'handles headers with multiple values' do
@@ -160,28 +162,44 @@ shared_examples 'a request method' do |http_method|
   end
 
   on_feature :parallel do
-    it 'handles parallel requests' do
-      resp1 = nil
-      resp2 = nil
-      payload1 = { a: '1' }
-      payload2 = { b: '2' }
-      request_stub.with(Hash[query_or_body, payload1])
-                  .to_return(body: payload1.to_json)
-      stub_request(http_method, remote).with(Hash[query_or_body, payload2])
-                                       .to_return(body: payload2.to_json)
+    context 'with parallel setup' do
+      before do
+        @resp1 = nil
+        @resp2 = nil
+        @payload1 = { a: '1' }
+        @payload2 = { b: '2' }
 
-      conn.in_parallel do
-        resp1 = conn.public_send(http_method, '/', payload1)
-        resp2 = conn.public_send(http_method, '/', payload2)
+        request_stub
+          .with(Hash[query_or_body, @payload1])
+          .to_return(body: @payload1.to_json)
 
-        expect(conn.in_parallel?).to be_truthy
-        expect(resp1.body).to be_nil
-        expect(resp2.body).to be_nil
+        stub_request(http_method, remote)
+          .with(Hash[query_or_body, @payload2])
+          .to_return(body: @payload2.to_json)
+
+        conn.in_parallel do
+          @resp1 = conn.public_send(http_method, '/', @payload1)
+          @resp2 = conn.public_send(http_method, '/', @payload2)
+
+          expect(conn.in_parallel?).to be_truthy
+          expect(@resp1.body).to be_nil
+          expect(@resp2.body).to be_nil
+        end
+
+        expect(conn.in_parallel?).to be_falsey
       end
 
-      expect(conn.in_parallel?).to be_falsey
-      expect(resp1&.body).to eq(payload1.to_json)
-      expect(resp2&.body).to eq(payload2.to_json)
+      it 'handles parallel requests status' do
+        expect(@resp1&.status).to eq(200)
+        expect(@resp2&.status).to eq(200)
+      end
+
+      unless http_method == :head && feature?(:skip_response_body_on_head)
+        it 'handles parallel requests body' do
+          expect(@resp1&.body).to eq(@payload1.to_json)
+          expect(@resp2&.body).to eq(@payload2.to_json)
+        end
+      end
     end
   end