Skip to content

Conversation

nicokaiser
Copy link
Contributor

In some scenarios it might be useful to be able to read the current "remaining" value for a limiter.

In this example, new login attempts are rejected when more at least 10 unsuccessful login attempts happened in the last 60 seconds.

const rateLimiter = new RateLimiter({
  db: new Redis()
  max: 10
  duration: 60 * 1000
})

const loginHandler = async (req, res, next) => {
  const limit = await rateLimiter.get({ id: req.clientIp, decrease: false })
  if (!limit.remaining) return sendError(req, res, 429)

  try {
    await doLogin(req);// or something like this
  } catch (err) {
    if (err) {
      await rateLimiter.get({ id: req.clientIp })
      return sendError(req, res, 401)
    }
  }

  next(req, res)
}

This is done with a new decrease parameter that controls if the remaining value should actually be decreased (default: true) or if the request is only done to read the current value.

What do you think?

@coveralls
Copy link

coveralls commented Oct 11, 2018

Pull Request Test Coverage Report for Build 40

  • 10 of 10 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.7%) to 98.077%

Totals Coverage Status
Change from base Build 39: 0.7%
Covered Lines: 34
Relevant Lines: 34

💛 - Coveralls

@nicokaiser
Copy link
Contributor Author

If you think this PR is useful, I can try to also implement this for tj/node-ratelimiter to make it consistent again.

@Kikobeats
Copy link
Member

why not read the value directly from redis?

@nicokaiser
Copy link
Contributor Author

why not read the value directly from redis?

That would be possible, but the "sliding window" effect would be gone then. When reading the value directly from Redis (with zcard), the count is only reset to 0 at duration after the last get call. When using get (or the PR implementation), the count is decreased duration after the first get call.

@Kikobeats
Copy link
Member

Hello @nicokaiser, can you implement it at tj/node-ratelimiter? I interested in be 1:1 replacement

@ghmeier
Copy link

ghmeier commented Mar 26, 2019

Any updates on this? It seems like essential functionality :)

@Kikobeats Kikobeats merged commit 924822e into microlinkhq:master Mar 27, 2019
@Kikobeats
Copy link
Member

Sorry for the delay! Added into the codebase, also I'm going to add the example as part of the documentation.

Thanks for all 🙂

@ghmeier
Copy link

ghmeier commented Mar 27, 2019

Oh yay! Thanks a bunch @Kikobeats!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants