Deny to pass values except Proc, Symbol or String to :with option of …

…rescue_from.

CHANGELOG.md

@@ -3,14 +3,14 @@
 
 #### Features
 
-* [#1227](https://github.com/ruby-grape/grape/pull/1227): Store `message_key` on Grape::Exceptions::Validation - [@stjhimy](https://github.com/sthimy).
+* [#1227](https://github.com/ruby-grape/grape/pull/1227): Store `message_key` on `Grape::Exceptions::Validation` - [@stjhimy](https://github.com/sthimy).
 * [#1232](https://github.com/ruby-grape/grape/pull/1232): Helpers are now available inside `rescue_from` - [@namusyaka](https://github.com/namusyaka).
 * [#1237](https://github.com/ruby-grape/grape/pull/1237): Allow multiple parameters in `given`, which behaves as if the scopes were nested in the inputted order - [@ochagata](https://github.com/ochagata).
 * [#1238](https://github.com/ruby-grape/grape/pull/1238): Call `after` of middleware on error - [@namusyaka](https://github.com/namusyaka).
 * [#1243](https://github.com/ruby-grape/grape/pull/1243): Add `header` support for middleware - [@namusyaka](https://github.com/namusyaka).
 * [#1252](https://github.com/ruby-grape/grape/pull/1252): Allow default to be a subset or equal to allowed values without raising IncompatibleOptionValues - [@jeradphelps](https://github.com/jeradphelps).
-* [#1255](https://github.com/ruby-grape/grape/pull/1255): Allow param type definition in route_param - [@namusyaka](https://github.com/namusyaka)
-* [#1257](https://github.com/ruby-grape/grape/pull/1257): Allow to pass method name to :with option of rescue_from - [@namusyaka](https://github.com/namusyaka)
+* [#1255](https://github.com/ruby-grape/grape/pull/1255): Allow param type definition in `route_param` - [@namusyaka](https://github.com/namusyaka).
+* [#1257](https://github.com/ruby-grape/grape/pull/1257): Allow Proc, Symbol or String in `rescue_from with: ...` - [@namusyaka](https://github.com/namusyaka).
 * Your contribution here.
 
 #### Fixes

README.md

@@ -1771,7 +1771,7 @@ end
 
 The `rescue_from` block must return a `Rack::Response` object, call `error!` or re-raise an exception.
 
-The `with` keyword is available as `rescue_from` options, it can be passed method name or `Rack::Response` object.
+The `with` keyword is available as `rescue_from` options, it can be passed method name or Proc object.
 
 ```ruby
 class Twitter::API < Grape::API
@@ -1783,7 +1783,7 @@ class Twitter::API < Grape::API
   end
 
   rescue_from :all,          with: :server_error!
-  rescue_from ArgumentError, with: Rack::Response.new('rescued with a method', 400)
+  rescue_from ArgumentError, with: -> { Rack::Response.new('rescued with a method', 400) }
 end
 ```
 

UPGRADING.md

@@ -3,6 +3,18 @@ Upgrading Grape
 
 ### Upgrading to >= 0.15.0
 
+#### Changes to availability of `:with` option of `rescue_from` method
+
+The `:with` option of `rescue_from` does not accept value except Proc, String or Symbol now.
+
+If you have been depending the old behavior, you should use lambda block instead.
+
+```ruby
+class API < Grape::API
+  rescue_from :all, with: -> { Rack::Response.new('rescued with a method', 400) }
+end
+```
+
 #### Changes to behavior of `after` method of middleware on error
 
 The `after` method of the middleware is now called on error.

lib/grape/dsl/request_response.rb

@@ -105,9 +105,10 @@ def rescue_from(*args, &block)
           end
 
           options = args.extract_options!
-          if options.key?(:with) && !options[:with].instance_of?(Symbol)
-            handler ||= proc { options[:with] }
+          if block_given? && options.key?(:with)
+            fail ArgumentError, 'both :with option and block cannot be passed'
           end
+          handler ||= extract_with(options)
 
           if args.include?(:all)
             namespace_inheritable(:rescue_all, true)
@@ -151,6 +152,16 @@ def represent(model_class, options)
           fail Grape::Exceptions::InvalidWithOptionForRepresent.new unless options[:with] && options[:with].is_a?(Class)
           namespace_stackable(:representations, model_class => options[:with])
         end
+
+        private
+
+        def extract_with(options)
+          return unless options.key?(:with)
+          with_option = options[:with]
+          return with_option if with_option.instance_of?(Proc)
+          return if with_option.instance_of?(Symbol) || with_option.instance_of?(String)
+          fail ArgumentError, "with: #{with_option.class}, expected Symbol, String or Proc"
+        end
       end
     end
   end

lib/grape/middleware/error.rb

@@ -50,7 +50,7 @@ def find_handler(klass)
           if respond_to?(with_option)
             handler ||= self.class.instance_method(with_option).bind(self)
           else
-            fail NoMethodError, "undefined method `#{with_option}' for your application"
+            fail NoMethodError, "undefined method `#{with_option}'"
           end
         end
         handler

spec/grape/api_spec.rb

@@ -1503,21 +1503,6 @@ class CommunicationError < StandardError; end
     end
   end
 
-  describe '.rescue_from klass, with: method' do
-    it 'rescues an error with the specified message' do
-      def rescue_arg_error
-        Rack::Response.new('rescued with a method', 400)
-      end
-
-      subject.rescue_from ArgumentError, with: rescue_arg_error
-      subject.get('/rescue_method') { fail ArgumentError }
-
-      get '/rescue_method'
-      expect(last_response.status).to eq(400)
-      expect(last_response.body).to eq('rescued with a method')
-    end
-  end
-
   describe '.rescue_from klass, with: :method_name' do
     it 'rescues an error with the specified method name' do
       subject.helpers do
@@ -1537,7 +1522,7 @@ def rescue_arg_error
       subject.rescue_from :all, with: :not_exist_method
       subject.get('/rescue_method') { fail StandardError }
 
-      expect { get '/rescue_method' }.to raise_error(NoMethodError, 'undefined method `not_exist_method\' for your application')
+      expect { get '/rescue_method' }.to raise_error(NoMethodError, 'undefined method `not_exist_method\'')
     end
   end
 

spec/grape/dsl/request_response_spec.rb

@@ -124,9 +124,22 @@ def self.imbue(key, value)
           end
 
           it 'sets a rescue handler declared through :with option' do
+            with_block = -> { 'hello' }
             expect(subject).to receive(:namespace_inheritable).with(:rescue_all, true)
             expect(subject).to receive(:namespace_inheritable).with(:all_rescue_handler, an_instance_of(Proc))
-            subject.rescue_from :all, with: 'ExampleHandler'
+            subject.rescue_from :all, with: with_block
+          end
+
+          it 'abort if :with option value is not Symbol, String or Proc' do
+            expect { subject.rescue_from :all, with: 1234 }.to raise_error(ArgumentError, 'with: Fixnum, expected Symbol, String or Proc')
+          end
+
+          it 'abort if both :with option and block are passed' do
+            expect do
+              subject.rescue_from :all, with: -> { 'hello' } do
+                error!('bye')
+              end
+            end.to raise_error(ArgumentError, 'both :with option and block cannot be passed')
           end
         end
 
@@ -158,9 +171,10 @@ def self.imbue(key, value)
           end
 
           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: 'ExampleHandler')
-            subject.rescue_from StandardError, with: 'ExampleHandler'
+            expect(subject).to receive(:namespace_stackable).with(:rescue_options, with: with_block)
+            subject.rescue_from StandardError, with: with_block
           end
         end
       end