diff --git a/CHANGELOG.md b/CHANGELOG.md index c2a0cba425..58df587c89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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) ================== diff --git a/lib/grape/dsl/request_response.rb b/lib/grape/dsl/request_response.rb index 6590a46c97..f745209d7d 100644 --- a/lib/grape/dsl/request_response.rb +++ b/lib/grape/dsl/request_response.rb @@ -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 diff --git a/lib/grape/middleware/error.rb b/lib/grape/middleware/error.rb index 053585aac4..854c764270 100644 --- a/lib/grape/middleware/error.rb +++ b/lib/grape/middleware/error.rb @@ -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 diff --git a/spec/grape/api_spec.rb b/spec/grape/api_spec.rb index 83301936ca..476c8556f3 100644 --- a/spec/grape/api_spec.rb +++ b/spec/grape/api_spec.rb @@ -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 @@ -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 diff --git a/spec/grape/dsl/request_response_spec.rb b/spec/grape/dsl/request_response_spec.rb index ad550964fa..82047d4fa4 100644 --- a/spec/grape/dsl/request_response_spec.rb +++ b/spec/grape/dsl/request_response_spec.rb @@ -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