Avoid rescue nil in the default throttled response

It has a couple of cons:

1. If we slip a typo in the whole line, we won't easily catch it.  Can
   you guys spot the problem problem in the following line? Chasing such
   issues is quite tricky.

```ruby
retry_after = evn['rack.attack.match_data'][:period] rescue nil
```

2. Throwing and catching an exception is quite slower than a new hash
   allocation, so there is a speed benefit too.

We are guaranteed from Rack that env is a `Hash`, so we can even use
`Hash#fetch`.

```ruby
retry_after = env.fetch('rack.attack.match_data', {})[:period]
```

This reads better, but always allocates the default value hash, when the
other version allocates it only when needed. If you prefer `Hash#fetch`,
I'm fine with that, as long as we avoid `rescue nil`.
This commit is contained in:
Genadi Samokovarov 2014-09-25 18:13:18 +02:00
parent 60d4834969
commit f737dbb78c

View file

@ -82,7 +82,7 @@ class Rack::Attack
@notifier = ActiveSupport::Notifications if defined?(ActiveSupport::Notifications)
@blacklisted_response = lambda {|env| [403, {'Content-Type' => 'text/plain'}, ["Forbidden\n"]] }
@throttled_response = lambda {|env|
retry_after = env['rack.attack.match_data'][:period] rescue nil
retry_after = (env['rack.attack.match_data'] || {})[:period]
[429, {'Content-Type' => 'text/plain', 'Retry-After' => retry_after.to_s}, ["Retry later\n"]]
}