Order of promise wrong

Question

I would like to return a list of promises with Promise.all, but the result is always empty.

Here is my theoric code:

function myFunction() {

    const pSqlLines = sql.openDatabase().then(mdb => {
        const query = `SELECT data FROM database`
        return mdb.prepare(query).all()
    })

    
    const globalPromise = new Promise( resolve => {

        const promises = []
        pSqlLines.then( sqlLines => {

            sqlLines.forEach(line => {
                promiseA()
                .then(res1 => { if (res1 == 1) return promiseB() }) 
                .then(res2 => { if (res2 == 1) return promiseC() }) 
                .then(res3 => { if (res3 == 1) promises.push( new Promise(resolve => resolve(line) ) ) })                
            }) // The promises chaining is correct as the 'promises' array is correctly fulfilled, but later

            resolve( Promise.all(promises) ); // It is always called before the foreach loop. Why?
        })
    })

    return globalPromise.then( result => console.log(result) )
}

Someone can help me please? Thanks a lot.

Use `async` and `await`, otherwise it will always execute first. — , Feb 01 '21 at 18:41
What is the sense of `if (res1 == 1)`? What if this is false, what do you expect the changed `then` callback to work with? — trincot, Feb 01 '21 at 18:42
`Promise.all(promises)` will always use an empty array for `promises`, because nothing makes the code wait for the array to be filled in. — VLAZ, Feb 01 '21 at 18:53

trincot · Accepted Answer · 2021-02-01T19:00:10.430

You should not need to create a new promise with new Promise when you already have a promise to work with -- in your case: pSqlLines. Wrapping pSqlLines inside a new Promise wrapper is an example of the promise constructor antipattern.

Secondly, this code:

 new Promise(resolve => resolve(line) )

...can be replaced with:

 Promise.resolve(line)

...and since it is to serve as the return value for a then callback, it can be just:

 line

As to your question: you are indeed calling Promise.all(promises) at a moment that the promises array is still empty, as the forEach loop has only executed Promise() and its then chain, but none of the asynchronous callbacks that are passed to those then calls have executed yet. You didn't await the resolution of these promises.

I get from your code that you want to exclude some line values depending on conditions. In that case the promise will resolve to undefined. I suppose you would want to exclude those undefined values, and so maybe a filter(Boolean) is appropriate.

Here is a theoretical solution for your theoretical code:

function myFunction() {
    const pSqlLines = sql.openDatabase().then(mdb => {
        const query = `SELECT data FROM database`;
        return mdb.prepare(query).all();
    });
    
    return pSqlLines.then( sqlLines => {
        const promises = sqlLines.map(line => {
            return promiseA()
                .then(res1 => { if (res1 == 1) return promiseB(); }) 
                .then(res2 => { if (res2 == 1) return promiseC(); }) 
                .then(res3 => { if (res3 == 1) return line; });
        }));
        return Promise.all(promises);
    }).then(result => result.filter(Boolean));
}

I suspect that `== 1` might not even be needed but I don't know what it actually checks for. At the very least, maybe there is a different way to verify things that is more descriptive. — VLAZ, Feb 01 '21 at 19:06
I asked the OP for clarification 5 minutes after they posted, but no reaction so far. I guess this is part of what they call "theoretic code". — trincot, Feb 01 '21 at 19:11
@trincot: Sorry for the late answer as I am trying your improvement code and it works perfectly! It seems very simple as you write :) The filter function will reduce others code I wrote. Thank you a lot for the tip. — Joker, Feb 01 '21 at 20:27

Order of promise wrong

1 Answers1