JMPerez / promise-throttle

A small library to throttle promises. Useful to avoid rate limiting when using REST APIs.
https://doxdox.org/jmperez/promise-throttle
MIT License
149 stars 11 forks source link

Abort - takes time to reject pending promises #54

Open sudhakarnraju opened 6 years ago

sudhakarnraju commented 6 years ago

I am using AbortController to abort my promises. However when aborting, rather than immediate rejection of all pending promises, the library continues to throttle and reject ends up taking more time. E.g if there are 50 promises with throttle of 1 req/sec, you'll end up with 50 seconds for abort to finish.

Below is the code sequence I run. Is there anything I'm missing?

const promiseThrottle = new PromiseThrottle({
  requestsPerSecond: 1,
  promiseImplementation: Promise // the Promise library you are using
});
let controller = new AbortController();
let signal = controller.signal;
let myPromises = [];
myPromises.push[promiseThrottle.add(my_axios_api, { signal })];
myPromises.push[promiseThrottle.add(my_axios_api { signal })];
return Promise.all(myPromises);
// and elsewhere , i loop and poll, if cancel should be aborted
this.cancelPoller = setInterval(() => {
  if (this.shouldCancelUpload()) {
    controller.abort();
  }
}, 1000); //check every 1 second if upload has to be cancelled
JMPerez commented 6 years ago

At the moment if you pass the signal to a single promise it will cancel that promise. The library doesn't take into account the case in which you are using the same signal for several promises added individually.

I think you can apply the example shown in the README file, where the signal is passed to the whole set of promises queued.

const promiseThrottle = new PromiseThrottle({
  requestsPerSecond: 1,
  promiseImplementation: Promise
});
let controller = new AbortController();
let signal = controller.signal;
return promiseThrottle.addAll([my_axios_api, my_axios_api], {signal});

Let me know if that works!