WeTransfer / prorate

Redis-based rate limiter (with a leaky bucket implementation in Lua)
MIT License
88 stars 5 forks source link

Would you be open to adding a 'throttle' method (without the '!')? #36

Open oskarpearson opened 3 years ago

oskarpearson commented 3 years ago

Hi Team. Love the gem

I was wondering if you'd be open to adding a 'throttle' method which calls run_lua_throttler but doesn't raise the exception? I'm happy to create a PR if required.

I was expecting status to do this, but that just returns data about an existing throttle (if one exists). It doesn't execute run_lua_throttler

There are a few reasons for this:

  1. In some cases, I'd like to raise my own exception, rather than catching Prorate::Throttled and immediately raising my own exception.
  2. One scenario I'd like to use the gem for is rate-limiting calls we make out. In this case, exceeding the limit isn't really an exception, just a feedback mechanism for other pieces of the code. It seems ugly to catch an exception when it's not actually an exception.

Most importantly, I have a situation where I'd like to have (say) three rate limits apply to the same connection, with different timeframes. For example, I may have a rate limit over a 10 minute period, but also a slow-burn rate limit over a 24h period.

I was thinking of doing something like this in the controller:

configured_throttlers.each do |throttler|
  throttler.throttle!
end

The problem with is that if I do the above, the second throttler (the slow-burn counter) never increments if the short-term limiting raises an exception, since the second throttler is never reached. So I'd need to do something conceptionally like this:

active_throttlers = false

configured_throttlers.map do |throttler|
  begin
    throttler.throttle!
  rescue Prorate::Throttled
    active_throttlers = true
  end
end

raise MyException if active_throttlers

@julik

julik commented 3 years ago

Sure, go ahead! We also use something similar for layered throttling already

On 16 Feb 2021, at 03:08, Oskar Pearson notifications@github.com wrote:

raise MyException if active_throttlers @julik