diff --git a/CHANGELOG.md b/CHANGELOG.md index 34c72c7287..1d95cf9eec 100644 --- a/CHANGELOG.md +++ b/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) diff --git a/lib/grape/dsl/inside_route.rb b/lib/grape/dsl/inside_route.rb index 320b45a6d4..ba1715de24 100644 --- a/lib/grape/dsl/inside_route.rb +++ b/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. diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 2dc71c1daf..573db672cc 100644 --- a/lib/grape/middleware/error.rb +++ b/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 diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index ca26b812a6..3ac2fe7454 100644 --- a/spec/grape/api_spec.rb +++ b/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