Skip to content

Commit

Permalink
Merge pull request #1749 from dm1try/allow_explicitly_rescue_non_stan…
Browse files Browse the repository at this point in the history
…dard_errors

Allow rescue from non-StandardError exceptions
  • Loading branch information
dblock authored Mar 11, 2018
2 parents 18c9d4b + f77a998 commit 648b53a
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 2 deletions.
4 changes: 2 additions & 2 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,9 @@
#### Fixes

* [#1740](/~https://github.com/ruby-grape/grape/pull/1740): Fix dependent parameter validation using `given` when parameter is a `Hash` - [@jvortmann](/~https://github.com/jvortmann).

* Your contribution here.
* [#1737](/~https://github.com/ruby-grape/grape/pull/1737): Fix translating error when passing symbols as params in custom validations - [@mlzhuyi](/~https://github.com/mlzhuyi).
* [#1749](/~https://github.com/ruby-grape/grape/pull/1749): Allow rescue from non-`StandardError` exceptions - [@dm1try](/~https://github.com/dm1try).
* Your contribution here.

### 1.0.2 (1/10/2018)

Expand Down
10 changes: 10 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@
- [Exception Handling](#exception-handling)
- [Rescuing exceptions inside namespaces](#rescuing-exceptions-inside-namespaces)
- [Unrescuable Exceptions](#unrescuable-exceptions)
- [Exceptions that should be rescued explicitly](#exceptions-that-should-be-rescued-explicitly)
- [Rails 3.x](#rails-3x)
- [Logging](#logging)
- [API Formats](#api-formats)
Expand Down Expand Up @@ -2079,6 +2080,9 @@ class Twitter::API < Grape::API
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).

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 @@ -2281,6 +2285,12 @@ Here `'inner'` will be result of handling occured `ArgumentError`.

`Grape::Exceptions::InvalidVersionHeader`, which is raised when the version in the request header doesn't match the currently evaluated version for the endpoint, will _never_ be rescued from a `rescue_from` block (even a `rescue_from :all`) This is because Grape relies on Rack to catch that error and try the next versioned-route for cases where there exist identical Grape endpoints with different versions.

#### Exceptions that should be rescued explicitly

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).

### Rails 3.x

When mounted inside containers, such as Rails 3.x, errors such as "404 Not Found" or
Expand Down
8 changes: 8 additions & 0 deletions lib/grape/middleware/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,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

Expand Down
13 changes: 13 additions & 0 deletions spec/grape/middleware/exception_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,19 @@ def app
end
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
end
end
it 'rescues the exception using the provided handler' do
get '/'
expect(last_response.body).to eq('rescued')
end
end
context do
subject do
Rack::Builder.app do
Expand Down

0 comments on commit 648b53a

Please sign in to comment.