Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve error initializer consistency #1095

Merged
merged 8 commits into from
Dec 31, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 39 additions & 15 deletions lib/faraday/error.rb
Original file line number Diff line number Diff line change
@@ -1,23 +1,15 @@
# frozen_string_literal: true

# Faraday namespace.
module Faraday
# Faraday error base class.
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 @@ -35,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.
Expand Down Expand Up @@ -82,9 +106,9 @@ 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)
iMacTia marked this conversation as resolved.
Show resolved Hide resolved
exc_msg_and_response!(exc, response)
super('http status could not be derived from the server response')
end
end

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 @@ -33,7 +33,7 @@ def on_complete(env)
when ServerErrorStatuses
raise Faraday::ServerError, response_values(env)
when nil
raise Faraday::NilStatusError, response: response_values(env)
raise Faraday::NilStatusError, response_values(env)
end
end

Expand Down
15 changes: 13 additions & 2 deletions 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::BadRequestError) 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::UnauthorizedError for 401 responses' do
expect { conn.get('unauthorized') }.to raise_error(Faraday::UnauthorizedError) 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::ForbiddenError for 403 responses' do
expect { conn.get('forbidden') }.to raise_error(Faraday::ForbiddenError) 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::ProxyAuthError for 407 responses' do
expect { conn.get('proxy-error') }.to raise_error(Faraday::ProxyAuthError) 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::ConflictError for 409 responses' do
expect { conn.get('conflict') }.to raise_error(Faraday::ConflictError) 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::UnprocessableEntityError for 422 responses' do
expect { conn.get('unprocessable-entity') }.to raise_error(Faraday::UnprocessableEntityError) 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
it 'raises Faraday::ServerError for 500 responses' do
expect { conn.get('server-error') }.to raise_error(Faraday::ServerError) 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