6

I'm trying to make an asynchronous loop with native ES6 promises It kind of works, but incorrectly. I suppose I made a huge mistake somewhere and I need someone to tell me where it is and how it's done correctly

var i = 0;

//creates sample resolver
function payloadGenerator(){
    return function(resolve) {
        setTimeout(function(){
            i++;
            resolve();
        }, 300)
    }
}

// creates resolver that fulfills the promise if condition is false, otherwise rejects the promise.
// Used only for routing purpose
function controller(condition){
    return function(resolve, reject) {
        console.log('i =', i);
        condition ? reject('fin') : resolve();
    }
}

// creates resolver that ties payload and controller together
// When controller rejects its promise, main fulfills its thus exiting the loop
function main(){
    return function(resolve, reject) {
        return new Promise(payloadGenerator())
            .then(function(){
                return new Promise(controller(i>6))
            })
            .then(main(),function (err) {
                console.log(err);
                resolve(err)
            })
            .catch(function (err) {
                console.log(err , 'caught');
                resolve(err)
            })
    }
}


new Promise(main())
    .catch(function(err){
        console.log('caught', err);
    })
    .then(function(){
        console.log('exit');
        process.exit()
    });

Now the output:

/usr/local/bin/iojs test.js
i = 1
i = 2
i = 3
i = 4
i = 5
i = 6
i = 7
fin
error: [TypeError: undefined is not a function]
error: [TypeError: undefined is not a function]
error: [TypeError: undefined is not a function]
error: [TypeError: undefined is not a function]
error: [TypeError: undefined is not a function]
error: [TypeError: undefined is not a function]
error: [TypeError: undefined is not a function]
caught [TypeError: undefined is not a function]
exit

Process finished with exit code 0

The good part: it reaches the end.

The bad part: it catches some errors and I don't know why.

laurent
  • 88,262
  • 77
  • 290
  • 428
amdc
  • 400
  • 1
  • 4
  • 17
  • 1
    Regardless of libraries being used the way you're using promises is _really_ strange. What is your end goal here? You want to implement a "while" with promises? – Benjamin Gruenbaum Jan 25 '15 at 12:27
  • 1
    `.then(main(),function (err) {`. When is the `main()` doing there? – Gil Elad Jan 25 '15 at 13:11
  • You forgot to tell us what this code is supposed to do. – JLRishe Jan 25 '15 at 16:05
  • @BenjaminGruenbaum Yeah I know. I'm new to promises and trying to figure out how they work. Yes, it's a while loop; JLRishe: It's supposed to count to 7 and do not produce any errors – amdc Jan 25 '15 at 20:31

5 Answers5

7

Any helper function with promise looping I have seen actually made it much worse than what you can do out of the box with recursion.

It is a little nicer with .thenReturn but yeah:

function readFile(index) {
    return new Promise(function(resolve) {
        setTimeout(function() {
            console.log("Read file number " + (index +1));
            resolve();
        }, 500);
    });
}

// The loop initialization
Promise.resolve(0).then(function loop(i) {
    // The loop check
    if (i < len) {              // The post iteration increment
        return readFile(i).thenReturn(i + 1).then(loop);
    }
}).then(function() {
    console.log("done");
}).catch(function(e) {
    console.log("error", e);
});

See it in jsfiddle http://jsfiddle.net/fd1wc1ra/

This is pretty much exactly equivalent to:

try {
    for (var i = 0; i < len; ++i) {
        readFile(i);
    }
    console.log("done");
} catch (e) {
    console.log("error", e);
}

If you wanted to do nested loops it is exactly the same:

http://jsfiddle.net/fd1wc1ra/1/

function printItem(item) {
    return new Promise(function(resolve) {
        setTimeout(function() {
            console.log("Item " + item);
            resolve();
        }, 500);
    });
}

var mdArray = [[1,2], [3,4], [5,6]];
Promise.resolve(0).then(function loop(i) {
    if (i < mdArray.length) {
        var array = mdArray[i];
        return Promise.resolve(0).then(function innerLoop(j) {
            if (j < array.length) {
                var item = array[j];
                return printItem(item).thenReturn(j + 1).then(innerLoop);
            }
        }).thenReturn(i + 1).then(loop);
    }
}).then(function() {
    console.log("done");
}).catch(function(e) {
    console.log("error", e);
});
Esailija
  • 138,174
  • 23
  • 272
  • 326
  • Magnificent, thank you! Nice trick with `thenReturn`. One remark: your code here is different from code on jsfiddle, I was confused at first. jsfiddle version worked for me since iojs' promise implementation doesn't have `thenReturn`. – amdc Jan 25 '15 at 20:19
  • @amdc `thenReturn` is from bluebird but can be implemented on native promises as shown in the jsifddle. – Esailija Jan 25 '15 at 20:19
  • Works very well! I want to be able to do like 5 loops at the same time and when one of those is done, go on with the 6th and so on until the array is done. Any suggestions? Checkout http://stackoverflow.com/q/36664272/1760313 – Tom Apr 16 '16 at 14:26
