Add ability to check remaining requests without incrementing counter
jkirkwood opened this issue · 5 comments
I'm working to update the rate limiting on a login endpoint to look something like this:
- Check if rate limit exceeded (without incrementing counter). If rate limit has been exceeded block the request.
- Process login request
- If login request failed, increment the rate limit counter.
Essentially I just want to rate limit the route based on how many failed login attempts occur. In order to do this I need to be able to check the current rate limit at the beginning of the request without adding an entry to the counter.
At the end of a failed request I will then call the normal limiter
function to increment the rate limit counter.
For the redis implementation a new function could be added that uses the current multi without the zadd
command.
Is this something you're open to adding to this lib? If so, I can put together a PR.
Yeah, this seems fine. I could also imagine this could be useful for metrics purposes (e.g. viewing the current state of the rate limiter).
However, I do want to add a small warning that this does have a race condition, as it's possible that between the first request and the second request, a user might go from being blocked to not being blocked. As an example:
- Imagine the rate limit is 3 requests in 10 units of time
- Time 0: Check if blocked and do action
- Time 1: Check if blocked and do action
- Time 2: Check if blocked and do action (further actions from user are now blocked until time 5)
- Time 4: Check if blocked (user appears to be blocked)
- Time 5: Attempt to do action (contrary to expectations, action is not blocked)
Yeah makes sense, although I don't think this will be a problem for my use case.
I'm wondering how this new functionality should be exposed. Creating a new RateLimiter
returns a single function. Would it make sense to add an optional 3rd options
argument to this function, where one of the options is readOnly
?
Alternatively we could expose a RateLimiter
class with multiple methods, but this would obviously be a breaking change.
You could also add a property to the function? So the two operations would be rateLimiter()
and rateLimiter.check()
?
I can get behind that. I will update my PR to use that API.
As mentioned in your PR, there's a new version out that includes this functionality. You can see the release notes here.