limiter.removeTokens of RateLimiterQueue with maximum wait time

[bkniffler]

I'd love to implement a maximum wait on removeTokens. Our serverless environment only allows execution time of maximum 30 seconds. I imagine we'd wait 25s, throw an error if we still don't have a token available. The current RateLimiterQueue could wait for hours or even days, which wouldn't work for this usecase.

Thought about something similar to

    const rateLimit = await Promise.race([
      limiter.removeTokens(1),
      waitFor(maxWait),
    ]);

But noticed that, obviously, the limiter.removeTokens is not cancelled and the token is removed even though we wouldn't permit the operation.

What do you think, is there any way to solve this without a PR on rate-limiter-flexible? With a PR I could imagine one of these solutions:

• adding support for CancelablePromise (as second argument on limiter.removeTokens)
• add support for AbortController (as second argument on limiter.removeTokens)
• have an optional ID (as second argument on limiter.removeTokens) and add another method limiter.cancelRemove(id)`

What do you think?

[animir]

@bkniffler Hi, this is interesting. I had a look into the codebase. I think it is better to implement an additional parameter for removeTokens method so that the process would be internal. I am afraid it would be not simple to deal with different edge cases if we allow disrupting the queue from outside. I would really try to avoid that.

After a quick check of the code, I think something like this would be the easiest to implement:

1. Add the second parameter expiresUnixAt = 0 to removeTokens function here https://github.com/animir/node-rate-limiter-flexible/blob/8c8f76138b792507d91b8431f73f67f3bc56b5ea/lib/RateLimiterQueue.js#L58

2. Set it to line 68 like this _this._queueRequest.call(_this, resolve, reject, tokens, expiresUnixAt);.

3. Add it to the function signature _queueRequest(resolve, reject, tokens, expiresUnixAt = 0) { on line 88 and push when queueing on line 91 _this._queue.push({resolve, reject, tokens, expiresUnixAt});.

4. Put this sketch of the code (I didn't test it). EDIT: I put nowSecs variable just now, so the code is more robust.

    const nowSecs = Math.floor(Date.now() / 1000)
    _this._queue.forEach((item) => {
      if (item.expiresUnixAt && item.expiresUnixAt >= nowSecs) {
        item.reject(new Error('expired'))
      }
    })
    _this._queue = _.this._queue.filter(
      item => !item.expiresUnixAt || item.expiresUnixAt < nowSecs)
    )
    if (_this._queue.length === 0) {
      return;
    }

before getting item from the queue on this line: https://github.com/animir/node-rate-limiter-flexible/blob/8c8f76138b792507d91b8431f73f67f3bc56b5ea/lib/RateLimiterQueue.js#L109

Note, the second check if (_this._queue.length === 0) { is necessary. It isn't mistake. The queue may be empty after expiring items.

This is the easiest since it is built into the normal cycle of FIFO queue processing of items in this package. If you are going to create a PR for that, please, write at least one test of the new feature. I hope this helps.