Merge pull request #1095 from lostisland/upstream-error-init

improve error initializer consistency

lib/faraday/error.rb

@@ -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
@@ -35,6 +27,38 @@ module Faraday
       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.
@@ -82,9 +106,9 @@ module Faraday
 
   # 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)
+      super('http status could not be derived from the server response')
     end
   end
 

lib/faraday/response/raise_error.rb

@@ -33,7 +33,7 @@ module Faraday
         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
 

spec/faraday/response/raise_error_spec.rb

@@ -14,7 +14,7 @@ RSpec.describe Faraday::Response::RaiseError do
         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 @@ RSpec.describe Faraday::Response::RaiseError do
     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
 
@@ -35,6 +36,7 @@ RSpec.describe Faraday::Response::RaiseError 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
 
@@ -42,6 +44,7 @@ RSpec.describe Faraday::Response::RaiseError 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
 
@@ -49,6 +52,7 @@ RSpec.describe Faraday::Response::RaiseError 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
 
@@ -56,6 +60,7 @@ RSpec.describe Faraday::Response::RaiseError 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
 
@@ -63,6 +68,7 @@ RSpec.describe Faraday::Response::RaiseError 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
 
@@ -70,12 +76,15 @@ RSpec.describe Faraday::Response::RaiseError 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
 
@@ -83,13 +92,15 @@ RSpec.describe Faraday::Response::RaiseError 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