2

If all you're trying to do is count to 7 with promises, then this will do it:

function f(p, i) {
  return p.then(function() {
    return new Promise(function(r) { return setTimeout(r, 300); });
  })
  .then(function() { console.log(i); });
}

var p = Promise.resolve();
for (var i = 0; i < 8; i++) {
  p = f(p, i);
}
p.then(function() { console.log('fin'); })
 .catch(function(e) { console.log(e.message); });

Looping with promises is hard, because it's almost impossible not to fall into JavaScript's closures in a loop trap, but it is doable. The above works because it pushes all use of .then() into a sub-function f of the loop (i.e. away from the loop).

A safer solution, that I use, is to forgo loops altogether and seek out patterns like forEach and reduce whenever I can, because they effectively force the sub-function on you:

[0,1,2,3,4,5,6,7].reduce(f, Promise.resolve())
.then(function() { console.log('fin'); })
.catch(function(e) { console.log(e.message); });

here f is the same function as above. Try it.

Update: In ES6 you can also use for (let i = 0; i < 8; i++) to avoid the "closures in a loop" trap without pushing code into a sub-function f.

PS: The mistake in your example is .then(main(), - it needs to be .then(function() { return new Promise(main()); }, but really, I think you're using the pattern wrong. main() should return a promise, not be wrapped by one.

Community
  • 1
  • 1
jib
  • 40,579
  • 17
  • 100
  • 158
  • This is a for loop - I think OP was going for a `while` which would not work with thenable chaining since you would not know how much chaining you need in advance. – Benjamin Gruenbaum Jan 26 '15 at 08:02
  • Maybe the OP meant that the condition depended on the previous steps having executed? If so, then yes recursion would be needed. In any case, using a while-loop instead of for-loop, doesn't solve this, or mean that. I've changed the for-loop to a while-loop to show that. The OP said in a comment he was trying to count to 7 with promises, and asked someone to point out his mistake, which I hope I did. – jib Jan 26 '15 at 14:19
  • Of course changing the for loop to another synchronous looping construct wouldn't help - I meant that conceptually the number of iterations depends on previous state. – Benjamin Gruenbaum Jan 26 '15 at 14:22
  • @BenjaminGruenbaum for loops are fine too. it's always good to know how to make it in different ways, thanks for contribution. – amdc Jan 26 '15 at 15:47
0

Try logging err.stack instead of just err when catching promise errors.

In this case, it looks like resolve and reject are not defined within the anonymous function that gets return from main after the initial iteration is complete. I can't totally follow your control flow, but that seems to make sense - after the 7 iterations are complete, there should no longer be any new promises. However, it seems like the code is still trying to run like there are more promises to resolve.

Edit: This is the problem .then(main(),function (err) {. Invoking main on its own will cause resolve and reject inside the anonymous function to be undefined. From the way I read it, main can only be invoked as an argument to the Promise constructor.

tydotg
  • 631
  • 4
  • 11
0

I've looked around for various solutions too and couldn't find one that satisfied me, so I ended up creating my own. Here it is in case it's useful to someone else:

The idea is to create an array of promise generators and to give this array to a helper function that's going to execute the promises one after another.

In my case the helper function is simply this:

function promiseChain(chain) {
    let output = new Promise((resolve, reject) => { resolve(); });
    for (let i = 0; i < chain.length; i++) {
        let f = chain[i];
        output = output.then(f);
    }
    return output;
}

Then, for example, to load multiple URLs one after another, the code would be like this:

// First build the array of promise generators:

let urls = [......];
let chain = [];
for (let i = 0; i < urls.length; i++) {
    chain.push(() => {
        return fetch(urls[i]);
    });
}

// Then execute the promises one after another:

promiseChain(chain).then(() => {
    console.info('All done');
});

The advantage of this approach is that it creates code that's relatively close to a regular for loop, and with minimal indentation.

laurent
  • 88,262
  • 77
  • 290
  • 428
0

I had a similar need and tried the accepted answer, but I was having a problem with the order of operations. Promise.all is the solution.

function work(context) {
  return new Promise((resolve, reject) => {
    operation(context)
      .then(result => resolve(result)
      .catch(err => reject(err));
  });
}

Promise
  .all(arrayOfContext.map(context => work(context)))
  .then(results => console.log(results))
  .catch(err => console.error(err));
jonpeck
  • 151
  • 4