Wait for the result of forEach before sending callback to main

Question

I want to collect the data with its child (another item which is bound to that product ID) I have a collection with this schema

// User Schema
const filtersSchema = mongoose.Schema({
    filter_name:{
        type: String,
        required: true
    },
    filter_code:{
        type: String,
        required: true,
        unique: true
    },
    bind_to: {
        type: Schema.Types.ObjectId,
        default: null
    },
    filter_status:{
        type: Boolean,
        default: true
    },
    created_on:{
        type: Date,
        default: Date.now
    },
    updated_on:{
        type: Date,
        default: Date.now
    }
});

If I enter a data to it, default bind_to value will be null, that means its parent. If I send the bind_to ID of a parent, it'll be ObjectID.

I want to collect data like this

[{
-- parent object --
children:[
  {
  -- child object --
  },
  {
  -- child object --
  }
]
}]

if we have more than one item it'll go through loop (forEach) but the callback is getting sent before the forEach loop getting finished. I know forEach is asynchronous and the request is synchronous. but confused about how to do that!

you can see the module below

// Get Filters by Parent ID
module.exports.getFiltersByParentId = (pid, callback) => {
    Filters.find({bind_to: pid}, callback);
}

//For getting the parent object and looping it to get its child objects
module.exports.getFilters = (callback, limit) => {
    Filters.find({bind_to: null}, (err, filters) => {
        if (err) {
            console.log(err);
            let obj = {status: false, error: err.errmsg};
            callback(obj);
        } else {
            const resObj = [];
            filters.forEach(async function (ele) {
                await Filters.getFiltersByParentId(ele._id, (err, cfil) => {
                    if (err) {
                        let obj = {status: false, message: err.errmsg};
                        callback(obj);
                    } else {
                        console.log(ele, "Obj");
                        ele.children = cfil;
                        resObj.push(ele);
                    }
                });
            });
            Promise.all(resObj).then(res => {
                let obj = {status: true, data: res, message: "Filters found"};
                callback(obj);
            });
        }
    });
}

but in this case result object will be empty. how can I get the proper object with the values as mentioned above?

Even I tried with this method

const resObj = [];
            filters.map(function (ele) {
                Filters.getFiltersByParentId(ele._id, (err, cfil) => {
                    if (err) {
                        let obj = {status: false, message: err.errmsg};
                        callback(obj);
                    } else {
                        console.log(ele, "Obj");
                        ele.children = cfil;
                        resObj.push(ele);
                    }
                });
            });
            Promise.all(resObj).then(res => {
                let obj = {status: true, data: res, message: "Filters found"};
                callback(obj);
            });

And This

Promise.all(filters.map(function (ele) {
                Filters.getFiltersByParentId(ele._id, (err, cfil) => {
                    if (err) {
                        let obj = {status: false, message: err.errmsg};
                        callback(obj);
                    } else {
                        console.log(ele, "Obj");
                        ele.children = cfil;
                        resObj.push(ele);
                    }
                });
            })).then(res => {
                let obj = {status: true, data: res, message: "Filters found"};
                callback(obj);
            });

Ok, Now im returning a promise from getFiltersByParentId

module.exports.getFiltersByParentId = (pid, callback) => {
    return new Promise(function(resolve, reject) {
        Filters.find({bind_to: pid}, function (err, results) {
            if (err) {
                reject(err);
            } else {
                resolve(results);
            }
        })
    });
}

