.then of Promise.all not executing

Question

I'm using firestore to retrieve data which has the following DS.
I have a Company collection which contains a subcollection Branches
So I'm trying to retrieve to list all the Companies with its Branches

Code:

exports.findAll = function (req, res) {
    getcompanies().
    then((companies) => {
        console.log("Main "+ companies) // info: Main TypeError: Cannot read property 'Symbol(Symbol.iterator)' of undefined
        return res.json(companies);
    })
    .catch((err) => {
        console.log('Error getting documents', err);
    });
}

function getCompanies(){
    var companiesRef = db.collection('companies');

    return companiesRef.get()
    .then((snapshot) => {
        let companies = [];
        return Promise.all(
            snapshot.forEach(doc => {  
                    let company = {};                
                    company.id = doc.id;
                    company.company = doc.data(); 
                    var branchesPromise = getBranchesForCompanyById(company.id);
                    return branchesPromise.then((branches) => {                    
                            company.branches = branches;
                            companies.push(company); 
                            if(snapshot.size === companies.length){
                                console.log("companies - Inside" + JSON.stringify(companies)); //This prints all companies with its branches
                            }
                            return Promise.resolve(companies);
                        })
                        .catch(err => {
                            console.log("Error getting sub-collection documents", err);
                            return Promise.reject(err);
                        }) 
            })
        )
        .then(companies => {
            console.log("Outside " + companies) // This is never executed 
            return companies;
        })
        .catch(err => {
            return err;
        });

    })
    .catch(err => {
        return err;
    });
}

function getBranchesForCompanyById(id){
    var branchesRef = db.collection('companies').doc(id).collection('branches');
    let branches = [];
    return branchesRef.get()
     .then(snapshot => {
        snapshot.forEach(brnch => {
            let branch = {};
            branch.id = brnch.id;
            branch.branch = brnch.data();
            branches.push(branch);
        })
        return branches;
    })
    .catch(err => {
        return err;
    })

 }

I've all the data needed at this point.

console.log("companies - Inside" + JSON.stringify(companies)); //This prints all companies with its branches

But the then of Promise.all is never executed. So getting this error -

info: Main TypeError: Cannot read property 'Symbol(Symbol.iterator)' of undefined

console.log("Main "+ companies) // info: Main TypeError: Cannot read property 'Symbol(Symbol.iterator)' of undefined

I feel I have followed all the rules specified here: https://stackoverflow.com/a/31414472/2114024 with respect to nested promises, not sure where I'm missing the point.
Thanks in advance!

Answer 1

I see at least 2 problems:

• forEach likely doesn't return anything, and you send the result of forEach into Promise.all().
• If Promise.all() throws an exception, some of your catch handlers just grab the error and return it. Returning it turns it into a non-exception.

You also really don't have to add a catch to every Promise chain, as long as you feed the result of a Promise chain back into another promise chain, you probably only need 1 catch block.

Also one of your then() functions should not be nested as deeply. Just move it a level up, that's the point of promises.

Answer 2

In your code, you can use map instead of forEach. Promise.all accept an array of promises but forEach does not return an array

return Promise.all(
    snapshot.map(doc => {
        let company = {};
        company.id = doc.id;
        company.company = doc.data();
        var branchesPromise = getBranchesForCompanyById(company.id);
        return branchesPromise.then((branches) => {
                company.branches = branches;
                companies.push(company);
                if (snapshot.size === companies.length) {
                    console.log("companies - Inside" + JSON.stringify(companies)); //This prints all companies with its branches
                }
                return Promise.resolve(companies);
            })
            .catch(err => {
                console.log("Error getting sub-collection documents", err);
                return Promise.reject(err);
            })
    })
)

Answer 3

Based on inputs from Evert and Rahul, thanks to both of you, I have resolved the problem here.

1. I handled all the error in the catch block
2. Promise.all was not returning anything so I converted the forEach to map.

So this is my updated code, which solves the problem:

exports.findAll = function (req, res) {
    getcompanies().
        then((companies) => {
            console.log("Main " + companies) // Prints all companies with its branches
            return res.json(companies);
        })
        .catch((err) => {
            console.log('Error getting documents', err);
            return res.status(500).json({ message: "Error getting the all companies" + err });
        });
}

function getCompanies() {
    var companiesRef = db.collection('companies');

    return companiesRef.get()
        .then((snapshot) => {
            let companies = [];
            return Promise.all(
                snapshot.docs.map(doc => {
                    let company = {};
                    company.id = doc.id;
                    company.company = doc.data();
                    var branchesPromise = getBranchesForCompanyById(company.id);
                    return branchesPromise.then((branches) => {
                        company.branches = branches;
                        companies.push(company);
                        if (snapshot.size === companies.length) {
                            console.log("companies - Inside" + JSON.stringify(companies));
                            return companies;
                        }
                    })
                        .catch(err => {
                            console.log("Error getting sub-collection documents", err);
                            throw new Error(err);
                        })
                })
            )
                .then(companies => {
                    console.log("Outside " + companies); // Executed now
                    return companies[companies.length - 1];
                })
                .catch(err => {
                    throw new Error(err);
                });

        })
        .catch(err => {
            throw new Error(err);
        });
}

function getBranchesForCompanyById(id) {
    var branchesRef = db.collection('companies').doc(id).collection('branches');
    let branches = [];
    return branchesRef.get()
        .then(snapshot => {
            snapshot.forEach(brnch => {
                let branch = {};
                branch.id = brnch.id;
                branch.branch = brnch.data();
                branches.push(branch);
            })
            return branches;
        })
        .catch(err => {
            throw new Error(err);
        })

}