Express Returning After next()

Question

came from Java. Was messing around with ExpressJS, I return out of a function after sending a next() if I dont add the return, then code after the next() function still executes when next() is invoked, currently it works, return escapes this behaviour, but I was wondering if this is correct way to do this, or am I developing bad habbits. Nothing is really after the next() in terms of code sequence.

function('/login', (req,res, next) => {
    User.findOne({
    email: username
}, (err, user) => {
    if (user) {
        var validPassword = user.comparePassword(password);
        if (validPassword) {
            let token = Helpers.getJwt(user);
            res.send({
                success: true,
                message: 'Successful Login',
                token: token
            })
        } else {
            next(
                Boom.badRequest('Invalid Credentials.', {
                    success: false,
                    message: 'Credentials did not match our records.'
                }));
            return;
        }
    } else {
        next(
            Boom.badRequest('User not found.', {
                success: false,
                message: 'User was not found, please register.'
            }));
        return;
    }
});

EDIT: I have middleware called with the next(), my error middleware.

Answer 1

First, You aren't next-ing to anything here(pun intended).

You call middleware:next() function if there is another middleware:function within the chain of a particular route or a group of routes app.use(middleware:function).

Example:

app.get('/user/:id', function (req, res, next) {
  console.log('ID:', req.params.id)
  next()
}, function (req, res, next) {
  res.send('User Info')
})

I'd advice you read on Express Middlewares.

so yeah, you want to check if the user is valid before proceeding with the request.

Below code is just to explain how middle-ware works: The idea is that, one middleware:function process the request and passes control down to the next function within the chain.

I'm assuming your Boom.badRequest just generates a json payload e.g {};

So this is probably an elegant way to achieve what you want, but im not sure doing this for login is ideal, maybe better for checking if a user's token is valid.

app.post('/login', /*MiddleWare Function*/(req, res, next) => {
        User.findOne({email: username}, (err, user) => {
            if (user) {
                const validPassword = user.comparePassword(req.body.password);
                if (!validPassword) return res.status(401).send(Boom.badRequest('User not found.', {
                    success: false,
                    message: 'User was not found, please register.'
                }));
                req.token = Helpers.getJwt(user);
                next();// it will move execution to the next function we have below
            } else {
                res.status(401).send(Boom.badRequest('User not found.', {
                    success: false,
                    message: 'User was not found, please register.'
                }));
              //no need to next here.
            }
        })
    }, /*OUR NEXT FUNCTION*/(req, res) => {
        res.send({
            success: true,
            message: 'Successful Login',
            token: req.token//notice that our token can be retrieved here
        })
    });

So in general, you might just want to have a middleware:function that is called first on a group of specific routes.

app.use(['/users*', '/logout'], (req, res, next) => {
   /*
    * What this means is that for every request that maps to /users 
    * or /logout this middleware:function will be called first.
    */
   //we can check|test if this visitor has valid credentials.
   next();//passes control to app.get('/users/whatEverItIS?') or app.post('/logout');
});

app.post('/users/whatEverItIs', (req, res)=>{
  res.send("I passed the middleware test that is why im called");
});

Answer 2

That return statement is unnecessary. Firstly due to the fact that you are not returning anything and secondly because you are passing control to your next middleware using next method, thus that return is useless there and will be returning undefined.