Possible duplicate of [Using async/await with a forEach loop](https://stackoverflow.com/questions/37576685/using-async-await-with-a-foreach-loop) — CertainPerformance, Mar 15 '19 at 07:08
`I know forEach is asynchronous and the request is synchronous` It's the other way around. — CertainPerformance, Mar 15 '19 at 07:08
That question didn't help me! I checked that answer before asking here, there they are using a const array, It works fine with an array or even without that method call inside the forEach. but when I add that method it's not working. its sending empty array for the result of callback object! — rakcode, Mar 15 '19 at 07:48
You're trying to `await` inside a `forEach` - the `forEach` line will finish before the async operations finish, so `resObj` is empty. Use `.map` instead — CertainPerformance, Mar 15 '19 at 07:49
I've tried with that, I updated the question with the method used, can you check it once!! — rakcode, Mar 15 '19 at 07:54
You need to return a Promise, right now the mapped array is just `undefined`s, look up how to convert a callback API to promises — CertainPerformance, Mar 15 '19 at 07:56
should `callback(obj)` in the `if (err) {` block terminate the processing, or is `callback` expecting to be called multiple times in case of errors and success — Jaromanda X, Mar 15 '19 at 07:57
also, does each iteration need to wait for the previous iteration to complete (i.e. `Filters.getFiltersByParentId` called in series) or should each `Filters.getFiltersByParentId` be run concurrently (i.e. parallel)? — Jaromanda X, Mar 15 '19 at 07:59
yes, it should terminate if there is an error and `Filters.getFiltersByParentId` takes bind_to ID, we get it with the `ele` object which is parent object. with that parent ID we need to fetch the children of that parent! — rakcode, Mar 15 '19 at 08:00
so, callback will be called exactly once and once only ... as to my second comment ... series or parallel — Jaromanda X, Mar 15 '19 at 08:02
series. i just need to get all the parent data with its children. and return one array of object with all the parents — rakcode, Mar 15 '19 at 08:05

Jaromanda X · Answer 1 · 2019-03-15T10:54:13.717

Firstly, since Filters.getFiltersByParentId doesn't return a promise, there is no sense in awaiting the response - so, I wrapped it in a new Promise - if I had time there's probably a simpler way to do this without bothering with async/await, since there's actually no promises at all in the code except for the promise no introduced to make use of async/await

Still, I believe the code is simpler to read this way, so, lets keep with the async/await and promise theme

Secondly, using for ... of loop makes the code very simple, especially since you want any error to cease further calls to Filters.getFiltersByParentId

code as follows

module.exports.getFilters = (callback, limit) => {

    Filters.find({bind_to: null}, async (err, filters) => {
        if (err) {
            console.log(err);
            let obj = {status: false, error: err.errmsg};
            callback(obj);
        } else {
            const resObj = [];
            for (const ele of filters) {
                try {
                    let result = await new Promise((resolve, reject) => {
                        Filters.getFiltersByParentId(ele._id, (err, cfil) => {
                            if (err) {
                                let obj = {status: false, message: err.errmsg};
                                reject(obj);
                            } else {
                                console.log(ele, "Obj");
                                ele.children = cfil;
                                resolve(ele);
                            }
                        });
                    });
                    resObj.push(result);
                } catch(e) {
                    return callback(obj);
                }
            }
            let obj = {status: true, data: resObj, message: "Filters found"};
            callback(obj);
        }
    });
};

edit: I had time :p

Here is the code without async/await, since there are no promises

module.exports.getFilters = (callback, limit) => {
    Filters.find({bind_to: null}, (err, filters) => {
        if (err) {
            console.log(err);
            let obj = {status: false, error: err.errmsg};
            callback(obj);
        } else {
            const resObj = [];
            const getFilters = (index) => {
                if (index < filters.length) {
                    const ele = filters[index];
                    Filters.getFiltersByParentId(ele._id, (err, cfil) => {
                        if (err) {
                            let obj = {status: false, message: err.errmsg};
                            callback(obj);
                            return;
                        } else {
                            console.log(ele, "Obj");
                            ele.children = cfil;
                            getFilters(index + 1);
                        }
                    });
                } else {
                    let obj = {status: true, data: resObj, message: "Filters found"};
                    callback(obj);
                }
            };
            getFilters(0);
        }
    });
};

This I've tried just now, this won't respond only! here result will always be `undefined`. — rakcode, Mar 15 '19 at 10:08
Inside the method `Filters.getFiltersByParentId`, i've logged a object named 'Obj', there object is perfect. its fetching the data — rakcode, Mar 15 '19 at 10:11
I assumed `Filters.getFiltersByParentId` returned a promise because of your original code - I see now your original code was fundamentally flawed if `Filters.getFiltersByParentId` *does not return a promise* - I'd actually be surprised if it did now that I look at the fact that it takes a callback argument! — Jaromanda X, Mar 15 '19 at 10:21
@rakcode - I've added an even better answer, since there are no Promises in your original code, only the ones you thought were there! — Jaromanda X, Mar 15 '19 at 10:54
if there is a flaw in my code, how can I improve it? I'm new to nodeJS :p please help me to get better at it! thanks — rakcode, Mar 15 '19 at 11:00
do you read ALL the comments I made before you reply to the first one? @rakcode — Jaromanda X, Mar 15 '19 at 11:12
Let us [continue this discussion in chat](https://chat.stackoverflow.com/rooms/190086/discussion-between-rakcode-and-jaromanda-x). — rakcode, Mar 15 '19 at 11:13

score 0 · Answer 2 · answered Mar 15 '19 at 11:51

After making some changes to my question and using @Jaromanda X's & @CertainPerformance guide I came up with a solution for my question. it's not exactly as I wanted, but it's quite the same output.

I changed gFilters.getFiltersByParentId method to return Promise

module.exports.getFiltersByParentId = (pid, callback) => {
    return new Promise(function(resolve, reject) {
        Filters.find({bind_to: pid._id}, function (err, results) {
            if (err) {
                reject(err);
            } else {
                let obj = {parent: pid, children: results};
                resolve(obj);
            }
        })
    });
}

Then changed forEach to array.map and used promise array in Promise.all function

module.exports.getFilters = (callback, limit) => {
    Filters.find({bind_to: null}, (err, filters) => {
        if (err) {
            console.log(err);
            let obj = {status: false, error: err.errmsg};
            callback(obj);
        } else {
            const resObj = [];
            var promises = [];

            filters.map(function (ele) {
                promises.push(Filters.getFiltersByParentId(ele, (err, cfil) => {
                    if (err) {
                        let obj = {status: false, message: err.errmsg};
                        callback(obj);
                    } else {
                        console.log(ele, "Obj");
                        ele.children = cfil;
                        resObj.push(ele);
                    }
                }));
            });

            Promise.all(promises).then(res => {
                let obj = {status: true, data: res, message: "Filters found"};
                callback(obj);
            });

        }
    });
}

Finally, my output looks something like this

[
 {
  parent: {
   --- parent object ---
  },
  children: [
    {
     --- child object 1 ---
    },
    {
     --- child object 2 ---
    },
    {
     --- child object n ---
    }
  ]
 }
]

Wait for the result of forEach before sending callback to main

2 Answers2