diff --git a/CHANGELOG.md b/CHANGELOG.md index e526b2f5b2..2504a5e5b1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ * Your contribution here. +* [#1398](https://github.com/ruby-grape/grape/pull/1398): Added rescue_from :grape_exceptions option to allow Grape to use the built in Grape::Exception handing and use rescue :all behavior for everything else - [@mmclead](https://github.com/mmclead). + #### Features * [#1393](https://github.com/ruby-grape/grape/pull/1393): Middleware can be inserted before or after default Grape middleware - [@ridiculous](https://github.com/ridiculous). diff --git a/README.md b/README.md index a0fba4b90d..fd0bd1f311 100644 --- a/README.md +++ b/README.md @@ -1853,6 +1853,17 @@ class Twitter::API < Grape::API end ``` +Grape can also rescue from all exceptions and still use the built-in exception handing. +This will give the same behavior as `rescue_from :all` with the addition that Grape will use the exception handling defined by all Exception classes that inherit Grape::Exceptions::Base. + +The intent of this setting is to provide a simple way to cover the most common exceptions and return any unexpected exceptions in the API format. + +```ruby +class Twitter::API < Grape::API + rescue_from :grape_exceptions +end +``` + You can also rescue specific exceptions. ```ruby diff --git a/lib/grape/dsl/request_response.rb b/lib/grape/dsl/request_response.rb index e50ec7c3c4..bbb5bbe567 100644 --- a/lib/grape/dsl/request_response.rb +++ b/lib/grape/dsl/request_response.rb @@ -110,9 +110,13 @@ def rescue_from(*args, &block) end handler ||= extract_with(options) - if args.include?(:all) + case + when args.include?(:all) namespace_inheritable(:rescue_all, true) namespace_inheritable :all_rescue_handler, handler + when args.include?(:grape_exceptions) + namespace_inheritable(:rescue_all, true) + namespace_inheritable(:rescue_grape_exceptions, true) else handler_type = case options[:rescue_subclasses] diff --git a/lib/grape/endpoint.rb b/lib/grape/endpoint.rb index 75ed75c48e..5d89197f19 100644 --- a/lib/grape/endpoint.rb +++ b/lib/grape/endpoint.rb @@ -274,6 +274,7 @@ def build_stack(helpers) content_types: namespace_stackable_with_hash(:content_types), default_status: namespace_inheritable(:default_error_status), rescue_all: namespace_inheritable(:rescue_all), + rescue_grape_exceptions: namespace_inheritable(:rescue_grape_exceptions), default_error_formatter: namespace_inheritable(:default_error_formatter), error_formatters: namespace_stackable_with_hash(:error_formatters), rescue_options: namespace_stackable_with_hash(:rescue_options) || {}, diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 5e0290627b..e4307fb01f 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -12,8 +12,9 @@ def default_options formatters: {}, error_formatters: {}, rescue_all: false, # true to rescue all exceptions + rescue_grape_exceptions: false, rescue_subclasses: true, # rescue subclasses of exceptions listed - rescue_options: { backtrace: false }, # true to display backtrace + rescue_options: { backtrace: false }, # true to display backtrace, true to let Grape handle Grape::Exceptions rescue_handlers: {}, # rescue handler blocks base_only_rescue_handlers: {}, # rescue handler blocks rescuing only the base class all_rescue_handler: nil # rescue handler block to rescue from all exceptions @@ -34,7 +35,7 @@ def call!(env) end) rescue StandardError => e is_rescuable = rescuable?(e.class) - if e.is_a?(Grape::Exceptions::Base) && !is_rescuable + if e.is_a?(Grape::Exceptions::Base) && (!is_rescuable || rescuable_by_grape?(e.class)) handler = ->(arg) { error_response(arg) } else raise unless is_rescuable @@ -63,6 +64,11 @@ def rescuable?(klass) rescue_all? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass) end + def rescuable_by_grape?(klass) + return false if klass == Grape::Exceptions::InvalidVersionHeader + options[:rescue_grape_exceptions] + end + def exec_handler(e, &handler) if handler.lambda? && handler.arity == 0 instance_exec(&handler) diff --git a/spec/grape/dsl/request_response_spec.rb b/spec/grape/dsl/request_response_spec.rb index 7e89bc0efa..d8dee46b75 100644 --- a/spec/grape/dsl/request_response_spec.rb +++ b/spec/grape/dsl/request_response_spec.rb @@ -143,6 +143,20 @@ def self.imbue(key, value) end end + describe ':grape_exceptions' do + it 'sets rescue all to true' do + expect(subject).to receive(:namespace_inheritable).with(:rescue_all, true) + expect(subject).to receive(:namespace_inheritable).with(:rescue_grape_exceptions, true) + subject.rescue_from :grape_exceptions + end + + it 'sets rescue_grape_exceptions to true' do + expect(subject).to receive(:namespace_inheritable).with(:rescue_all, true) + expect(subject).to receive(:namespace_inheritable).with(:rescue_grape_exceptions, true) + subject.rescue_from :grape_exceptions + end + end + describe 'list of exceptions is passed' do it 'sets hash of exceptions as rescue handlers' do expect(subject).to receive(:namespace_reverse_stackable).with(:rescue_handlers, StandardError => nil) diff --git a/spec/grape/exceptions/body_parse_errors_spec.rb b/spec/grape/exceptions/body_parse_errors_spec.rb index 2bd91797e4..422e6719e6 100644 --- a/spec/grape/exceptions/body_parse_errors_spec.rb +++ b/spec/grape/exceptions/body_parse_errors_spec.rb @@ -52,6 +52,43 @@ def app end end + context 'api with rescue_from :grape_exceptions handler' do + subject { Class.new(Grape::API) } + before do + subject.rescue_from :all do |_e| + rack_response 'message was processed', 400 + end + subject.rescue_from :grape_exceptions + + subject.params do + requires :beer + end + subject.post '/beer' do + 'beer received' + end + end + + def app + subject + end + + context 'with content_type json' do + it 'returns body parsing error message' do + post '/beer', 'test', 'CONTENT_TYPE' => 'application/json' + expect(last_response.status).to eq 400 + expect(last_response.body).to include 'message body does not match declared format' + end + end + + context 'with content_type xml' do + it 'returns body parsing error message' do + post '/beer', 'test', 'CONTENT_TYPE' => 'application/xml' + expect(last_response.status).to eq 400 + expect(last_response.body).to include 'message body does not match declared format' + end + end + end + context 'api without a rescue handler' do subject { Class.new(Grape::API) } before do