QUESTION: cancel removeTokens()

[daldridge-cs]

I'm wondering how folks are gracefully cancelling/aborting calls to RateLimiter.removeTokens()?

Here's a sample use case where I'd want to cancel:

1. User registers with my app to pull Fitbit data as it becomes available
2. App adds a listener for notifications from Fitbit that new data is available
3. App instantiates a subscription handler that queues inbound updates and throttles requests to FB via a RateLimiter instance):

class SubscriptionHandler {
  constructor(key, handlerFn) {
    this.key = key;
    this.handlerFn = handlerFn;
    this.rateLimiter = new limiter.RateLimiter(FITBIT_LIMIT_PER_USER_PER_HOUR, 'hour');
    this.queue = async.queue((notificationId, callback) => {
      this.rateLimiter.removeTokens(1, (error, remainingRequests) => {
        this._processNotification(notificationId, callback);
      });
    });

  enqueue(data, completionCallback) {
    this.queue.push(data.id);
  }

const myHandler = new SubscriptionHandler(myKey, myFBDataFetch);

3. Upon receiving such a notification, the app enqueues its handling, which occurs when RateLimiter::removeTokens() successfully drains its token bucket.
4. User de-registers before any number of enqueued notifications have been processed (correctly on-hold/pending by the rate limiter)
5. ???

Step (5) is what's in question. I'd like to cancel the wait on removeTokens() and allow the handler instance to be garbage collected:

class SubscriptionHandler {
  constructor(key, handlerFn) {
    this.key = key;
    this.handlerFn = handlerFn;
    this.rateLimiter = new limiter.RateLimiter(FITBIT_LIMIT_PER_USER_PER_HOUR, 'hour');
    this.queue = async.queue((notificationId, callback) => {
      this.tokenTimeoutID = this.rateLimiter.removeTokens(1, (error, remainingRequests) => {
        this._processNotification(notificationId, callback);
      });
    });

  release() {
    // Maybe something like this?
    clearTimeout(this.tokenTimeoutID);
    delete this.rateLimiter;
    this.queue.kill();
    delete this.queue;
  }
}

myHandler.release();
delete myHandler;

Unfortunately, with the existing implementation of RateLimiter and its internal TokenBucket, there is no way to obtain the timeoutID assigned when TokenBucket invokes its internal comeBackLater() method. (See https://github.com/jhurliman/node-rate-limiter/blob/master/lib/tokenBucket.js, line 108). Thus, there is always an outstanding reference to the SubscriptionHandler instance, and i cannot get it garbage collected.

If the timer ID allocated by setTimeout() were returned from comeBackLater() (rather than the false value currently returned) and bubbled back out as the return value from RateLimiter::removeTokens(), then I could cancel it.

Obviously that breaks encapsulation, and perhaps the RateLimiter or its internal TokenBucket could keep rack of the timer ID and have methods for cancelling it, but I'm simply trying to get my idea across...

Given the above scenario, what would you recommend? @jhurliman ?

(Note that I prefer the semantics of the asynchronous removeTokens() method, and would rather not switch to my own periodic retry calling tryRemoveTokens()...)

[jhurliman]

Hi, apologies for the delay. You're right that there is currently no way to cancel in-progress requests. It's something I would be open to adding, as long as it can be done in a way that doesn't expose too much of the internals (i.e. add a cancel method that takes an ID rather than exposing the fact that setTimeout / clearTimeout is used under the hood). Right now I think what most people are doing is using the try* variant that returns immediately, but that pushes a lot of complexity up into your code if you need to manage a queue of pending token removals.

[jhurliman]

This is essentially the same issue / request as #37

[thyming]

the node:timers/promises variant of setTimeout optionally accepts an AbortSignal. This would, however, make node 16 the minimum node version for this library.