diff --git a/CHANGELOG.md b/CHANGELOG.md index 124148452..66776fa8f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ * [#1749](https://github.com/ruby-grape/grape/pull/1749): Allow rescue from non-`StandardError` exceptions - [@dm1try](https://github.com/dm1try). * [#1750](https://github.com/ruby-grape/grape/pull/1750): Fix a circular dependency warning due to router being loaded by API - [@salasrod](https://github.com/salasrod). * [#1752](https://github.com/ruby-grape/grape/pull/1752): Fix `include_missing` behavior for aliased parameters - [@jonasoberschweiber](https://github.com/jonasoberschweiber). +* [#1754](https://github.com/ruby-grape/grape/pull/1754): Allow rescue from non-`StandardError` exceptions to use default error handling - [@jelkster](https://github.com/jelkster). * Your contribution here. ### 1.0.2 (1/10/2018) diff --git a/README.md b/README.md index 229e5bbc6..fee874c23 100644 --- a/README.md +++ b/README.md @@ -2289,7 +2289,7 @@ Here `'inner'` will be result of handling occured `ArgumentError`. Any exception that is not subclass of `StandardError` should be rescued explicitly. Usually it is not a case for an application logic as such errors point to problems in Ruby runtime. -This is following [standard recomendations for exceptions handling](https://ruby-doc.org/core/Exception.html). +This is following [standard recommendations for exceptions handling](https://ruby-doc.org/core/Exception.html). ### Rails 3.x diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index bdb0b073c..fe26e84af 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -36,7 +36,7 @@ def call!(env) error_response(catch(:error) do return @app.call(@env) end) - rescue StandardError => e + rescue Exception => e # rubocop:disable Lint/RescueException is_rescuable = rescuable?(e.class) if e.is_a?(Grape::Exceptions::Base) && (!is_rescuable || rescuable_by_grape?(e.class)) handler = ->(arg) { error_response(arg) } @@ -46,14 +46,6 @@ def call!(env) end handler.nil? ? handle_error(e) : exec_handler(e, &handler) - rescue Exception => e # rubocop:disable Lint/RescueException - handler = options[:rescue_handlers].find do |error_class, error_handler| - break error_handler if e.class <= error_class - end - - raise unless handler - - exec_handler(e, &handler) end end @@ -72,7 +64,12 @@ def find_handler(klass) def rescuable?(klass) return false if klass == Grape::Exceptions::InvalidVersionHeader - rescue_all? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass) + + if klass <= StandardError + rescue_all? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass) + else + rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass) + end end def rescuable_by_grape?(klass) diff --git a/spec/grape/middleware/exception_spec.rb b/spec/grape/middleware/exception_spec.rb index 54df280fe..2f65cc541 100644 --- a/spec/grape/middleware/exception_spec.rb +++ b/spec/grape/middleware/exception_spec.rb @@ -110,18 +110,35 @@ def app end context 'Non-StandardError exception with a provided rescue handler' do - subject do - Rack::Builder.app do - use Spec::Support::EndpointFaker - use Grape::Middleware::Error, rescue_handlers: { NotImplementedError => -> { [200, {}, 'rescued'] } } - run ExceptionSpec::OtherExceptionApp + context 'default error response' do + subject do + Rack::Builder.app do + use Spec::Support::EndpointFaker + use Grape::Middleware::Error, rescue_handlers: { NotImplementedError => nil } + run ExceptionSpec::OtherExceptionApp + end + end + it 'rescues the exception using the default handler' do + get '/' + expect(last_response.body).to eq('snow!') end end - it 'rescues the exception using the provided handler' do - get '/' - expect(last_response.body).to eq('rescued') + + context 'custom error response' do + subject do + Rack::Builder.app do + use Spec::Support::EndpointFaker + use Grape::Middleware::Error, rescue_handlers: { NotImplementedError => -> { [200, {}, 'rescued'] } } + run ExceptionSpec::OtherExceptionApp + end + end + it 'rescues the exception using the provided handler' do + get '/' + expect(last_response.body).to eq('rescued') + end end end + context do subject do Rack::Builder.app do