Promises in Mongoose in recursive, returning issue

Question

I'm struggling with promises with Mongoose because I am used to do synchronous programming.

Context

I have many categories. Each category have no or one parent category. A knowledge is associated with a category. I would like to create a tree with all categories with their sub-categories and knowledge(s).

Actually, it looks like a model with directories and files. The expected output would be like a JSON :

[
    {"name": "cat0",
     "children": [{"name": know0}]
    },
    {"name": "cat1",
    ...
]

Code

I use a recursive way. I call the function with null (which get all roots categories), then the recursive call will apply on the sub-categories.

static findKnowledgeByCategory(query = null){


    return new Promise((resolve, reject) => {
        CategoryModel.find({parent: query})
            .then((categories) => {
                console.log(query + ' : CategoryModel.find success');
                return new Promise((resolve, reject) => {
                    console.log(query + ' : new Promise');
                    categories.forEach(cat => { // cat == { _id: ..., parent: ..., name: 'smthng' }
                        KnowledgeModel.find({category: cat._id})
                            .then((knowledges) => {
                                console.log(query + ' : KnowledgeModel.find success');
                                cat.knowledges = knowledges;
                                Model.findKnowledgeByCategory(cat._id)
                                    .then((categories) =>{
                                        cat.children = categories;
                                    });
                            })
                    })
                }).then(() => {
                    console.log(query + ' : Début resolve');
                    return resolve(categories);
                })
            })
            .catch((err) =>{
                console.log(err);
                return reject();
            } )
    });

}

I must return a promise with this code, because at a more global scope its resolve is used to return the tree in JSON.

Global Scope

findKnowledgeByCategory()
        .then((catAndKnow) => res.status(200).json(catAndKnow))
        .catch(err => Validation.handleError(res, 500, err));

There is no error displayed, but when I call the function, the server does not respond.

Notice it will never display "Début resolve"... I'm using Node 6.11.2 so I can't use "await". Any idea would be appreciated. I apologize if my issue is not relevant. I think I don't manage well my promises but I don't have any clue.

That's a 6th level callback hell. You really need to break this code into smaller pieces, functions, or async/await blocks. Or use the [async](https://www.npmjs.com/package/async) module. Because debugging that as it is now is the reason why it's called _hell_. — Jeremy Thille, Jan 23 '18 at 16:19

score 1 · Accepted Answer · answered Jan 23 '18 at 16:35

You should not use the promise constructor anti-pattern. As your method calls (like find) already produce promises, you should return the promise chain you attach to those.

Secondly, keep your nesting flat: promises were invented for this reason, so instead of nesting then calls, return the promise to the outer chain as soon as you can, and chain a then call there.

Finally, when you create promises within a forEach, you'll need to combine those somehow into one promise. Promise.all is just the remedy for that. So instead of forEach, use map returning promises in the callback, and pass that result to Promise.all.

Here is how your code could look (not tested):

static findKnowledgeByCategory(query = null) {
    return CategoryModel.find({parent: query}).then((categories) => {
        console.log(query + ' : CategoryModel.find success');
        return Promise.all(categories.map(cat => {
            return KnowledgeModel.find({category: cat._id}).then((knowledges) => {
                console.log(query + ' : KnowledgeModel.find success');
                cat.knowledges = knowledges;
                return Model.findKnowledgeByCategory(cat._id);
            }).then((children) => {
                cat.children = children;
            });
        })).then(() => {
            console.log(query + ' : Début resolve');
            return categories;
        })
    });
}

Thank you for your help. Looks like it does not enter in the then(children) but I think I can find by myself. — Yukeer, Jan 24 '18 at 09:03
If execution does not get to that `then` callback, check that the `KnowledgeModel.find({category: cat._id})` promise maybe rejects instead of fulfills. You could chain a `.catch` after that `then ((children)` to check that. — trincot, Jan 24 '18 at 09:06

score 0 · Answer 2 · answered Jan 23 '18 at 16:13

0

Take a look at this part of the code:

return new Promise((resolve, reject) => {
    console.log(query + ' : new Promise');
    categories.forEach(cat => { 
        KnowledgeModel.find({category: cat._id})
            .then((knowledges) => {
                console.log(query + ' : KnowledgeModel.find success');
                cat.knowledges = knowledges;
                Model.findKnowledgeByCategory(cat._id)
                .then((categories) =>{
                    cat.children = categories;
                });
        })
    })
})

You are never calling resolve nor reject inside this promise so it is never resolved. Try to split your code into smaller functions returning promises and then arrange them together - it would be much easier to catch situations like this.

answered Jan 23 '18 at 16:13

Szymon Kups

56
5

Thank you for responding. To explain this, I tried to put a resolve() after the forEach() bloc, but it is not what I want to. Because it will fulfill the promise although the loop is not finished yet. How to handle that (in other words, a synchronous process in a promise) ? – Yukeer Jan 23 '18 at 16:23
Also I tried without the "return" but without better results. – Yukeer Jan 23 '18 at 16:25
By using `category.forEach` you are iterating over each category and doing some queries for each of them. This means you should wait for all that queries to end. Take a look at https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/all method. But first of all try to reorganize this big function into smaller ones and compose them together - it will make the code more readable. – Szymon Kups Jan 23 '18 at 16:34

Promises in Mongoose in recursive, returning issue

2 Answers2