CODEWITHSUNDEEP
node.jsmongodbexpress
difegui
difegui

Reputation: 57

Node.js express with mongodb calls does not finish execution

I have a .post function where I am using mongodb call, something like this:

router.post('/insert')=>{

-findOne
// Some code
// with error return redirect('/error')

-Count
// with error return redirect('/error')

-Insert
// with error return redirect('/error') 
// with normal execution return redirect('/save')
}

My problem is that after this rediret, this function execution does not finish. So, if what i am checknig after findOne fails, i do not want to go inside Count or Insert

note that I am already using return redirect()

Edit Adding some code:

router.post('/insert_article', common.restrict, (req, res) => {
db.findOne({
        _id: root_id
    },(err, item) => {
            if (item.language !== req.body.form_language) {
                req.session.message = req.i18n.__('Language does not match');
                req.session.message_type = 'Fail!';
                return res.redirect(req.app_context + '/insert');
            }
    })

 db.insert(doc, (err, newDoc) => {
                if (err) {
                    console.error('Error inserting document: ' + err);

                } else {
                  //some stuff
                  req.session.message = req.i18n.__('New article successfully created');
                  req.session.message_type = 'success';

                    // redirect to new doc
                  res.redirect(req.app_context + '/edit/' + newId);
                }
    })
}

I think now it is clearer, when that "language" doesnt match, i do not want to do this insert anyway, and this keep doing it

Upvotes: 0

Views: 118

Answers (1)

awarrier99
awarrier99

Reputation: 3855

Edit: With your updated code, it's clear you are trying to return from a callback, refer to the snippets below

Without more code it's hard to say but my best guess is that you are using return redirect('/error') inside of a callback function (such as the callback supplied to .then if, for example, you are using promises for your database methods), so rather than returning from the overall function, you are returning from the callback, and then continuing through the route handler. If this is the case, then it may be better to use an async route handler so you can await your database call instead and return res.redirect if something goes wrong, which will then act as you expect

Your code is doing this where you are returning from the callback, so execution inside of the route handler will continue

router.post('/insert_article', common.restrict, (req, res) => {
    db.kb.findOne({
        _id: root_id
    },(err, item) => {
            if (item.language !== req.body.form_language) {
                req.session.message = req.i18n.__('Language does not match');
                req.session.message_type = 'Fail!';
                // the following line will return from your callback, NOT the route handler
                return res.redirect(req.app_context + '/insert');
            }
    })
    // because you returned from the callback, code here will continue to execute
});

you could instead do this

router.post('/insert_article', common.restrict, (req, res) => {
    db.kb.findOne({
        _id: root_id
    },(err, item) => {
            if (item.language !== req.body.form_language) {
                req.session.message = req.i18n.__('Language does not match');
                req.session.message_type = 'Fail!';
                return res.redirect(req.app_context + '/insert'); // returns from callback, not the route handler
            }

            db.kb.insert(doc, (err, newDoc) => {
                if (err) {
                    console.error('Error inserting document: ' + err);
                } else {
                    //some stuff
                    req.session.message = req.i18n.__('New article successfully created');
                    req.session.message_type = 'success';

                    // redirect to new doc
                    res.redirect(req.app_context + '/edit/' + newId);
                }
                });
            }
    })
});

But the above approach will create "callback hell", which is callbacks inside of callbacks inside of callbacks...and so on

A better way to do this would be using promises rather than callback functions and async/await syntax:

router.post('/insert_article', common.restrict, async (req, res) => {
    try {
        const item = await db.kb.findOne({ _id: root_id });
        if (item.language !== req.body.form_language) {
            req.session.message = req.i18n.__('Language does not match');
            req.session.message_type = 'Fail!';
            return res.redirect(req.app_context + '/insert');
        }

        await db.kb.insert(doc); // if there is an error, it will be caught in the catch block
        req.session.message = req.i18n.__('New article successfully created');
        req.session.message_type = 'success';
        // redirect to new doc
        return res.redirect(req.app_context + '/edit/' + newId);
    } catch (e) {
        return res.redirect(req.app_context + '/insert');
    }
});

Upvotes: 1

Related Questions