CODEWITHSUNDEEP
node.jspromisebluebird
kurt343
kurt343

Reputation: 165

What is a sensible way to structure my control flow (promises and looping)?

I'm not sure of how to adequately achieve my desired control flow using promises/bluebird.

Essentially I have a database with X 'tasks' stored and each needs to be loaded and executed sequentially. I don't want to run more than one task concurrently and the entire code must continue executing indefinitely.

I have achieved this with the following code so far:

export default function syncLoop() {
  getNextTaskRunner().then((taskRunner) => {
    if (taskRunner) {
      taskRunner.startTask()
      .then(syncLoop)
      .catch((error) => {
        throw new Error(error);
      });
    } else {
      syncLoop();
    }
  });
}

getNextTaskRunner() simply loads and resolves with the next task from the database (calc'd based on timestamps). Or it resolves with null (no task avail).

taskRunner.startTask() resolves with null when the full task has completed.

I've been advised that the way it is structured (recursive /w promises) could lead to stack issues after it has been running for some time.

What I've thought about doing is to restructure it to something like:

  let running = false;
  setInterval(() => {
    if (!running) {
      running = true;
      getNextTaskRunner().then((taskRunner) => {
        if (taskRunner) {
          taskRunner.startTask()
          .then(() => {
            running = false;
          })
          .catch((error) => {
            log.error(error);
          });
        } else {
          running = false;
        }
      });
    }
  }, 5000);

Or as yet another possibility, using event emitters in some form?

task.on('complete', nextTask());

Thoughts and advice will be greatly appreciated!

Upvotes: 0

Views: 44

Answers (1)

freakish
freakish

Reputation: 56477

What stack issues? The way you've written your code is perfectly fine as long as getNextTaskRunner is truly async (i.e. it gives control back to the main loop at some point, e.g. if it does async io). There is no recursion in your code in that case. Whoever told you that is mistaken.

Though you might want to add a setTimeout somewhere so you won't flood your db with requests. Plus it will help you if getNextTaskRunner will no longer be sync (due to for example in memory caching):

export default function syncLoop() {
  setTimeout(() => {
    getNextTaskRunner().then((taskRunner) => {
      if (taskRunner) {
        taskRunner.startTask()
          .then(syncLoop)
          .catch((error) => {
            throw new Error(error);
          });
      } else {
        syncLoop();
      }
    });
  }, 2000);
}

Upvotes: 1

Related Questions