Skip to content

Commit 4412b81

Browse files
committed
fixup! add rescue_from :exception to truly rescue all exceptions
1 parent 42866b7 commit 4412b81

File tree

7 files changed

+4
-102
lines changed

7 files changed

+4
-102
lines changed

CHANGELOG.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
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).
65
* Your contribution here.
76

87
#### Fixes
@@ -12,6 +11,7 @@
1211
* [#1749](https://github.com/ruby-grape/grape/pull/1749): Allow rescue from non-`StandardError` exceptions - [@dm1try](https://github.com/dm1try).
1312
* [#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).
1413
* [#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).
1515
* Your contribution here.
1616

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

README.md

-8
Original file line numberDiff line numberDiff line change
@@ -2083,14 +2083,6 @@ 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-
20942086
Grape can also rescue from all exceptions and still use the built-in exception handing.
20952087
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`.
20962088

lib/grape/dsl/request_response.rb

-3
Original file line numberDiff line numberDiff line change
@@ -116,9 +116,6 @@ 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)
122119
else
123120
handler_type =
124121
case options[:rescue_subclasses]

lib/grape/endpoint.rb

+1-3
Original file line numberDiff line numberDiff line change
@@ -285,14 +285,12 @@ 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),
289288
default_error_formatter: namespace_inheritable(:default_error_formatter),
290289
error_formatters: namespace_stackable_with_hash(:error_formatters),
291290
rescue_options: namespace_stackable_with_hash(:rescue_options) || {},
292291
rescue_handlers: namespace_reverse_stackable_with_hash(:rescue_handlers) || {},
293292
base_only_rescue_handlers: namespace_stackable_with_hash(:base_only_rescue_handlers) || {},
294-
all_rescue_handler: namespace_inheritable(:all_rescue_handler),
295-
exception_rescue_handler: namespace_inheritable(:exception_rescue_handler)
293+
all_rescue_handler: namespace_inheritable(:all_rescue_handler)
296294

297295
stack.concat namespace_stackable(:middleware)
298296

lib/grape/middleware/error.rb

+2-8
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@ def default_options
1313
error_formatters: {},
1414
rescue_all: false, # true to rescue all exceptions
1515
rescue_grape_exceptions: false,
16-
rescue_exception: false,
1716
rescue_subclasses: true, # rescue subclasses of exceptions listed
1817
rescue_options: {
1918
backtrace: false, # true to display backtrace, true to let Grape handle Grape::Exceptions
@@ -54,7 +53,6 @@ def find_handler(klass)
5453
handler = options[:rescue_handlers].find(-> { [] }) { |error, _| klass <= error }[1]
5554
handler ||= options[:base_only_rescue_handlers][klass]
5655
handler ||= options[:all_rescue_handler]
57-
handler ||= options[:exception_rescue_handler]
5856

5957
if handler.instance_of?(Symbol)
6058
raise NoMethodError, "undefined method `#{handler}'" unless respond_to?(handler)
@@ -68,9 +66,9 @@ def rescuable?(klass)
6866
return false if klass == Grape::Exceptions::InvalidVersionHeader
6967

7068
if klass <= StandardError
71-
rescue_all? || rescue_exception? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
69+
rescue_all? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
7270
else
73-
rescue_exception? || rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
71+
rescue_class_or_its_ancestor?(klass) || rescue_with_base_only_handler?(klass)
7472
end
7573
end
7674

@@ -128,10 +126,6 @@ def rescue_all?
128126
options[:rescue_all]
129127
end
130128

131-
def rescue_exception?
132-
options[:rescue_exception]
133-
end
134-
135129
def rescue_class_or_its_ancestor?(klass)
136130
(options[:rescue_handlers] || []).any? { |error, _handler| klass <= error }
137131
end

spec/grape/dsl/request_response_spec.rb

-41
Original file line numberDiff line numberDiff line change
@@ -157,47 +157,6 @@ 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-
201160
describe 'list of exceptions is passed' do
202161
it 'sets hash of exceptions as rescue handlers' do
203162
expect(subject).to receive(:namespace_reverse_stackable).with(:rescue_handlers, StandardError => nil)

spec/grape/middleware/exception_spec.rb

-38
Original file line numberDiff line numberDiff line change
@@ -341,42 +341,4 @@ def app
341341
expect(last_response.body).to include('error', 'rain!', 'backtrace', 'original exception', 'RuntimeError')
342342
end
343343
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
382344
end

0 commit comments

Comments
 (0)