Hi guys I'm doing my first full-stack MERN application and I've been trying everything to check if a user already exists (by either .exec() or using a callback function) in my database but I keep running into request errors (500 etc), code is below. I need your help geniuses!!!!

'userModel.js'

const mongoose = require('mongoose');
const Schema = mongoose.Schema;

// new user model for mongo
const UserSchema = new Schema({
  username: {
    type: String,
    required: true,
  },
  password: {
    type: String,
    required: true
  }
}, 
{
  collection: 'users'
})

module.exports = User = mongoose.model('user', UserSchema);

'users.js'

const express = require('express')
const router = express.Router()
const userModel = require('../models/userModel')
const mongoose = require('mongoose')

// POST users to db
router.post('/', (req,res) =>{

  // new instance of user model to be mutated
  const User = new userModel({
      // grabbing user register data and pushing to mongo
      username: req.body.username,
      password: req.body.password
  })

  // checking if user already exists
  User.findOne({username: req.body.username}, async function(err, user){
    if (err) {
      console.log(err);
    }
    if(user){
      console.log('user exists')
    }
    else{
      const newUser = await user.save()
      res.json(newUser)
      console.log("username: "  req.body.username   ", password: "   req.body.password)
      /*catch (err) {
        err => {res.json({message: err.message})}
      }*/
    }

  })
})

module.exports = router;

'app.js'

// Connect to database
const db = require('./bin/mongoURI').mongoURI;

mongoose
  .connect(db, {
    useNewUrlParser: true,
    useUnifiedTopology: true,
  })
  .then(() => console.log('CONNECTED TO MONGODB'))
  .catch(err => console.log(err));

CodePudding user response：

There are a few things that are wrong with your code, here are the explanations:

1) Take advantage of mongoose framework

Instead of searching for a User that exists under the same username, you can take advantage of the unique property from mongoose. It will not allow you to post a user which username already exists in your DB.

That way, you can get rid of your call to findOne() which in turn, reduces the total response time of your endpoint. You can read more here.

Your schema would then look like this:

const UserSchema = new Schema({
  username: {
    type: String,
    required: true,
    unique: true        // here
  },
  password: {
    type: String,
    required: true
  }
}, 
{
  collection: 'users'
})

2) Do not confuse Model and Document

In your users.js file, you wrote this:

const User = new userModel({
      // grabbing user register data and pushing to mongo
      username: req.body.username,
      password: req.body.password
  })

  // checking if user already exists
  User.findOne({username: req.body.username}, [...]

By writing const User = new userModel([....]), you declare a document, which is an instance of a model. On the next line, you attempt to call the method findOne() on this document, which is never going to work since this is a method from the models. You can read more about that here.

You should've written instead:

const User = new userModel({
      username: req.body.username,
      password: req.body.password
  })

  // NOTE HERE the use of userModel, instead of User
  userModel.findOne({username: req.body.username}, [...]

3) async/await, try/catch and how I would write your router.post route

You are using a callback syntax while also using async/await syntax:

User.findOne({username: req.body.username}, async function(err, user){[...]

which is very confusing.

Now, I don't know if the combination of the two is even possible since I've never tried it, but it might be source of errors. I recommend sticking to one of the two, at your liking (although I prefer async/await myself).

You can also take advantage of try/catch blocks, to gracefully handle errors and send feedback to your user even if something went wrong in the execution of the code.

Here is a full example (non-tested, mind you) of how I would've done it, keeping most of your structure not to confuse you:

// Note the use of async right before (req,res)

router.post('/', async (req,res) => {
  try {
    // You don't actually need to precisely fill the constructor, mongoose will complain if anything is out of place
    const User = new userModel(req.body);

    const newUser = await user.save();
    console.log("username: "  req.body.username   ", password: "   req.body.password);
    res.json(newUser);
  } catch (err) {
    // Handle here any kind of problem, be it ValidationError or more
    return res.status(400).send({error: 'Oops, something went wrong'});
  }
});