jhurliman/node-rate-limiter

Set current tokens remaining in constructor

hongkongkiwi opened this issue · 7 comments

In my case, I create the limiter object after the first call using the returned header.

So for now I create the object, then run a removeTokens function and ignore the result. It would be really fantastic if I could simply pass the remaining tokens for this interval into the constructor, that way I wouldn't have to make a simply removeTokens method with a blank callback.

Such as Limiter(120, 'minute', 119);

Yup. I have a somewhat similar use-case with a remote that allows quite agressive bursting and then a trickle of requests. So I have huge initial capacity and then a low-fill rate afterwards.

I tried hacking it, but it does seem the count is kept in a closure, so I can't get to it:

var limiter = new RateLimiter(1, 60 * 1000);
limiter.tokenBucket.content = 10;

Edit:

After hacking around a bit, I think I've found the offending part:

if (count > this.tokenBucket.tokensPerInterval - this.tokensThisInterval) {
  ...
}

In particular, it compares the requested amount of tokens (count) with how many tokens it gets per interval minus how many it has already used this interval.

Nowhere does it actually check how many tokens there are left in the bucket...

The check for how many tokens are left in the bucket happens one level deeper, in the call to this.tokenBucket.removeTokens(). A proposed fix for this was submitted in #41 but I'd prefer to add another parameter to the constructor as suggested here rather than changing the default behavior. If someone puts together a PR for this I'm happy to review and merge.

@hongkongkiwi , @jhurliman time to close this issue?

@SirR4T it's a valid request that no one has implemented yet; I think we can keep it open.

Sure, misunderstood the requirement first time round.

At first glance, this looks as simple as:

  • Accepting a new number parameter in constructor, say startWithTokens,
  • setting this.tokensThisInterval to (tokensPerInterval - startWithTokens) instead of 0, to ensure that this many tokens are already used up. (First call to removeTokens() will check that count is less than startWithTokens, until this interval is reset).

But then, this will not ensure that getTokensRemaining() returns startWithTokens (Before any calls to removeTokens() are made, say).

Is that ok? Or should this be fixed at the TokenBucket level?

I'd find it helpful if the TokenBucket constructor took an initial bucket content, or had a flag to indicate the bucket should start full instead of the current start empty behavior.

TokenBucket content is now exposed in 2.0.0