4
\$\begingroup\$

I've implemented a promises in mongoose to reduce the amount of callback hell but I am not sure whether my technique is correct.

The scenario:

Before I can save my schema I want to run some functions in specific order. These functions update other collections in mongodb. So I have used promises to implement this.

Code: (using promises)

// before saving do this
return incrementSessionTypesTotal(req, res)
.then(
    // before saving do this
    incrementSessionsTypes(req, res, newSurvey))
   //  now save my schema!
.then(newSurvey.save(function(err, data) {
    if (err) {
        sendJsonResponse(res, 400, err)
    } else {
        sendJsonResponse(res, 200, data);
    }
}));

Code:

function incrementSessionTypesTotal(req, res) {

return Total.findOne({dP: "xyz"}, function(err, doc) {
    if (err) {
        sendJsonResponse(res, 401, err)
    } else if (doc) {

            if (req.body.q1 === "poor") {
                doc.poorSessions += 1;
            } else if (req.body.q1 === "ok") {
                doc.okSessions += 1;
            } else if (req.body.q1 === "well") {
                doc.wellSessions += 1;
            } else if (req.body.q1 === "very well") {
                doc.veryWellSessions += 1;
            }
         }

     doc.save(function(err, data) {
        if (err) {
            sendJsonResponse(res, 400, err)
        } else {
            return data;
        }
    });

    }).exec();
 }

Is this implementation okay? I have not really used promises with mongoose before so I am not sure whether this is correct. I have tested the code and it works but I would like some feedback.

\$\endgroup\$
  • \$\begingroup\$ Edited my answer. Feel free to have a look! \$\endgroup\$ – Flame_Phoenix Apr 2 '17 at 8:54
1
\$\begingroup\$

What I don't like in your solution

In the latest version of Mongoose, save can also return a Promise. The "problem" with your code is that when you are saving you are using a callback, which defeats the purpose of Promises overall.

How I would do it

This is how I would implement using Promises:

function incrementSessionTypesTotal(req, res) {
    return Total.findOne({dP: "xyz"})
        .then(doc => {
            incrementSessionsTypes(req, res, doc));
            return doc.save();
        })
        .then(() => {
            sendJsonResponse(res, 200, data);
        })
        .catch(error => {
            sendJsonResponse(res, 400, err);
        });
}

Following the code flow

Following the code flow, without the callbacks it is clear what you do.

  1. First you findOne
  2. After finding an object, we increment it. If doc===null then the increment function will throw an exception, which will be caught later on.
  3. Then we save it. Remember I said save can also return a Promise? Well, because Promises are monads (sort of) if you return a Promise within a then, its result will follow the chain!
  4. Save was successful, send message!

Problems with my solution

As you may have guessed ... this solution catches any error and sends the same response for all of them (400).

With this structure, if you want error differentiation, the only thing you can do is to detect an error occurred, build an object with that information and then throw it:

.then(doc => {
    if(doc === null)
        throw {reason: "ObjectNotFound"};

    incrementSessionsTypes(req, res, doc));
    return doc.save();
})
.catch(error => {
    if(error.reason === "ObjectNotFound")  
        sendJsonResponse(res, 404, err);
    else if(error.reason === "Something else ")
        sendJsonResponse(/*you get the idea*/);
    else //if we don't know the reasons, its because the server likely crashed
        sendJsonResponse(res, 500, err);
});

Another solution would be to have an ErrorFactory and/or to subclass your errors.

I personally don't find any of these solutions particularly attractive because it means you will have a huge if then else chain of statements in your catch block, if you want error differentiation.

This is an issue I am still fighting with, but its up to you.

Hope it helps!


EDIT EDIT EDIT

In another question I posted what I believe to be the solution to complete error differentiation while using promises (see solution 2):

That post also has a good example of classical error building and differentiation, alongside an analysis of its limitation (solution 1).

I hope you enjoy!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.