Correctly choose exception handler in error middleware. Fixes #1364.

Now exception handler in options can be a symbol (or string) name of
helper method to handle exception.

Author: ktimothy

CHANGELOG.md

@@ -4,8 +4,13 @@
 * Your contribution here.
 
 #### Features
+
 * [#1366](https://github.com/ruby-grape/grape/pull/1366): Store `message_key` on `Grape::Exceptions::Validation` - [@mkou](https://github.com/mkou).
 
+#### 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)
 ==================
 

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

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
 

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

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