jessetane / queue

Asynchronous function queue with adjustable concurrency
MIT License
764 stars 66 forks source link

How add async functions to queue? #77

Closed ErikKalkoken closed 3 years ago

ErikKalkoken commented 3 years ago

I am trying to add an async function to the queue. While the function is executed, the task appears to stay in the queue. e.g. the queue length remains at 1.

Here is some example code:

const Queue = require('queue');
const queue = new Queue({ autostart: true });

async function output() {
    console.log('hi');
}

queue.push(async (cb) => {
    await output();
    cb();
});

What am I missing?

jessetane commented 3 years ago

Have you considered using Promise.all instead? I wrote this library before promises were a thing...

See also: https://github.com/jessetane/queue/issues/34

ErikKalkoken commented 3 years ago

Thanks for the quick response and your insight. It reminded me that you can use async function also as promises. So I restructured my code accordingly and this seams to work:

queue.push((cb) => {
    output().then(cb());
});

Need to test a bit more before, but this looks like it could be a solution.

jessetane commented 3 years ago

You are passing the return value of cb (most likely undefined) to then. This probably won't do what you want. Why not just use the built in Promise.all?

try {
  const results = await Promise.all([
    output1(),
    output2()
  ])
} catch (err) {
  console.error(err)
}
ErikKalkoken commented 3 years ago

Unfortunately that did not work for me. And I frankly do not understand why this should work, since the problem is not how to get the results of multiple Promisses (I only have one), but why the task is not marked as completed.

I guess it has to do with the callback that has to be called in order to trigger next() in you library.

Anyways, I gave up on finding a solution that works with async functions and moved to p-queue that worked nicely right out of the box.

Thanks for your help!

jessetane commented 3 years ago

To make your code would work, you would have to do:

queue.push(cb => {
    output().then(() => cb());
});

You need to execute cb after then, you were executing it before.

Sounds like p-queue is designed for promises, nice find. If you only have one promise though, what is the purpose of a queue? Just curious.

ErikKalkoken commented 3 years ago

My use case is that I have a task that must only have one instance running at a time. I am using a queue with concurrency = 1 to ensure that.

jessetane commented 3 years ago

ah makes sense, your example didn't show any concurrency limit so i assumed you wanted parallel execution (which is what Promise.all is good at)