CODEWITHSUNDEEP
javascriptpromise
webnoob
webnoob

Reputation: 15934

Promises not resolving in the way I would expect

I have the following:

for (let job of jobs) {
  resets.push(
    new Promise((resolve, reject) => {
      let oldRef = job.ref
      this._sequenceService.attachRef(job).then(() => {
        this._dbService.saveDoc('job', job).then(jobRes => {
          console.log('[%s / %s]: %s', oldRef, jobRes['jobs'][0].ref, this._moment.unix(job.created).format('DD/MM/YYYY HH:mm'))
          resolve()
        }).catch(error => {
          reject(error)
        })
      }).catch(error => {
        reject(error)
      })
    })
  )
}

return Promise.all(resets).then(() => {
  console.log('Done')
})

this._sequenceService.attachRef has a console.log() call.

When this runs, I am seeing all the console logs from the this._sequenceService.attachRef() call and then I see all the logs in the saveDoc.then() call. I was expecting to see them alternate. I understand that according to this article, promises don't resolve in order but I wouldn't expect my promise to resolve until I call resolve() so would still expect alternating logs, even if not in order.

Why is this not the case?

Upvotes: 1

Views: 69

Answers (2)

jfriend00
jfriend00

Reputation: 708046

Your code can be written a lot cleaner by avoiding the promise anti-pattern of wrapping a promise in a new manually created promise. Instead, you just push the outer promise into your array and chain the inner promises to the outer one by returning them from inside the .then() handler. It can all be done as simply as this:

for (let job of jobs) {
    let oldRef = job.ref;
    resets.push(this._sequenceService.attachRef(job).then(() => {
        // chain this promise to parent promise by returning it 
        // inside the .then() handler
        return this._dbService.saveDoc('job', job).then(jobRes => {
            console.log('[%s / %s]: %s', oldRef, jobRes['jobs'][0].ref, this._moment.unix(job.created).format('DD/MM/YYYY HH:mm'));
        });
    }));
}

return Promise.all(resets).then(() => {
    console.log('Done')
}).catch(err => {
    console.log(err);
});

Rejections will automatically propagate upwards so you don't need any .catch() handlers inside the loop.

As for sequencing, here's what happens:

  • The for loop is synchronous. It just immediately runs to completion.
  • All the calls to .attachRef() are asynchronous. That means that calling them just initiates the operation and then they return and the rest of your code continues running. This is also called non-blocking.
  • All .then() handlers are asynchronous. The earliest they can run is on the next tick.
  • As such this explains why the first thing that happens is all calls to .attachRef() execute since that's what the loop does. It immediately just calls all the .attachRef() methods. Since they just start their operation and then immediately return, the for loop finishes it's work pretty quickly of launching all the .attachRef() operations.
  • Then, as each .attachRef() finishes, it will trigger the corresponding .saveDoc() to be called.
  • The relative timing between when .saveDoc() calls finish is just a race depending upon when they got started (how long their .attachRef() that came before them took) and how long their own .saveDoc() call took to execute. This relative timing of all these is likely not entirely predictable, particularly if there's a multi-threaded database behind the scenes that can process multiple requests at the same time.

The fact that the relative timing is not predictable should not be a surprise. You're running multiple two-stage async operations purposely in parallel which means you don't care what order they run or complete in. They are all racing each other. If they all took exactly the same time to execute with no variation, then they'd probably finish in the same order they were started, but any slight variation in execution time could certainly change the completion order. If the underlying DB gets has lock contention between all the different requests in flight at the same time, that can drastically change the timing too.

So, this code is designed to do things in parallel and notify you when they are all done. Somewhat by definition, that means that you aren't concerned about controlling the precise order things run or complete in, only when they are all done.

Upvotes: 3

Chirag Ravindra
Chirag Ravindra

Reputation: 4830

Clarifying my comments above:

In this case the promises are at four levels.

Level 0: Promise created with Promise.all

Level 1: Promise created with new Promise for each Job

Level 2: Promise as generated by this._sequenceService.attachRef(job)

Level 3: Promise as generated by this._dbService.saveDoc('job', job)

Let's say we have two jobs J1 and J2

One possible order of execution:

L0 invoked

J1-L1 invoked
J2-L1 invoked

J1-L2 invoked
J2-L2 invoked

J1-L2 resolves, log seen at L2 for J1
J1-L3 invoked

J2-L2 resolves, log seen at L2 for J2
J2-L3 invoked

J1-L3 resolves, log seen at L3 for J1
J1-L1 resolves

J2-L3 resolves, log seen at L3 for J2
J2-L1 resolves

L0 resolves, 'Done' is logged

Which is probably why you see all L2 logs, then all L3 logs and then finally the Promise.all log

Upvotes: 2

Related Questions