jessetane / queue

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

Return Promise from queue.start() #34

Closed slavafomin closed 1 year ago

slavafomin commented 7 years ago

Hello!

Thank you for this great module! It's very convenient.

However, it would be great to add support for Promises in queue.start().

Right now I have to wrap it like this:

function startQueue (queue) {
  return new Promise((resolve, reject) => {
    queue.start(error => {
      if (error) {
        reject(error);
      } else {
        resolve();
      }
    });
  });
}

And then:

const queue = $queue();
queue.push(…);
await startQueue(queue);
// queue is finished here

The better would be:

const queue = $queue();
queue.push(…);
await queue.start();
// queue is finished here
jessetane commented 7 years ago

@slavafomin feel free to send a PR

jessetane commented 6 years ago

@slavafomin @kwolfy anyone interested in working on a patch for this?

sempasha commented 6 years ago

Before returning Promise there is one ambiguous thing should be resolved. Starting queue with callback and without it has different errors handling behaviour:

  1. In first case queue.start(callback), when error occurs queue will trigger error event, and stops execution;
  2. While in second case queue.start(), when error occurs queue will just trigger error event and nothing more.

In my view there should be more transparent way to determine, what queue should do when error occurs - just trigger error event or stops execution next jobs with error event.

jessetane commented 6 years ago

@sempasha good point - I'm open to changes that break the API if we all agree they improve the design

MaksimLavrenyuk commented 1 year ago

@slavafomin Resolve https://github.com/jessetane/queue/pull/93