Skip to content

Commit 42866b7

Browse files
committed
add rescue_from :exception to truly rescue all exceptions
1 parent 5613186 commit 42866b7

File tree

8 files changed

+135
-22
lines changed

8 files changed

+135
-22
lines changed

.rubocop_todo.yml

+2-2
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ Metrics/BlockLength:
3939
# Offense count: 9
4040
# Configuration parameters: CountComments.
4141
Metrics/ClassLength:
42-
Max: 288
42+
Max: 290
4343

4444
# Offense count: 32
4545
Metrics/CyclomaticComplexity:
@@ -54,7 +54,7 @@ Metrics/LineLength:
5454
# Offense count: 57
5555
# Configuration parameters: CountComments.
5656
Metrics/MethodLength:
57-
Max: 33
57+
Max: 34
5858

5959
# Offense count: 11
6060
# Configuration parameters: CountComments.

CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
#### Features
44

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

78
#### Fixes

README.md

+9-1
Original file line numberDiff line numberDiff line change
@@ -2083,6 +2083,14 @@ end
20832083
This mimics [default `rescue` behaviour](https://ruby-doc.org/core/StandardError.html) when an exception type is not provided.
20842084
Any other exception should be rescued explicitly, see [below](#exceptions-that-should-be-rescued-explicitly).
20852085

2086+
In the event you want to truly rescue all exceptions, this can be done.
2087+
2088+
```ruby
2089+
class Twitter::API < Grape::API
2090+
rescue_from :exception
2091+
end
2092+
```
2093+
20862094
Grape can also rescue from all exceptions and still use the built-in exception handing.
20872095
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`.
20882096

@@ -2289,7 +2297,7 @@ Here `'inner'` will be result of handling occured `ArgumentError`.
22892297

22902298
Any exception that is not subclass of `StandardError` should be rescued explicitly.
22912299
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).
2300+
This is following [standard recommendations for exceptions handling](https://ruby-doc.org/core/Exception.html).
22932301

22942302
### Rails 3.x
22952303

lib/grape/dsl/request_response.rb

+3
Original file line numberDiff line numberDiff line change
@@ -116,6 +116,9 @@ def rescue_from(*args, &block)
116116
elsif args.include?(:grape_exceptions)
117117
namespace_inheritable(:rescue_all, true)
118118
namespace_inheritable(:rescue_grape_exceptions, true)
119+
elsif args.include?(:exception)
120+
namespace_inheritable(:rescue_exception, true)
121+
namespace_inheritable(:exception_rescue_handler, handler)
119122
else
120123
handler_type =
121124
case options[:rescue_subclasses]

lib/grape/endpoint.rb

+3-1
Original file line numberDiff line numberDiff line change
@@ -285,12 +285,14 @@ def build_stack(helpers)
285285
default_status: namespace_inheritable(:default_error_status),
286286
rescue_all: namespace_inheritable(:rescue_all),
287287
rescue_grape_exceptions: namespace_inheritable(:rescue_grape_exceptions),
288+
rescue_exception: namespace_inheritable(:rescue_exception),
288289
default_error_formatter: namespace_inheritable(:default_error_formatter),
289290
error_formatters: namespace_stackable_with_hash(:error_formatters),
290291
rescue_options: namespace_stackable_with_hash(:rescue_options) || {},
291292
rescue_handlers: namespace_reverse_stackable_with_hash(:rescue_handlers) || {},
292293
base_only_rescue_handlers: namespace_stackable_with_hash(:base_only_rescue_handlers) || {},
293-
all_rescue_handler: namespace_inheritable(:all_rescue_handler)
294+
all_rescue_handler: namespace_inheritable(:all_rescue_handler),
295+
exception_rescue_handler: namespace_inheritable(:exception_rescue_handler)
294296

295297
stack.concat namespace_stackable(:middleware)
296298

lib/grape/middleware/error.rb

+13-10
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ def default_options
1313
error_formatters: {},
1414
rescue_all: false, # true to rescue all exceptions
1515
rescue_grape_exceptions: false,
16+
rescue_exception: false,
1617
rescue_subclasses: true, # rescue subclasses of exceptions listed
1718
rescue_options: {
1819
backtrace: false, # true to display backtrace, true to let Grape handle Grape::Exceptions
@@ -36,7 +37,7 @@ def call!(env)
3637
error_response(catch(:error) do
3738
return @app.call(@env)
3839
end)
39-
rescue StandardError => e
40+
rescue Exception => e # rubocop:disable Lint/RescueException
4041
is_rescuable = rescuable?(e.class)
4142
if e.is_a?(Grape::Exceptions::Base) && (!is_rescuable || rescuable_by_grape?(e.class))
4243
handler = ->(arg) { error_response(arg) }
@@ -46,21 +47,14 @@ def call!(env)
4647
end
4748

4849
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)
5750
end
5851
end
5952

6053
def find_handler(klass)
6154
handler = options[:rescue_handlers].find(-> { [] }) { |error, _| klass <= error }[1]
6255
handler ||= options[:base_only_rescue_handlers][klass]
6356
handler ||= options[:all_rescue_handler]
57+
handler ||= options[:exception_rescue_handler]
6458

6559
if handler.instance_of?(Symbol)
6660
raise NoMethodError, "undefined method `#{handler}'" unless respond_to?(handler)
@@ -72,7 +66,12 @@ def find_handler(klass)
7266

7367
def rescuable?(klass)
7468
return false if klass == Grape::Exceptions::InvalidVersionHeader
75-
rescue_all? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
69+
70+
if klass <= StandardError
71+
rescue_all? || rescue_exception? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
72+
else
73+
rescue_exception? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
74+
end
7675
end
7776

7877
def rescuable_by_grape?(klass)
@@ -129,6 +128,10 @@ def rescue_all?
129128
options[:rescue_all]
130129
end
131130

131+
def rescue_exception?
132+
options[:rescue_exception]
133+
end
134+
132135
def rescue_class_or_its_ancestor?(klass)
133136
(options[:rescue_handlers] || []).any? { |error, _handler| klass <= error }
134137
end

spec/grape/dsl/request_response_spec.rb

+41
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,47 @@ def self.imbue(key, value)
157157
end
158158
end
159159

160+
describe ':exception' do
161+
it 'sets rescue_exception to true' do
162+
expect(subject).to receive(:namespace_inheritable).with(:rescue_exception, true)
163+
expect(subject).to receive(:namespace_inheritable).with(:exception_rescue_handler, nil)
164+
subject.rescue_from :exception
165+
end
166+
167+
it 'sets given proc as rescue handler' do
168+
rescue_handler_proc = proc {}
169+
expect(subject).to receive(:namespace_inheritable).with(:rescue_exception, true)
170+
expect(subject).to receive(:namespace_inheritable).with(:exception_rescue_handler, rescue_handler_proc)
171+
subject.rescue_from :exception, rescue_handler_proc
172+
end
173+
174+
it 'sets given block as rescue handler' do
175+
rescue_handler_proc = proc {}
176+
expect(subject).to receive(:namespace_inheritable).with(:rescue_exception, true)
177+
expect(subject).to receive(:namespace_inheritable).with(:exception_rescue_handler, rescue_handler_proc)
178+
subject.rescue_from :exception, &rescue_handler_proc
179+
end
180+
181+
it 'sets a rescue handler declared through :with option' do
182+
with_block = -> { 'hello' }
183+
expect(subject).to receive(:namespace_inheritable).with(:rescue_exception, true)
184+
expect(subject).to receive(:namespace_inheritable).with(:exception_rescue_handler, an_instance_of(Proc))
185+
subject.rescue_from :exception, with: with_block
186+
end
187+
188+
it 'abort if :with option value is not Symbol, String or Proc' do
189+
expect { subject.rescue_from :exception, with: 1234 }.to raise_error(ArgumentError, "with: #{integer_class_name}, expected Symbol, String or Proc")
190+
end
191+
192+
it 'abort if both :with option and block are passed' do
193+
expect do
194+
subject.rescue_from :exception, with: -> { 'hello' } do
195+
error!('bye')
196+
end
197+
end.to raise_error(ArgumentError, 'both :with option and block cannot be passed')
198+
end
199+
end
200+
160201
describe 'list of exceptions is passed' do
161202
it 'sets hash of exceptions as rescue handlers' do
162203
expect(subject).to receive(:namespace_reverse_stackable).with(:rescue_handlers, StandardError => nil)

spec/grape/middleware/exception_spec.rb

+63-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
@@ -324,4 +341,42 @@ def app
324341
expect(last_response.body).to include('error', 'rain!', 'backtrace', 'original exception', 'RuntimeError')
325342
end
326343
end
344+
345+
context 'with rescue_exception' do
346+
context 'StandardError raised' do
347+
subject do
348+
Rack::Builder.app do
349+
use Spec::Support::EndpointFaker
350+
use Grape::Middleware::Error, rescue_exception: true
351+
run ExceptionSpec::ExceptionApp
352+
end
353+
end
354+
it 'sets the message appropriately' do
355+
get '/'
356+
expect(last_response.body).to eq('rain!')
357+
end
358+
it 'defaults to a 500 status' do
359+
get '/'
360+
expect(last_response.status).to eq(500)
361+
end
362+
end
363+
364+
context 'Non-StandardError raised' do
365+
subject do
366+
Rack::Builder.app do
367+
use Spec::Support::EndpointFaker
368+
use Grape::Middleware::Error, rescue_exception: true
369+
run ExceptionSpec::OtherExceptionApp
370+
end
371+
end
372+
it 'sets the message appropriately' do
373+
get '/'
374+
expect(last_response.body).to eq('snow!')
375+
end
376+
it 'defaults to a 500 status' do
377+
get '/'
378+
expect(last_response.status).to eq(500)
379+
end
380+
end
381+
end
327382
end

0 commit comments

Comments
 (0)