From ab33c8ace10f6c4c3a127c30a043238e3b54364c Mon Sep 17 00:00:00 2001 From: iMacTia Date: Tue, 5 Mar 2019 18:26:46 +0000 Subject: [PATCH 1/4] Adds em-http adapter to RSpec tests --- lib/faraday/adapter/em_http.rb | 12 ++-- spec/faraday/adapter/em_http_spec.rb | 16 +++++ spec/spec_helper.rb | 1 + spec/support/helper_methods.rb | 18 +++--- .../support/shared_examples/request_method.rb | 62 ++++++++++++------- 5 files changed, 72 insertions(+), 37 deletions(-) create mode 100644 spec/faraday/adapter/em_http_spec.rb diff --git a/lib/faraday/adapter/em_http.rb b/lib/faraday/adapter/em_http.rb index 264b9287..53e86ef3 100644 --- a/lib/faraday/adapter/em_http.rb +++ b/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] diff --git a/spec/faraday/adapter/em_http_spec.rb b/spec/faraday/adapter/em_http_spec.rb new file mode 100644 index 00000000..d11ac92d --- /dev/null +++ b/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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9bdf7b2f..1ec4df77 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -25,6 +25,7 @@ SimpleCov.formatters = [SimpleCov::Formatter::HTMLFormatter, Coveralls::SimpleCo SimpleCov.start do add_filter '/spec/' + add_filter '/lib/faraday/adapter/em_http_ssl_patch' minimum_coverage 90 minimum_coverage_by_file 70 end diff --git a/spec/support/helper_methods.rb b/spec/support/helper_methods.rb index 6560d686..af2f1d67 100644 --- a/spec/support/helper_methods.rb +++ b/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 diff --git a/spec/support/shared_examples/request_method.rb b/spec/support/shared_examples/request_method.rb index 3bfe96cf..a8ab0d4e 100644 --- a/spec/support/shared_examples/request_method.rb +++ b/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 From b9ea1d92ce11710e0f70b56037f434afd2a094e4 Mon Sep 17 00:00:00 2001 From: Geoff Hubbard Date: Wed, 6 Mar 2019 09:46:51 +0100 Subject: [PATCH 2/4] Accurate test code coverage percentage (#934) --- spec/spec_helper.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 9bdf7b2f..0633ad8a 100644 --- a/spec/spec_helper.rb +++ b/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'].each { |file| require file } + require 'faraday' require 'pry' From 04dff67cfa48309b600581cd0360c30a37f7b6b1 Mon Sep 17 00:00:00 2001 From: iMacTia Date: Wed, 6 Mar 2019 09:20:47 +0000 Subject: [PATCH 3/4] Fixes an issue with macOS require order. --- spec/spec_helper.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0633ad8a..d611e709 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -31,7 +31,7 @@ end # Ensure all /lib files are loaded # so they will be included in the test coverage report. -Dir['./lib/**/*.rb'].each { |file| require file } +Dir['./lib/**/*.rb'].sort.each { |file| require file } require 'faraday' require 'pry' From 1d3d6758219fcbdd17ceff4a5a8a7882f15d5e88 Mon Sep 17 00:00:00 2001 From: Sergey Prikhodko Date: Wed, 6 Mar 2019 19:13:14 +0300 Subject: [PATCH 4/4] Add the log formatter that is easy to override and safe to inherit (#889) --- README.md | 17 +++++++ lib/faraday/logging/formatter.rb | 76 ++++++++++++++++++++++++++++ lib/faraday/response/logger.rb | 67 +++--------------------- spec/faraday/response/logger_spec.rb | 55 ++++++++++++++++++++ 4 files changed, 155 insertions(+), 60 deletions(-) create mode 100644 lib/faraday/logging/formatter.rb diff --git a/README.md b/README.md index 4cf684a6..ef8d9595 100644 --- a/README.md +++ b/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: diff --git a/lib/faraday/logging/formatter.rb b/lib/faraday/logging/formatter.rb new file mode 100644 index 00000000..b355ced7 --- /dev/null +++ b/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 diff --git a/lib/faraday/response/logger.rb b/lib/faraday/response/logger.rb index b489715e..0ad09bee 100644 --- a/lib/faraday/response/logger.rb +++ b/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 diff --git a/spec/faraday/response/logger_spec.rb b/spec/faraday/response/logger_spec.rb index 9256f8ef..ac4f5fd4 100644 --- a/spec/faraday/response/logger_spec.rb +++ b/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')