Separating Mongoose code from Express Router

Question

So basically, I'm trying to separate my code that handles data (mongoose) from my express Router code, since I might want to use it elsewhere too.

The first thing I did was, I got rid of the res.json() calls, since I don't want the code to only work returning a http response. I want it to return data, so I can then return that data from my router as a http response, but still use it as regular data elsewhere.

Here is a function I wrote to get data from mongoose.

module.exports.user_login = data => {
    console.log(data);
    ModelUser.findOne({email: data.email}).then(user => {
        if(!user){
            console.log({email: 'E-mail address not found'});
            return {
                status: response_code.HTTP_404,
                response: {email: 'E-mail address not found'}
            }
        }

        bcrypt.compare(data.password, user.password).then(isMatch => {
            if(!isMatch){
                console.log({password: 'Invalid password'});
                return {
                    status: response_code.HTTP_400,
                    response: {password: 'Invalid password'}
                }
            }
                
            const payload = {
                id: user.id,
                email: user.email
            };

            jwt.sign(
                payload,
                config.PASSPORT_SECRET,
                { 
                    expiresIn: "1h"
                },
                (err, token) => {
                    console.log({
                        status: response_code.HTTP_200,
                        response: {
                            success: true,
                            token: token
                        }
                    });
                    return {
                        status: response_code.HTTP_200,
                        response: {
                            success: true,
                            token: token
                        }
                    }
                }
            );
        });
    });
};

When this code gets executed in my route like so:

router.post("/login", (req, res) => {
    const { errors, isValid } = validateLogin(req.body);
    if(!isValid) return res.status(400).json(errors);

    console.log("ret", dm_user.user_login(req.body));
    
});

The log says the return value of user_login() is undefined, even though right before the return statement in user_login() I am logging the exact same values and they are getting logged.

Before I changed it to a log, I tried to store the return value in a variable, but obviously that remained undefined as well, and I got the error: 'Cannot read propery 'status' of undefined' when trying to use the value.

I am definitely missing something..

Answer 1

Please notice that whenever you are trying to return any value you are always present in the callback function and that is definitely not going to return any value to its intended place.

There are a couple of things you can improve about your code :

1.Donot use jwt inside your code where you are making database calls, instead move it where your routes are defined or make a separate file.

2.If you are intending to re-use the code, I would suggest you either use async-await as shown in the answer above by Ifaruki or you can use something like async.js. But the above shown approach is better.

3. Also always use 'error' field when you are making db calls like this:

ModelUser.findOne({email: data.email}).then((error,user) => {

Answer 2

Well you have an small callback hell here. It might be a good idea to go with async / await and splitting up your code into smaller chunks instead of putting everyhing in 1 file.

I rewrote your user_login function:

const { generateToken } = require("./token.js");
module.exports.user_login = async data => {
  let user = await ModelUser.findOne({ email: data.email });
  if (!user) {
    console.log({ email: "E-mail address not found" });
    return {
      status: response_code.HTTP_404,
      response: { email: "E-mail address not found" }
    };
  }

  let isMatch = await bcrypt.compare(data.password, user.password);
  if (!isMatch) {
    console.log({ password: "Invalid password" });
    return {
      status: response_code.HTTP_400,
      response: { password: "Invalid password" }
    };
  }

  const payload = {
    id: user.id,
    email: user.email
  };

  let response = await generateToken(
    payload,
    config.PASSPORT_SECRET,
    response_code
  );
  return response;
};

I have moved your token signing method into another file and promisfied it:

module.exports.generateToken = (payload, secret, response_code) => {
  return new Promise((res, rej) => {
    jwt.sign(
      payload,
      secret,
      {
        expiresIn: "1h"
      },
      (err, token) => {
        if (err) {
          rej(err);
        }
        res({
          status: response_code.HTTP_200,
          response: {
            success: true,
            token: token
          }
        });
      }
    );
  });
};

Now you need to change your router function into an async:

router.post("/login", async (req, res) => {
    const { errors, isValid } = validateLogin(req.body);
    if(!isValid) return res.status(400).json(errors);

    let result = await dm_user.user_login(req.body);
    console.log(result);
    
});

In addition: You get undefined because you return your value to an callback function

I also would seperate your routes from your controllers instead of writing your code inside an anonymous function