From 9b5b02816b71b20fec859c95e520361aa25e2e39 Mon Sep 17 00:00:00 2001 From: Hiro Asari Date: Tue, 10 Dec 2019 11:15:19 -0500 Subject: [PATCH 1/8] Fix 1.9 syntax error 0.1x is supposed to be 1.9-compat --- lib/faraday/error.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/faraday/error.rb b/lib/faraday/error.rb index 39b0d582b..338ca6818 100644 --- a/lib/faraday/error.rb +++ b/lib/faraday/error.rb @@ -85,7 +85,7 @@ def initialize(exc = 'timeout', response = nil) # Raised by Faraday::Response::RaiseError in case of a nil status in response. class NilStatusError < ServerError - def initialize(_exc, response: nil) + def initialize(_exc, response = nil) message = 'http status could not be derived from the server response' super(message, response) end From c7d56a7da5325e532df654dc0e0c52f003f82e61 Mon Sep 17 00:00:00 2001 From: Kiselev Valentine Date: Tue, 10 Dec 2019 08:42:08 +0300 Subject: [PATCH 2/8] Fix CI for Ruby 2.3 Support for github action of ruby 2.3 was removed. Now according to https://github.com/actions/setup-ruby/blob/master/action.yml theminimal version is 2.4. --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0c9d111b5..f90b5007e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - ruby: [2.3.x, 2.4.x, 2.5.x, 2.6.x] + ruby: [2.4.x, 2.5.x, 2.6.x] steps: - uses: actions/checkout@v1 From c2a87e4a8ef2d68e55233430a43cee60a6927f76 Mon Sep 17 00:00:00 2001 From: technoweenie Date: Fri, 27 Dec 2019 11:55:08 -0700 Subject: [PATCH 3/8] improve how errors pull out exceptions and responses --- lib/faraday/error.rb | 62 +++++++++++++++++------ spec/faraday/response/raise_error_spec.rb | 13 ++++- 2 files changed, 59 insertions(+), 16 deletions(-) diff --git a/lib/faraday/error.rb b/lib/faraday/error.rb index af40c18ea..745a64ed6 100644 --- a/lib/faraday/error.rb +++ b/lib/faraday/error.rb @@ -9,18 +9,7 @@ class Error < StandardError attr_reader :response, :wrapped_exception def initialize(exc, response = nil) - @wrapped_exception = nil - @response = response - - if exc.respond_to?(:backtrace) - super(exc.message) - @wrapped_exception = exc - elsif exc.respond_to?(:each_key) - super("the server responded with status #{exc[:status]}") - @response = exc - else - super(exc.to_s) - end + super(exc_msg_and_response!(exc, response)) end def backtrace @@ -38,6 +27,38 @@ def inspect inner += " #{super}" if inner.empty? %(#<#{self.class}#{inner}>) end + + protected + + # Pulls out potential parent exception and response hash, storing them in + # instance variables. + # exc - Either an Exception, a string message, or a response hash. + # response - Hash + # :status - Optional integer HTTP response status + # :headers - String key/value hash of HTTP response header + # values. + # :body - Optional string HTTP response body. + # + # If a subclass has to call this, then it should pass a string message + # to `super`. See NilStatusError. + def exc_msg_and_response!(exc, response = nil) + if @response.nil? && @wrapped_exception.nil? + @wrapped_exception, msg, @response = exc_msg_and_response(exc, response) + return msg + end + + exc.to_s + end + + # Pulls out potential parent exception and response hash. + def exc_msg_and_response(exc, response = nil) + return [exc, exc.message, response] if exc.respond_to?(:backtrace) + + return [nil, "the server responded with status #{exc[:status]}", exc] \ + if exc.respond_to?(:each_key) + + [nil, exc.to_s, response || {}] + end end # Faraday client error class. Represents 4xx status responses. @@ -85,9 +106,20 @@ def initialize(exc = 'timeout', response = nil) # Raised by Faraday::Response::RaiseError in case of a nil status in response. class NilStatusError < ServerError - def initialize(_exc, response = nil) - message = 'http status could not be derived from the server response' - super(message, response) + def initialize(exc, response = nil) + exc_msg_and_response!(exc, response) + @response = unwrap_resp(@response) + super('http status could not be derived from the server response') + end + + private + + def unwrap_resp(resp) + if inner = resp.keys.size == 1 && resp[:response] + return unwrap_resp(inner) + end + + resp end end diff --git a/spec/faraday/response/raise_error_spec.rb b/spec/faraday/response/raise_error_spec.rb index a2348d1b8..033cd6d5c 100644 --- a/spec/faraday/response/raise_error_spec.rb +++ b/spec/faraday/response/raise_error_spec.rb @@ -14,7 +14,7 @@ stub.get('conflict') { [409, { 'X-Reason' => 'because' }, 'keep looking'] } stub.get('unprocessable-entity') { [422, { 'X-Reason' => 'because' }, 'keep looking'] } stub.get('4xx') { [499, { 'X-Reason' => 'because' }, 'keep looking'] } - stub.get('nil-status') { [nil, { 'X-Reason' => 'bailout' }, 'fail'] } + stub.get('nil-status') { [nil, { 'X-Reason' => 'nil' }, 'fail'] } stub.get('server-error') { [500, { 'X-Error' => 'bailout' }, 'fail'] } end end @@ -28,6 +28,7 @@ expect { conn.get('bad-request') }.to raise_error(Faraday::ClientError) do |ex| expect(ex.message).to eq('the server responded with status 400') expect(ex.response[:headers]['X-Reason']).to eq('because') + expect(ex.response[:status]).to eq(400) end end @@ -35,6 +36,7 @@ expect { conn.get('unauthorized') }.to raise_error(Faraday::ClientError) do |ex| expect(ex.message).to eq('the server responded with status 401') expect(ex.response[:headers]['X-Reason']).to eq('because') + expect(ex.response[:status]).to eq(401) end end @@ -42,6 +44,7 @@ expect { conn.get('forbidden') }.to raise_error(Faraday::ClientError) do |ex| expect(ex.message).to eq('the server responded with status 403') expect(ex.response[:headers]['X-Reason']).to eq('because') + expect(ex.response[:status]).to eq(403) end end @@ -49,6 +52,7 @@ expect { conn.get('not-found') }.to raise_error(Faraday::ResourceNotFound) do |ex| expect(ex.message).to eq('the server responded with status 404') expect(ex.response[:headers]['X-Reason']).to eq('because') + expect(ex.response[:status]).to eq(404) end end @@ -56,6 +60,7 @@ expect { conn.get('proxy-error') }.to raise_error(Faraday::ConnectionFailed) do |ex| expect(ex.message).to eq('407 "Proxy Authentication Required "') expect(ex.response[:headers]['X-Reason']).to eq('because') + expect(ex.response[:status]).to eq(407) end end @@ -63,6 +68,7 @@ expect { conn.get('conflict') }.to raise_error(Faraday::ClientError) do |ex| expect(ex.message).to eq('the server responded with status 409') expect(ex.response[:headers]['X-Reason']).to eq('because') + expect(ex.response[:status]).to eq(409) end end @@ -70,12 +76,15 @@ expect { conn.get('unprocessable-entity') }.to raise_error(Faraday::ClientError) do |ex| expect(ex.message).to eq('the server responded with status 422') expect(ex.response[:headers]['X-Reason']).to eq('because') + expect(ex.response[:status]).to eq(422) end end it 'raises Faraday::NilStatusError for nil status in response' do expect { conn.get('nil-status') }.to raise_error(Faraday::NilStatusError) do |ex| expect(ex.message).to eq('http status could not be derived from the server response') + expect(ex.response[:headers]['X-Reason']).to eq('nil') + expect(ex.response[:status]).to be_nil end end @@ -83,6 +92,7 @@ expect { conn.get('4xx') }.to raise_error(Faraday::ClientError) do |ex| expect(ex.message).to eq('the server responded with status 499') expect(ex.response[:headers]['X-Reason']).to eq('because') + expect(ex.response[:status]).to eq(499) end end @@ -90,6 +100,7 @@ expect { conn.get('server-error') }.to raise_error(Faraday::ClientError) do |ex| expect(ex.message).to eq('the server responded with status 500') expect(ex.response[:headers]['X-Error']).to eq('bailout') + expect(ex.response[:status]).to eq(500) end end end From 9be191a7096b9af9a789556903b3ea07365b7fdb Mon Sep 17 00:00:00 2001 From: technoweenie Date: Fri, 27 Dec 2019 11:57:29 -0700 Subject: [PATCH 4/8] deprecate :response handling --- lib/faraday/error.rb | 7 ++++++- lib/faraday/response/raise_error.rb | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/lib/faraday/error.rb b/lib/faraday/error.rb index 745a64ed6..46b1cf33b 100644 --- a/lib/faraday/error.rb +++ b/lib/faraday/error.rb @@ -108,12 +108,14 @@ def initialize(exc = 'timeout', response = nil) class NilStatusError < ServerError def initialize(exc, response = nil) exc_msg_and_response!(exc, response) - @response = unwrap_resp(@response) + @response = unwrap_resp!(@response) super('http status could not be derived from the server response') end private + extend Faraday::Deprecate + def unwrap_resp(resp) if inner = resp.keys.size == 1 && resp[:response] return unwrap_resp(inner) @@ -121,6 +123,9 @@ def unwrap_resp(resp) resp end + + alias_method :unwrap_resp!, :unwrap_resp + deprecate('unwrap_resp', nil, '1.0') end # A unified error for failed connections. diff --git a/lib/faraday/response/raise_error.rb b/lib/faraday/response/raise_error.rb index d7b3edb45..dc8ac6de3 100644 --- a/lib/faraday/response/raise_error.rb +++ b/lib/faraday/response/raise_error.rb @@ -14,7 +14,7 @@ def on_complete(env) when ClientErrorStatuses raise Faraday::ClientError, response_values(env) when nil - raise Faraday::NilStatusError, response: response_values(env) + raise Faraday::NilStatusError, response_values(env) end end From 3922569d5276dd58ad1633e96afbefcf0f839b31 Mon Sep 17 00:00:00 2001 From: technoweenie Date: Fri, 27 Dec 2019 12:34:24 -0700 Subject: [PATCH 5/8] don't force response to be {} --- lib/faraday/error.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/faraday/error.rb b/lib/faraday/error.rb index 46b1cf33b..5baf41df0 100644 --- a/lib/faraday/error.rb +++ b/lib/faraday/error.rb @@ -57,7 +57,7 @@ def exc_msg_and_response(exc, response = nil) return [nil, "the server responded with status #{exc[:status]}", exc] \ if exc.respond_to?(:each_key) - [nil, exc.to_s, response || {}] + [nil, exc.to_s, response] end end @@ -117,7 +117,7 @@ def initialize(exc, response = nil) extend Faraday::Deprecate def unwrap_resp(resp) - if inner = resp.keys.size == 1 && resp[:response] + if inner = (resp.keys.size == 1 && resp[:response]) return unwrap_resp(inner) end @@ -146,8 +146,8 @@ class ParsingError < ClientError class RetriableResponse < ClientError end - %i[ClientError ConnectionFailed ResourceNotFound - ParsingError TimeoutError SSLError RetriableResponse].each do |const| + [:ClientError, :ConnectionFailed, :ResourceNotFound, + :ParsingError, :TimeoutError, :SSLError, :RetriableResponse].each do |const| Error.const_set( const, DeprecatedClass.proxy_class(Faraday.const_get(const)) From e85ac8ee935cb181ab13e4b62aa595779e95698f Mon Sep 17 00:00:00 2001 From: technoweenie Date: Fri, 27 Dec 2019 12:46:39 -0700 Subject: [PATCH 6/8] init these ivars --- lib/faraday/error.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/faraday/error.rb b/lib/faraday/error.rb index 5baf41df0..57254a3ff 100644 --- a/lib/faraday/error.rb +++ b/lib/faraday/error.rb @@ -9,6 +9,7 @@ class Error < StandardError attr_reader :response, :wrapped_exception def initialize(exc, response = nil) + @wrapped_exception = @response = nil super(exc_msg_and_response!(exc, response)) end From 49c614bd6fee53be545158a73c4e52c9c8b37d28 Mon Sep 17 00:00:00 2001 From: technoweenie Date: Fri, 27 Dec 2019 13:03:51 -0700 Subject: [PATCH 7/8] dont re-init them --- lib/faraday/error.rb | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/faraday/error.rb b/lib/faraday/error.rb index 57254a3ff..5fae7c5fa 100644 --- a/lib/faraday/error.rb +++ b/lib/faraday/error.rb @@ -9,7 +9,8 @@ class Error < StandardError attr_reader :response, :wrapped_exception def initialize(exc, response = nil) - @wrapped_exception = @response = nil + @wrapped_exception = nil unless @wrapped_exception + @response = nil unless @response super(exc_msg_and_response!(exc, response)) end From 7f29467b29aeb081d4b5b34ab473d6634bcb1c4c Mon Sep 17 00:00:00 2001 From: technoweenie Date: Fri, 27 Dec 2019 13:13:36 -0700 Subject: [PATCH 8/8] how do i get this to init --- lib/faraday/error.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/faraday/error.rb b/lib/faraday/error.rb index 5fae7c5fa..e4927a169 100644 --- a/lib/faraday/error.rb +++ b/lib/faraday/error.rb @@ -9,8 +9,8 @@ class Error < StandardError attr_reader :response, :wrapped_exception def initialize(exc, response = nil) - @wrapped_exception = nil unless @wrapped_exception - @response = nil unless @response + @wrapped_exception = nil unless defined?(@wrapped_exception) + @response = nil unless defined?(@response) super(exc_msg_and_response!(exc, response)) end