Executing and delaying array of promises in series

Question

I'm trying to execute a series of functions synchronousl usingy. Each function is supposed to be delayed by 3 seconds before invoking the next one. I must be doing something wrong because they are all invoked at the same time after 3 seconds instead of in order.

What am I doing wrong?

var tasks = []
    allGroups.forEach(function(group){
      tasks.push(deleteFromGroup(group))
    })

    tasks.reduce(function(cur, next) {
      return cur.then(next);
    }, Promise.resolve()).then(function() {
     console.log("all executed")
    });
  })
}

function deleteFromGroup(group){
  return new Promise(function(resolve, reject) {
    setTimeout(function(){
      console.log(group.id)
      resolve()
     }, 3000);
  })
}

why have you pasted the answer code into your question? You shouldn't do that because it destroys context. — Alnitak, Nov 13 '15 at 13:38
Your code initially didn't work as well and I wanted to ask you why. I found out it was due to my own mistake. I rolled back the question. — Hedge, Nov 13 '15 at 13:41
thanks. Please - never destroy parts of a question that have already had answers posted. — Alnitak, Nov 13 '15 at 13:44
I won't. Sadly the almighty mods came and marked a percetly fine question as a duplicate... — Hedge, Nov 13 '15 at 15:21
@Alnitak: That shouldn't matter as it's really exactly the same problem and solution. I can edit the "Q" out of the title if you want :-) — Bergi, Nov 13 '15 at 15:55
@Bergi it's _functionally_ the same problem, but the APIs _are_ different. — Alnitak, Nov 13 '15 at 15:57
@Alnitak and/or Hedge - Just edit the question to include info that makes it clearly unique from the supposed duplicate, and start the re-open cycle. If it's sufficiently differentiated it will be reopened. If not, it remains as a signpost for future users, and is still useful. — Mogsdad, Nov 13 '15 at 21:39

Alnitak · Accepted Answer · 2015-11-13T14:16:52.557

The way you're creating your tasks array inevitably results in the timeouts all hitting at (about) the same time, because you're creating the tasks simultaneously in that very first .forEach loop.

To achieve the effect you require you need to actually not create the next task until the current one is resolved. Here's a pseudo-recursive way of achieving that:

return new Promise(resolve, reject) {
    var groups = allGroups.slice(0);  // clone
    (function loop() {
        if (groups.length) {
            deleteFromGroup(groups.shift()).catch(reject).then(loop);
        } else {
            console.log("all executed")
            resolve();
        }
    })();
});

p.s. in practise you might actually want to incorporate the 3s timeout directly into the loop, instead of into deleteFromGroup - as written the code above (and your original code) won't show "completed" until 3s after the final delete call, but I expect it's really supposed to occur immediately at the end.

Wow, this works amazingly well. Thanks a lot. – Hedge Nov 13 '15 at 13:32 — Hedge, Nov 13 '15 at 13:32

Benjamin Gruenbaum · Answer 2 · 2015-11-13T14:23:59.077

2

You do not need to resort to callbacks and explicit construction here. You can in fact use a for loop - but not on the actions since a promise is an already started operation.

All you need to do is merge your two loops:

allGroups.reduce(function(cur, next) {
  return cur.then(function(){ return deleteFromGroup(next) });
}, Promise.resolve()).then(function() {
 console.log("all executed")
});

edited Nov 13 '15 at 14:23

answered Nov 13 '15 at 14:13

Benjamin Gruenbaum

270,886
87
504
504

I've (probably) fixed the `.catch` case in my answer. This is kinda funky and I've not personally seen it before, and I do grok it, but I can imagine some folks struggling with this use of `.reduce` – Alnitak Nov 13 '15 at 14:18
@Alnitak that might be true, but the OP used reduce in his own code so I assume that's not a problem. – Benjamin Gruenbaum Nov 13 '15 at 14:22

Executing and delaying array of promises in series

2 Answers2