Skip to content

Commit

Permalink
Fix 1.9 syntax error (with Error fixes) (#1094)
Browse files Browse the repository at this point in the history
* Fix 1.9 syntax error (with Error fixes)

0.1x is supposed to be 1.9-compat

* 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.

* improve how errors pull out exceptions and responses

* deprecate :response handling

* don't force response to be {}

* init these ivars

* dont re-init them

* how do i get this to init

Co-authored-by: Hiro Asari <[email protected]>
Co-authored-by: Valentine Kiselev <[email protected]>
Co-authored-by: Mattia <[email protected]>
  • Loading branch information
4 people authored Dec 30, 2019
1 parent 243dc92 commit cbde3d3
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 19 deletions.
73 changes: 56 additions & 17 deletions lib/faraday/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,9 @@ 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
@wrapped_exception = nil unless defined?(@wrapped_exception)
@response = nil unless defined?(@response)
super(exc_msg_and_response!(exc, response))
end

def backtrace
Expand All @@ -38,6 +29,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.
Expand Down Expand Up @@ -85,10 +108,26 @@ 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

extend Faraday::Deprecate

def unwrap_resp(resp)
if inner = (resp.keys.size == 1 && resp[:response])
return unwrap_resp(inner)
end

resp
end

alias_method :unwrap_resp!, :unwrap_resp
deprecate('unwrap_resp', nil, '1.0')
end

# A unified error for failed connections.
Expand All @@ -109,8 +148,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))
Expand Down
2 changes: 1 addition & 1 deletion lib/faraday/response/raise_error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
13 changes: 12 additions & 1 deletion spec/faraday/response/raise_error_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -28,68 +28,79 @@
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

it 'raises Faraday::ClientError for 401 responses' do
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

it 'raises Faraday::ClientError for 403 responses' do
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

it 'raises Faraday::ResourceNotFound for 404 responses' do
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

it 'raises Faraday::ConnectionFailed for 407 responses' do
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

it 'raises Faraday::ClientError for 409 responses' do
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

it 'raises Faraday::ClientError for 422 responses' do
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

it 'raises Faraday::ClientError for other 4xx responses' do
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

it 'raises Faraday::ClientError for 500 responses' do
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

0 comments on commit cbde3d3

Please sign in to comment.