mcollina / fastq

Fast, in memory work queue
ISC License
932 stars 47 forks source link

unshift does not work as expected in a retry demo. #77

Closed aguegu closed 10 months ago

aguegu commented 10 months ago
import fastq from 'fastq';

const worker = async ({ index, retries }) => {
  console.log({ index, retries });
  throw new Error('Oops');
};

const queue = fastq.promise(worker, 1);

queue.error((err, { index, retries }) => {
  if (err) {
    if (retries) {
      queue.unshift({ index, retries: retries - 1 });
      // queue.push({ index, retries: retries - 1 });
    } else {
      console.log({ err }, 'all retries failed');
    }
  }
});

queue.push({ index: 0, retries: 3 });

await queue.drain();

I would like to implement a simple retry mechanism with fastq(1.15.0)'s error handler, like every task got 3 more chances. So for the following tries, I would prefer them to be unshifted into the queue. But the log looks like:

{ index: 0, retries: 3 }
{ index: 0, retries: 2 }

What I expected is the log like this which is replacing unshift() with push() :

{ index: 0, retries: 3 }
{ index: 0, retries: 2 }
{ index: 0, retries: 1 }
{ index: 0, retries: 0 }
{
  err: Error: Oops
      at worker (file:///home/agu/workspace/TimeFabric/livefeed/api/src/queDemo.js:5:9)
      at asyncWrapper (/home/agu/workspace/TimeFabric/livefeed/api/node_modules/fastq/queue.js:213:12)
      at Task.release (/home/agu/workspace/TimeFabric/livefeed/api/node_modules/fastq/queue.js:149:16)
      at worked (/home/agu/workspace/TimeFabric/livefeed/api/node_modules/fastq/queue.js:201:10)
} all retries failed

so the worker got executed 4 times before giving up.

I do not know why unshift only got the work executed once. But unshift is what I want in this queue, because an immediate retry may just get it done. It does not have to retry it later or wait all over again.

mcollina commented 10 months ago

You've found yourself a bug! If you would like to send a PR, it would be amazing. Otherwise, I'll take a look when I can.

aguegu commented 10 months ago

Thanks @mcollina for your encouragement. Feel proud and lucky to contribute.

catdevnull commented 10 months ago

fixed?

aguegu commented 10 months ago

Fixed with #78