Skip to content

Commit

Permalink
adding new rescue_from option to rescue all and still use built in Gr…
Browse files Browse the repository at this point in the history
…ape::Exception handling (#1398)
  • Loading branch information
Mason McLead authored and dblock committed Jun 30, 2016
1 parent c059f98 commit 42107da
Show file tree
Hide file tree
Showing 7 changed files with 78 additions and 3 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand Down
11 changes: 11 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 5 additions & 1 deletion lib/grape/dsl/request_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
1 change: 1 addition & 0 deletions lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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) || {},
Expand Down
10 changes: 8 additions & 2 deletions lib/grape/middleware/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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)
Expand Down
14 changes: 14 additions & 0 deletions spec/grape/dsl/request_response_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
37 changes: 37 additions & 0 deletions spec/grape/exceptions/body_parse_errors_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit 42107da

Please sign in to comment.