Fix absence of original_exception and/or backtrace even if passed in …

…error! (#2471)

* Fixes #2470 Expose original_exception and/or backtrace if present

* Update CHANGELOG.md

CHANGELOG.md

@@ -6,7 +6,7 @@
 
 #### Fixes
 
-* Your contribution here.
+* [#2471](https://github.com/ruby-grape/grape/pull/2471): Fix absence of original_exception and/or backtrace even if passed in error! - [@numbata](https://github.com/numbata).
 
 ### 2.1.3 (2024-07-13)
 

lib/grape/dsl/inside_route.rb

@@ -170,7 +170,12 @@ def configuration
       def error!(message, status = nil, additional_headers = nil, backtrace = nil, original_exception = nil)
         status = self.status(status || namespace_inheritable(:default_error_status))
         headers = additional_headers.present? ? header.merge(additional_headers) : header
-        throw :error, message: message, status: status, headers: headers, backtrace: backtrace, original_exception: original_exception
+        throw :error,
+              message: message,
+              status: status,
+              headers: headers,
+              backtrace: backtrace,
+              original_exception: original_exception
       end
 
       # Creates a Rack response based on the provided message, status, and headers.

lib/grape/middleware/error.rb

@@ -120,9 +120,9 @@ def run_rescue_handler(handler, error, endpoint)
           handler.arity.zero? ? endpoint.instance_exec(&handler) : endpoint.instance_exec(error, &handler)
         end
 
-        response = error!(response[:message], response[:status], response[:headers]) if error?(response)
-
-        if response.is_a?(Rack::Response)
+        if error?(response)
+          error_response(response)
+        elsif response.is_a?(Rack::Response)
           response
         else
           run_rescue_handler(:default_rescue_handler, Grape::Exceptions::InvalidResponse.new, endpoint)
@@ -137,7 +137,9 @@ def error!(message, status = options[:default_status], headers = {}, backtrace =
       end
 
       def error?(response)
-        response.is_a?(Hash) && response[:message] && response[:status] && response[:headers]
+        return false unless response.is_a?(Hash)
+
+        response.key?(:message) && response.key?(:status) && response.key?(:headers)
       end
     end
   end

spec/grape/api_spec.rb

@@ -2599,6 +2599,150 @@ def self.call(message, _backtrace, _options, _env, _original_exception)
       it_behaves_like 'a json format api', :failure
       it_behaves_like 'a json format api', { error: :failure }
     end
+
+    context 'when rescue_from enables backtrace without original exception' do
+      let(:app) do
+        response_type = response_format
+
+        Class.new(Grape::API) do
+          format response_type
+
+          rescue_from :all, backtrace: true, original_exception: false do |e|
+            error!('raining dogs and cats!', 418, {}, e.backtrace, e)
+          end
+
+          get '/exception' do
+            raise 'rain!'
+          end
+        end
+      end
+
+      before do
+        get '/exception'
+      end
+
+      context 'with json response type format' do
+        subject { JSON.parse(last_response.body) }
+
+        let(:response_format) { :json }
+
+        it { is_expected.to include('error' => a_kind_of(String), 'backtrace' => a_kind_of(Array)) }
+        it { is_expected.not_to include('original_exception') }
+      end
+
+      context 'with txt response type format' do
+        subject { last_response.body }
+
+        let(:response_format) { :txt }
+
+        it { is_expected.to include('backtrace') }
+        it { is_expected.not_to include('original_exception') }
+      end
+
+      context 'with xml response type format' do
+        subject { Grape::Xml.parse(last_response.body)['error'] }
+
+        let(:response_format) { :xml }
+
+        it { is_expected.to have_key('backtrace') }
+        it { is_expected.not_to have_key('original-exception') }
+      end
+    end
+
+    context 'when rescue_from enables original exception without backtrace' do
+      let(:app) do
+        response_type = response_format
+
+        Class.new(Grape::API) do
+          format response_type
+
+          rescue_from :all, backtrace: false, original_exception: true do |e|
+            error!('raining dogs and cats!', 418, {}, e.backtrace, e)
+          end
+
+          get '/exception' do
+            raise 'rain!'
+          end
+        end
+      end
+
+      before do
+        get '/exception'
+      end
+
+      context 'with json response type format' do
+        subject { JSON.parse(last_response.body) }
+
+        let(:response_format) { :json }
+
+        it { is_expected.to include('error' => a_kind_of(String), 'original_exception' => a_kind_of(String)) }
+        it { is_expected.not_to include('backtrace') }
+      end
+
+      context 'with txt response type format' do
+        subject { last_response.body }
+
+        let(:response_format) { :txt }
+
+        it { is_expected.to include('original exception') }
+        it { is_expected.not_to include('backtrace') }
+      end
+
+      context 'with xml response type format' do
+        subject { Grape::Xml.parse(last_response.body)['error'] }
+
+        let(:response_format) { :xml }
+
+        it { is_expected.to have_key('original-exception') }
+        it { is_expected.not_to have_key('backtrace') }
+      end
+    end
+
+    context 'when rescue_from include backtrace and original exception' do
+      let(:app) do
+        response_type = response_format
+
+        Class.new(Grape::API) do
+          format response_type
+
+          rescue_from :all, backtrace: true, original_exception: true do |e|
+            error!('raining dogs and cats!', 418, {}, e.backtrace, e)
+          end
+
+          get '/exception' do
+            raise 'rain!'
+          end
+        end
+      end
+
+      before do
+        get '/exception'
+      end
+
+      context 'with json response type format' do
+        subject { JSON.parse(last_response.body) }
+
+        let(:response_format) { :json }
+
+        it { is_expected.to include('error' => a_kind_of(String), 'backtrace' => a_kind_of(Array), 'original_exception' => a_kind_of(String)) }
+      end
+
+      context 'with txt response type format' do
+        subject { last_response.body }
+
+        let(:response_format) { :txt }
+
+        it { is_expected.to include('backtrace', 'original exception') }
+      end
+
+      context 'with xml response type format' do
+        subject { Grape::Xml.parse(last_response.body)['error'] }
+
+        let(:response_format) { :xml }
+
+        it { is_expected.to have_key('backtrace') & have_key('original-exception') }
+      end
+    end
   end
 
   describe '.content_type' do