Skip to content

Commit

Permalink
Deny to pass values except Proc, Symbol or String to :with option of …
Browse files Browse the repository at this point in the history
…rescue_from
  • Loading branch information
namusyaka committed Jan 27, 2016
1 parent e3e8491 commit 114cc10
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 22 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
* [#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)
* [#1257](https://github.com/ruby-grape/grape/pull/1257): Deny to pass values except Proc, Symbol or String to :with option of rescue_from - [@namusyaka](https://github.com/namusyaka)
* Your contribution here.

#### Fixes
Expand Down
4 changes: 2 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
```

Expand Down
12 changes: 12 additions & 0 deletions UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
8 changes: 6 additions & 2 deletions lib/grape/dsl/request_response.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,12 @@ def rescue_from(*args, &block)
end

options = args.extract_options!
if options.key?(:with) && !options[:with].instance_of?(Symbol)
handler ||= proc { options[:with] }
if options.key?(:with)
if options[:with].instance_of?(Proc)
handler ||= options[:with]
elsif !(options[:with].instance_of?(Symbol) || options[:with].instance_of?(String))
fail ArgumentError, ":with option expected Symbol, String or Proc, got #{options[:with].class}"
end
end

if args.include?(:all)
Expand Down
15 changes: 0 additions & 15 deletions spec/grape/api_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
12 changes: 9 additions & 3 deletions spec/grape/dsl/request_response_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,14 @@ 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 option expected Symbol, String or Proc, got Fixnum')
end
end

Expand Down Expand Up @@ -158,9 +163,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
Expand Down

0 comments on commit 114cc10

Please sign in to comment.