SGrondin / bottleneck

Job scheduler and rate limiter, supports Clustering
MIT License
1.83k stars 79 forks source link

Retries can exceed rate limiting options #170

Open scooper91 opened 4 years ago

scooper91 commented 4 years ago

Hi!

I'm trying to rate limit calls to an API, for example, kick off one request every second. Using this library works fine, until I add in retries. The retry is ignoring the reservoir and minTime options, meaning I would potentially be hitting the API more than once per second. I understand the retrying doesn't use the minTime option - I am specifying the same minTime in the failed listener.

Consider the following, very oversimplified, code:

process.on('unhandledRejection', () => {});
const Bottleneck = require('bottleneck');

const bottleneck = new Bottleneck({
  minTime: 1000,
  reservoir: 1,
  reservoirRefreshAmount: 1,
  reservoirRefreshInterval: 1000
});

bottleneck.on('failed', (error, jobInfo) => {
  if (jobInfo.retryCount === 0) { return 1000; }
});

const job = async i => {
  console.log('running', i, new Date());
  if (i % 2 === 0) { throw new Error('bang'); }
};

for (let i = 0; i < 10; i++) { bottleneck.schedule(() => job(i)); }

Output:

➜ node .
running 0 2020-09-24T14:48:48.958Z
running 1 2020-09-24T14:48:49.957Z
running 0 2020-09-24T14:48:49.975Z
running 2 2020-09-24T14:48:50.957Z
running 3 2020-09-24T14:48:51.957Z
running 2 2020-09-24T14:48:51.959Z
running 4 2020-09-24T14:48:52.959Z
running 5 2020-09-24T14:48:53.961Z
running 4 2020-09-24T14:48:53.963Z
running 6 2020-09-24T14:48:54.964Z
running 6 2020-09-24T14:48:55.965Z
running 7 2020-09-24T14:48:55.968Z
running 8 2020-09-24T14:48:56.968Z
running 8 2020-09-24T14:48:57.969Z
running 9 2020-09-24T14:48:57.970Z

I would expect the retry to happen one second after it failed the first time. I would then expect the next job in the queue to occur one second later. However, I'm seeing the failed job and the next job happening at around the same time.

I understand from the docs that if I were to use maxConcurrent, the next job wouldn't be kicked off if there were too many running, but in real life, I don't care how many are actually running at once, just that they are kicked off one second apart.

Is this expected behaviour? If so, is there any way of making the library work the way I need it to?

Thanks!