queue-promise icon indicating copy to clipboard operation
queue-promise copied to clipboard

feat: return task id at enqueue and dequeue

Open manuman94 opened this issue 2 years ago • 6 comments

Hi! I needed the task ID from outside the queue to keep control of tasks sent to the queue.

This is my approach to get it.

Thanks!

manuman94 avatar Jan 05 '22 11:01 manuman94

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 48654ec5bfa4842303119e87648c2957830b0d6f:

Sandbox Source
Vanilla Configuration
queue-promise-sandbox-manual Configuration
queue-promise-sandbox-automatic Configuration

codesandbox-ci[bot] avatar Jan 05 '22 11:01 codesandbox-ci[bot]

Hey @manuman94, thanks for your contribution. Could you elaborate a little bit more on your use case and how those changes solve your problem?

Bartozzz avatar Jan 08 '22 18:01 Bartozzz

Hi!

Yes! We are currently developing a Jasmine reporter that sends information to a custom made API (spec information, http requests done, screenshots).

Before using queue-promise, this API calls were blocking the actual tests, now using queue-promise we are parallelizing those calls.

By getting the task ID from outside the queue we know which async tasks are already pending and which of them are completed. We show information about that while the process is being executed.

If one task fail, we know which task is it, and show information about what are we sending and the error returned by the server.

That's our use case, but we find useful to keep control of the status of the tasks queued in the promise queue.

Thank you, greetings.

Side note: sorry about closing pull request, Github doesn't ask for confirmation in any of their actions. I'm very sorry about the confusion. (I'm writing this in my mobile phone, where is easy to miss click buttons)

manuman94 avatar Jan 08 '22 18:01 manuman94

Hi, is there any problem with the PR? I can solve whatever thing you need.

Greetings.

manuman94 avatar Jan 18 '22 11:01 manuman94

Hey @manuman94, first of all thanks for your contribution! I am not sure if this should be a part of this repository. I would recommend you create your own ID/progress system. Maybe something similar to the code below would work for your case:

// Simulate a job
function job(shouldSuccess = true) {
  return new Promise((resolve, reject) => {
    if (shouldSuccess) {
      setTimeout(() => resolve(`Success`), 500)
    } else {
      setTimeout(() => reject(`Failure`), 500)
    }
  });
}

// Wrap the job with an ID
function executeJob(id, callback) {
  return async () => {
    try {
      const response = await callback();

      return { success: true, id, response }
    } catch (error) {
      // Maybe you can even re-throw here this object literal to keep the
      // ID and be able to handle errors in the reject event listener
      return { success: false, id, response: error }
    }
  }
}

const queue = new Queue();

queue.on("resolve", ({ success, id, response }) => {
  if (success) {
    console.debug(`Resolved task with id ${id}`);
  } else {
    console.error(`Rejected task with id ${id}`);
  }
});

queue.enqueue(executeJob('job-1', () => job(true)));
queue.enqueue(executeJob('job-2', () => job(true)));
queue.enqueue(executeJob('job-3', () => job(true)));
queue.enqueue(executeJob('job-4', () => job(false)));
Output
Resolved task with id job-1 
Resolved task with id job-2 
Resolved task with id job-3 
Rejected task with id job-4

My main concern is the inconsistent API design between the methods and the equivalent events. The enqueue method returns the enqueued tasks ID, but the information is lost when calling dequeue. One has to implement an event listener for the dequeue event and implement his own logic for matching the tasks and dequeued tasks IDs.

Another concern that I have is the fact that uniqueId is basically a counter, incrementing as you enqueue new tasks. There's no strong relation between the task and the task ID. Just changing the order of enqueue calls will change the task ID associated with the task. Because of that, as your tasks resolve, you still need to implement a mechanism to match the tasks and the tasks IDs somehow anyway.

Using the code snippet I posted above, the task ID is predictable and can be easily associated with the task itself. Let me know your thoughts on this.

Bartozzz avatar Jan 18 '22 12:01 Bartozzz

Hi @Bartozzz , thanks for your response!

I appreciate your purpose, but my thought is that the queue library should be self sufficient and keep control of the status of the async tasks from outside is something inherent to the library functions.

Maybe my approach is not the best, as I agree with all your concerns. But I think the library should offer a way to keep tracking of the promises queued.

But that's my opinion! I find your comments correct, so I understand this library is going to another way.

Thank you so much for answering and for this awesome library. Greetings.

manuman94 avatar Jan 19 '22 11:01 manuman94