nodejs - Returning a Promise after the end of a for loop

Question

I need to call an async function in a for loop where every iteration 'builds' an object. At the end of the loop, this object should be resolved in a Promise so that the next function in chain can pick it up for further processing. While the iterations get the data correctly, I am not able to collect the 'built up' object and return it. I keep getting {}. Can you please help me resolve this?

async function imageDetailsCache(images) {
    var imageData = {}; // This object is built up every iteration
    var imageNo = '';
    var promises = [];
    //
    for (let i = 0; i < images.length; i++) {
        imageNo = images[i]
        promises.push(
            new Promise(function (resolve, reject) {
                imageDetails(imageNo, 'BIG').then(function (result) {
                    imageData[imageNo] = { 'BIG': result }
                    imageDetails(imageNo, 'TINY').then(function (result) {
                        imageData[imageNo] = { 'TINY': result }
                    })
                })
                resolve(imageData)
            })
        )
    }
    Promise.all(promises).then(function (result) {
        return result; // Always {}
    })
}

`return result; // Always {}` - that's unlikely because the result of `Promise.all` is an array. — Estus Flask, Oct 20 '18 at 13:15

Bergi · Accepted Answer · 2018-10-21T09:27:16.233

2

Avoid the Promise constructor antipattern! You were calling resolve before the asynchronous things in imageDetails had happened. And don't use then when working with async/await:

async function imageDetailsCache(images) {
    var imageData = {}; // This object is built up every iteration
    var promises = images.map(async (image) => {
        var result = await imageDetails(image, 'BIG');
        imageData[image] = { 'BIG': result }
        var result = await imageDetails(image, 'TINY');
        imageData[image] = { 'TINY': result };
    });
    await Promise.all(promises);
    return imageData;
}

edited Oct 21 '18 at 09:27

answered Oct 20 '18 at 13:47

Bergi

630,263
148
957
1,375

Shouldn't the async function passed to map return a promise? – danh Oct 20 '18 at 20:47
@danh It does, because it's an `async` function. The returned promise does fulfill when the body with all its `await`ed promises finishes. – Bergi Oct 20 '18 at 21:08
1

@cogitoergosum Right, thanks, there isn't even any `result` in my code – Bergi Oct 21 '18 at 09:27
I tried a variation of your example - https://pastebin.com/PVevygSm But, `data` keeps turning out to be empty. – cogitoergosum Oct 21 '18 at 10:12
So the function that calls `imageDetailsCache(images)` keeps getting a `Promise { }` not the `imageData` object. – cogitoergosum Oct 21 '18 at 10:58
@cogitoergosum In your pastebin, the `async function` that you pass as a callback to `map` doesn't `return` anything to resolve the promise with. Do not mix `then` with `await` syntax. – Bergi Oct 21 '18 at 11:11
@cogitoergosum Yes, `imageDetailsCache` returns a promise for the `imageData`. You cannot get the asynchronous result immediately. – Bergi Oct 21 '18 at 11:11
I was using `await` and `then` because `imageCacheData` returns a resolved Promise. I don't see a `return` in your example above ... – cogitoergosum Oct 21 '18 at 11:17
@cogitoergosum I don't `return` anything because I ignore the results of `Promise.all`, I only wait for it. In your pastebin code you are doing something with the `data` array and wonder why it is empty - it contains the results of the `promises` which all are `undefined`. And you should use *either* `await` *or* `then` to deal with promises, not both. – Bergi Oct 21 '18 at 11:22

ancag · Answer 2 · 2018-10-21T16:33:14.620

If you want to use Promises, then: I guess this will work:

async function imageDetailsCache(images) {
var imageData = {};
var promises = [];
for(let i = 0; i < images.length; i++) { // notice the let instead of var
   promises.push(
         new Promise( (resolve, reject) => {
            imageDetails(images[i], 'BIG')
                .catch(e => {reject(e)})
                .then( rBig => {
                    imageDetails(images[i], 'SMALL')
                        .catch(e => { reject(e)})
                        .then( rSmall => {
                            imageData[images[i]] = {
                                 'BIG': rBig ,
                                 'SMALL': rSmall
                            }
                            resolve ({
                                BIG: rBig,
                                SMALL: rSmall
                            });
                        });
                    });
                }
            )
        );
}
await Promise.all(promises);
return imageData;

}

imageDetailsCache(images).then((r) => {console.log(r)});

I do have to emphasize that using async & await properly (as https://stackoverflow.com/users/1048572/bergi suggested) it will make this code cleaner .

Personally I would do it like this:

async function imageDetailsCache(images) {
var imageData = {};
var promises = [];
for(let i = 0; i < images.length; i++) {
    try {
        imageData[images[i]] = {
            'BIG': await imageDetails(images[i], 'BIG') ,
            'SMALL': await imageDetails(images[i], 'SMALL')
        }
    } catch (e)  {
        console.log(e);
    }
}
return imageData;

}

imageDetailsCache(images).then((r) => {console.log(r)});

Hope it helps!

Avoid the [`Promise` constructor antipattern](https://stackoverflow.com/q/23803743/1048572?What-is-the-promise-construction-antipattern-and-how-to-avoid-it)! — Bergi, Oct 21 '18 at 09:28
I was just trying to show cogitoergosum how to fix his code while pointing at the good practices you wrote earlier. I do agree with what you wrote, that is why I marked your answer as right. — ancag, Oct 21 '18 at 09:32
So `var d = imageDetailsCache(images)` should make `d` have the complete `imageData`? — cogitoergosum, Oct 21 '18 at 10:39
@ancag It's not just "cleaner". Using the `new Promise` constructor like you did will prevent errors from being handled. Just use `then` chaining instead. — Bergi, Oct 21 '18 at 11:08
@cogitoergosum I modified my answer with two example. Hope it helps. — ancag, Oct 21 '18 at 16:37
@bergi you are right :) the function should be async, because it has to wait until all the promises are ready also in the previous comment I did not .catch on errors. Thanks for the heads up! — ancag, Oct 21 '18 at 16:37

nodejs - Returning a Promise after the end of a for loop

2 Answers2