Skip to content

Commit

Permalink
add rescue_from :exception to truly rescue all exceptions
Browse files Browse the repository at this point in the history
  • Loading branch information
Jelkster committed Mar 27, 2018
1 parent 5613186 commit 42866b7
Show file tree
Hide file tree
Showing 8 changed files with 135 additions and 22 deletions.
4 changes: 2 additions & 2 deletions .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ Metrics/BlockLength:
# Offense count: 9
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 288
Max: 290

# Offense count: 32
Metrics/CyclomaticComplexity:
Expand All @@ -54,7 +54,7 @@ Metrics/LineLength:
# Offense count: 57
# Configuration parameters: CountComments.
Metrics/MethodLength:
Max: 33
Max: 34

# Offense count: 11
# Configuration parameters: CountComments.
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

#### Features

* [#1754](https://github.com/ruby-grape/grape/pull/1754): Added `rescue_from :exception` to truly rescue from all exceptions - [@jelkster](https://github.com/jelkster).
* Your contribution here.

#### Fixes
Expand Down
10 changes: 9 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -2083,6 +2083,14 @@ end
This mimics [default `rescue` behaviour](https://ruby-doc.org/core/StandardError.html) when an exception type is not provided.
Any other exception should be rescued explicitly, see [below](#exceptions-that-should-be-rescued-explicitly).

In the event you want to truly rescue all exceptions, this can be done.

```ruby
class Twitter::API < Grape::API
rescue_from :exception
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`.

Expand Down Expand Up @@ -2289,7 +2297,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

Expand Down
3 changes: 3 additions & 0 deletions lib/grape/dsl/request_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,9 @@ def rescue_from(*args, &block)
elsif args.include?(:grape_exceptions)
namespace_inheritable(:rescue_all, true)
namespace_inheritable(:rescue_grape_exceptions, true)
elsif args.include?(:exception)
namespace_inheritable(:rescue_exception, true)
namespace_inheritable(:exception_rescue_handler, handler)
else
handler_type =
case options[:rescue_subclasses]
Expand Down
4 changes: 3 additions & 1 deletion lib/grape/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,14 @@ def build_stack(helpers)
default_status: namespace_inheritable(:default_error_status),
rescue_all: namespace_inheritable(:rescue_all),
rescue_grape_exceptions: namespace_inheritable(:rescue_grape_exceptions),
rescue_exception: namespace_inheritable(:rescue_exception),
default_error_formatter: namespace_inheritable(:default_error_formatter),
error_formatters: namespace_stackable_with_hash(:error_formatters),
rescue_options: namespace_stackable_with_hash(:rescue_options) || {},
rescue_handlers: namespace_reverse_stackable_with_hash(:rescue_handlers) || {},
base_only_rescue_handlers: namespace_stackable_with_hash(:base_only_rescue_handlers) || {},
all_rescue_handler: namespace_inheritable(:all_rescue_handler)
all_rescue_handler: namespace_inheritable(:all_rescue_handler),
exception_rescue_handler: namespace_inheritable(:exception_rescue_handler)

stack.concat namespace_stackable(:middleware)

Expand Down
23 changes: 13 additions & 10 deletions lib/grape/middleware/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ def default_options
error_formatters: {},
rescue_all: false, # true to rescue all exceptions
rescue_grape_exceptions: false,
rescue_exception: false,
rescue_subclasses: true, # rescue subclasses of exceptions listed
rescue_options: {
backtrace: false, # true to display backtrace, true to let Grape handle Grape::Exceptions
Expand All @@ -36,7 +37,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) }
Expand All @@ -46,21 +47,14 @@ 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

def find_handler(klass)
handler = options[:rescue_handlers].find(-> { [] }) { |error, _| klass <= error }[1]
handler ||= options[:base_only_rescue_handlers][klass]
handler ||= options[:all_rescue_handler]
handler ||= options[:exception_rescue_handler]

if handler.instance_of?(Symbol)
raise NoMethodError, "undefined method `#{handler}'" unless respond_to?(handler)
Expand All @@ -72,7 +66,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_exception? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
else
rescue_exception? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
end
end

def rescuable_by_grape?(klass)
Expand Down Expand Up @@ -129,6 +128,10 @@ def rescue_all?
options[:rescue_all]
end

def rescue_exception?
options[:rescue_exception]
end

def rescue_class_or_its_ancestor?(klass)
(options[:rescue_handlers] || []).any? { |error, _handler| klass <= error }
end
Expand Down
41 changes: 41 additions & 0 deletions spec/grape/dsl/request_response_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,47 @@ def self.imbue(key, value)
end
end

describe ':exception' do
it 'sets rescue_exception to true' do
expect(subject).to receive(:namespace_inheritable).with(:rescue_exception, true)
expect(subject).to receive(:namespace_inheritable).with(:exception_rescue_handler, nil)
subject.rescue_from :exception
end

it 'sets given proc as rescue handler' do
rescue_handler_proc = proc {}
expect(subject).to receive(:namespace_inheritable).with(:rescue_exception, true)
expect(subject).to receive(:namespace_inheritable).with(:exception_rescue_handler, rescue_handler_proc)
subject.rescue_from :exception, rescue_handler_proc
end

it 'sets given block as rescue handler' do
rescue_handler_proc = proc {}
expect(subject).to receive(:namespace_inheritable).with(:rescue_exception, true)
expect(subject).to receive(:namespace_inheritable).with(:exception_rescue_handler, rescue_handler_proc)
subject.rescue_from :exception, &rescue_handler_proc
end

it 'sets a rescue handler declared through :with option' do
with_block = -> { 'hello' }
expect(subject).to receive(:namespace_inheritable).with(:rescue_exception, true)
expect(subject).to receive(:namespace_inheritable).with(:exception_rescue_handler, an_instance_of(Proc))
subject.rescue_from :exception, with: with_block
end

it 'abort if :with option value is not Symbol, String or Proc' do
expect { subject.rescue_from :exception, with: 1234 }.to raise_error(ArgumentError, "with: #{integer_class_name}, expected Symbol, String or Proc")
end

it 'abort if both :with option and block are passed' do
expect do
subject.rescue_from :exception, with: -> { 'hello' } do
error!('bye')
end
end.to raise_error(ArgumentError, 'both :with option and block cannot be passed')
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
71 changes: 63 additions & 8 deletions spec/grape/middleware/exception_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -324,4 +341,42 @@ def app
expect(last_response.body).to include('error', 'rain!', 'backtrace', 'original exception', 'RuntimeError')
end
end

context 'with rescue_exception' do
context 'StandardError raised' do
subject do
Rack::Builder.app do
use Spec::Support::EndpointFaker
use Grape::Middleware::Error, rescue_exception: true
run ExceptionSpec::ExceptionApp
end
end
it 'sets the message appropriately' do
get '/'
expect(last_response.body).to eq('rain!')
end
it 'defaults to a 500 status' do
get '/'
expect(last_response.status).to eq(500)
end
end

context 'Non-StandardError raised' do
subject do
Rack::Builder.app do
use Spec::Support::EndpointFaker
use Grape::Middleware::Error, rescue_exception: true
run ExceptionSpec::OtherExceptionApp
end
end
it 'sets the message appropriately' do
get '/'
expect(last_response.body).to eq('snow!')
end
it 'defaults to a 500 status' do
get '/'
expect(last_response.status).to eq(500)
end
end
end
end

0 comments on commit 42866b7

Please sign in to comment.