Skip to content

Commit e2e6bd9

Browse files
committed
allow rescue from non-StandardError exceptions to use default error handling
1 parent 5613186 commit e2e6bd9

File tree

4 files changed

+34
-19
lines changed

4 files changed

+34
-19
lines changed

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
* [#1749](https://github.com/ruby-grape/grape/pull/1749): Allow rescue from non-`StandardError` exceptions - [@dm1try](https://github.com/dm1try).
1212
* [#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).
1313
* [#1752](https://github.com/ruby-grape/grape/pull/1752): Fix `include_missing` behavior for aliased parameters - [@jonasoberschweiber](https://github.com/jonasoberschweiber).
14+
* [#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).
1415
* Your contribution here.
1516

1617
### 1.0.2 (1/10/2018)

README.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -2289,7 +2289,7 @@ Here `'inner'` will be result of handling occured `ArgumentError`.
22892289

22902290
Any exception that is not subclass of `StandardError` should be rescued explicitly.
22912291
Usually it is not a case for an application logic as such errors point to problems in Ruby runtime.
2292-
This is following [standard recomendations for exceptions handling](https://ruby-doc.org/core/Exception.html).
2292+
This is following [standard recommendations for exceptions handling](https://ruby-doc.org/core/Exception.html).
22932293

22942294
### Rails 3.x
22952295

lib/grape/middleware/error.rb

+7-10
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ def call!(env)
3636
error_response(catch(:error) do
3737
return @app.call(@env)
3838
end)
39-
rescue StandardError => e
39+
rescue Exception => e # rubocop:disable Lint/RescueException
4040
is_rescuable = rescuable?(e.class)
4141
if e.is_a?(Grape::Exceptions::Base) && (!is_rescuable || rescuable_by_grape?(e.class))
4242
handler = ->(arg) { error_response(arg) }
@@ -46,14 +46,6 @@ def call!(env)
4646
end
4747

4848
handler.nil? ? handle_error(e) : exec_handler(e, &handler)
49-
rescue Exception => e # rubocop:disable Lint/RescueException
50-
handler = options[:rescue_handlers].find do |error_class, error_handler|
51-
break error_handler if e.class <= error_class
52-
end
53-
54-
raise unless handler
55-
56-
exec_handler(e, &handler)
5749
end
5850
end
5951

@@ -72,7 +64,12 @@ def find_handler(klass)
7264

7365
def rescuable?(klass)
7466
return false if klass == Grape::Exceptions::InvalidVersionHeader
75-
rescue_all? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
67+
68+
if klass <= StandardError
69+
rescue_all? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
70+
else
71+
rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
72+
end
7673
end
7774

7875
def rescuable_by_grape?(klass)

spec/grape/middleware/exception_spec.rb

+25-8
Original file line numberDiff line numberDiff line change
@@ -110,18 +110,35 @@ def app
110110
end
111111

112112
context 'Non-StandardError exception with a provided rescue handler' do
113-
subject do
114-
Rack::Builder.app do
115-
use Spec::Support::EndpointFaker
116-
use Grape::Middleware::Error, rescue_handlers: { NotImplementedError => -> { [200, {}, 'rescued'] } }
117-
run ExceptionSpec::OtherExceptionApp
113+
context 'default error response' do
114+
subject do
115+
Rack::Builder.app do
116+
use Spec::Support::EndpointFaker
117+
use Grape::Middleware::Error, rescue_handlers: { NotImplementedError => nil }
118+
run ExceptionSpec::OtherExceptionApp
119+
end
120+
end
121+
it 'rescues the exception using the default handler' do
122+
get '/'
123+
expect(last_response.body).to eq('snow!')
118124
end
119125
end
120-
it 'rescues the exception using the provided handler' do
121-
get '/'
122-
expect(last_response.body).to eq('rescued')
126+
127+
context 'custom error response' do
128+
subject do
129+
Rack::Builder.app do
130+
use Spec::Support::EndpointFaker
131+
use Grape::Middleware::Error, rescue_handlers: { NotImplementedError => -> { [200, {}, 'rescued'] } }
132+
run ExceptionSpec::OtherExceptionApp
133+
end
134+
end
135+
it 'rescues the exception using the provided handler' do
136+
get '/'
137+
expect(last_response.body).to eq('rescued')
138+
end
123139
end
124140
end
141+
125142
context do
126143
subject do
127144
Rack::Builder.app do

0 commit comments

Comments
 (0)