Skip to content

Commit

Permalink
Correctly choose exception handler in error middleware. Fixes #1364.
Browse files Browse the repository at this point in the history
Now exception handler in options can be a symbol (or string) name of
helper method to handle exception.
  • Loading branch information
ktimothy committed Apr 22, 2016
1 parent 0331563 commit ad89c7b
Show file tree
Hide file tree
Showing 5 changed files with 50 additions and 10 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@

* Your contribution here.

#### Fixes

* [#1365](https://github.com/ruby-grape/grape/pull/1365): Fix finding exception handler in error middleware - [@ktimothy](https://github.com/ktimothy).

0.16.2 (4/12/2016)
==================

Expand Down
4 changes: 2 additions & 2 deletions lib/grape/dsl/request_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -157,9 +157,9 @@ def represent(model_class, options)

def extract_with(options)
return unless options.key?(:with)
with_option = options[:with]
with_option = options.delete(:with)
return with_option if with_option.instance_of?(Proc)
return if with_option.instance_of?(Symbol) || with_option.instance_of?(String)
return with_option.to_sym if with_option.instance_of?(Symbol) || with_option.instance_of?(String)
fail ArgumentError, "with: #{with_option.class}, expected Symbol, String or Proc"
end
end
Expand Down
11 changes: 6 additions & 5 deletions lib/grape/middleware/error.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,15 @@ 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]
with_option = options[:rescue_options][:with]
if with_option.instance_of?(Symbol)
if respond_to?(with_option)
handler ||= self.class.instance_method(with_option).bind(self)

if handler.instance_of?(Symbol)
if respond_to?(handler)
handler = self.class.instance_method(handler).bind(self)
else
fail NoMethodError, "undefined method `#{with_option}'"
fail NoMethodError, "undefined method `#{handler}'"
end
end

handler
end

Expand Down
39 changes: 37 additions & 2 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1593,13 +1593,23 @@ class CommunicationError < StandardError; end
def rescue_arg_error
error!('500 ArgumentError', 500)
end

def rescue_no_method_error
error!('500 NoMethodError', 500)
end
end
subject.rescue_from ArgumentError, with: :rescue_arg_error
subject.get('/rescue_method') { fail ArgumentError }
subject.rescue_from NoMethodError, with: :rescue_no_method_error
subject.get('/rescue_arg_error') { fail ArgumentError }
subject.get('/rescue_no_method_error') { fail NoMethodError }

get '/rescue_method'
get '/rescue_arg_error'
expect(last_response.status).to eq(500)
expect(last_response.body).to eq('500 ArgumentError')

get '/rescue_no_method_error'
expect(last_response.status).to eq(500)
expect(last_response.body).to eq('500 NoMethodError')
end

it 'aborts if the specified method name does not exist' do
Expand All @@ -1608,6 +1618,31 @@ def rescue_arg_error

expect { get '/rescue_method' }.to raise_error(NoMethodError, 'undefined method `not_exist_method\'')
end

it 'correctly chooses exception handler if :all handler is specified' do
subject.helpers do
def rescue_arg_error
error!('500 ArgumentError', 500)
end

def rescue_all_errors
error!('500 AnotherError', 500)
end
end

subject.rescue_from ArgumentError, with: :rescue_arg_error
subject.rescue_from :all, with: :rescue_all_errors
subject.get('/argument_error') { fail ArgumentError }
subject.get('/another_error') { fail NoMethodError }

get '/argument_error'
expect(last_response.status).to eq(500)
expect(last_response.body).to eq('500 ArgumentError')

get '/another_error'
expect(last_response.status).to eq(500)
expect(last_response.body).to eq('500 AnotherError')
end
end

describe '.rescue_from klass, rescue_subclasses: boolean' do
Expand Down
2 changes: 1 addition & 1 deletion spec/grape/dsl/request_response_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ def self.imbue(key, value)
it 'sets a rescue handler declared through :with option for each key in hash' do
with_block = -> { 'hello' }
expect(subject).to receive(:namespace_stackable).with(:rescue_handlers, StandardError => an_instance_of(Proc))
expect(subject).to receive(:namespace_stackable).with(:rescue_options, with: with_block)
expect(subject).to receive(:namespace_stackable).with(:rescue_options, {})
subject.rescue_from StandardError, with: with_block
end
end
Expand Down

0 comments on commit ad89c7b

Please sign in to comment.