You should remove final return statement (from the end of your code) and return at the correct place inside each callback if there is no error.
If you send your response in the body of the function your callbacks will never get the chance to run. Therefore you must nest your callbacks and only call res.send if you are
- returning early due to an error or
- if everything is complete.
e.g.
// Create a new user
var user = new User ({
username: input.username,
email: input.email,
password: input.password,
active: false
});
user.save(function(err) {
if(err) return res.json({success: false, errors: 'Failed To Create User'});
// Create a new Token
var newToken = createToken('new', null, user._id);
// Assign New Token To New User
if(newToken) {
user.tokens.push(newToken._id);
user.save(function(err) {
if(err) return res.json({success: false, errors: 'Failed To Save User Token'});
// Send Email To User
var mailData = {
from: 'deleted@hotmail.com',
to: input.email,
subject: 'Activate Your Account',
text: 'http://localhost:8080/api/auth/activate/' + token
}
mail.messages().send(mailData, function(err, body) {
if(err) return res.json({ success: false, errors: 'Failed To Send Email' });
return res.json({
success: true,
status: 'Successfully Registered User, Check Email To Activate'
});
});
});
}
});
Asynchronous alternatives
Unfortunately, with node.js you should get used to and understand callbacks; even if you end up using something else most of the time. The way your code was structured was neater and logical but does not work in node.js because you have to wait for the callbacks to complete before you can return from your function.
However, callbacks are the default but one of the worst mechanisms for handling asynchronous logic. If you want to structure the code differently you have quite a few options. Here are just a couple:
- Use promises instead of callbacks.
In your case your database library (mongoose? sequelize?) should have something built in that allows you to write your code like this:
user.save()
.then(function () {
// step 1
})
.then(funciton () {
// step 2
})
.done()
This style of programming is well worth learning and will make your code more readable than callbacks. callbacks vs promises
- Use Koa instead of express.
Koa, is the next generation of express written by the same people. It uses generators instead of callbacks which means you can write code that looks more like this:
// this is just an example
var result = user.save();
if (result.error) return res.send({success : false, ...});
user.token = getNewToken();
user.update();
if (result.error) return res.send({success : false, ...});
return res.send({success : true, message : "Good news, no errors"});
Generators/(aka async functions) are the direction Javascript is moving in but there is a learning curve to start using. Behind the scenes there is something very complex going on to make asynchronous code appear exactly like synchronous code. Basically, the functions know how to pause execution until they are required again.
Start with callbacks
Like I say, callbacks are not that nice. However, you should get used to using them. They are the basic building blocks of node.js and it take a while to get comfortable with better alternatives. It's also important to get used to them because otherwise you won't appreciate why the alternatives are better.
Good luck and watch out for callbacks inside loops :)
