From: Luigi Pinca Date: Tue, 24 Dec 2013 14:31:07 +0000 (+0100) Subject: refactoring and improved error handling X-Git-Url: https://git.saalbach.dev/?a=commitdiff_plain;h=0406f838c1152d08c7cc48ca54f904bbbb4341d9;p=binbsis50.git refactoring and improved error handling --- diff --git a/app.js b/app.js index 074f9ef..c3155cc 100644 --- a/app.js +++ b/app.js @@ -2,7 +2,8 @@ * Module dependencies. */ -var express = require('express') +var errorHandler = require('./lib/middleware/errorHandler') + , express = require('express') , http = require('http') , port = require('./config').port , redisstore = require('connect-redis')(express) @@ -31,6 +32,8 @@ app.use(express.session({ rolling: true, store: sessionstore })); +app.use(app.router); +app.use(errorHandler); // Routes app.get('/', site.home); diff --git a/lib/middleware/errorHandler.js b/lib/middleware/errorHandler.js new file mode 100644 index 0000000..cc8301b --- /dev/null +++ b/lib/middleware/errorHandler.js @@ -0,0 +1,15 @@ +/** + * Basic error handling middleware. + */ + +module.exports = function(err, req, res, next) { + if (Array.isArray(err)) { + err.forEach(function(err) { + console.error(err.message); + }); + } + else { + console.error(err.message); + } + res.send(500); +}; diff --git a/lib/rooms.js b/lib/rooms.js index e0c105e..39c159e 100644 --- a/lib/rooms.js +++ b/lib/rooms.js @@ -4,7 +4,6 @@ var amatch = require('./match') , clients = require('./redis-clients') - , collectStats = require('./stats') , config = require('../config') , fifolength = config.songsinarun * config.gameswithnorepeats , primus @@ -12,6 +11,7 @@ var amatch = require('./match') , rooms = {} // The Object that contains all the room instances , songsdb = clients.songs , sparks + , updateStats = require('./stats') , usersdb = clients.users , utils = require('./utils') , isString = utils.isString @@ -121,7 +121,7 @@ function Room(roomname) { if (usersData[nickname].registered) { stats.userscore = usersData[nickname].points; stats.guesstime = usersData[nickname].guesstime; - collectStats(nickname, stats); + updateStats(nickname, stats); } }; @@ -162,13 +162,13 @@ function Room(roomname) { // Collect podium stats if (podium[0] && podium[0].registered) { - collectStats(podium[0].nickname, {firstplace:true}); + updateStats(podium[0].nickname, {firstplace:true}); } if (podium[1] && podium[1].registered) { - collectStats(podium[1].nickname, {secondplace:true}); + updateStats(podium[1].nickname, {secondplace:true}); } if (podium[2] && podium[2].registered) { - collectStats(podium[2].nickname, {thirdplace:true}); + updateStats(podium[2].nickname, {thirdplace:true}); } resetPoints(false); @@ -203,8 +203,8 @@ function Room(roomname) { spark.send('artistmatched'); primus.room(roomname).send('updateusers', usersData); if (usersData[spark.nickname].registered) { - var stats = {points:1,userscore:usersData[spark.nickname].points}; - collectStats(spark.nickname, stats); + var stats = {points:1, userscore:usersData[spark.nickname].points}; + updateStats(spark.nickname, stats); } } else if (amatch(title, guess)) { @@ -214,8 +214,8 @@ function Room(roomname) { spark.send('titlematched'); primus.room(roomname).send('updateusers', usersData); if (usersData[spark.nickname].registered) { - var stats = {points:1,userscore:usersData[spark.nickname].points}; - collectStats(spark.nickname, stats); + var stats = {points:1, userscore:usersData[spark.nickname].points}; + updateStats(spark.nickname, stats); } } else { @@ -255,7 +255,7 @@ function Room(roomname) { if (usersData[who]) { // Inform the bad player that he/she is being ignored var recipient = sparks[who]; - recipient.send('chatmsg', executor+' is ignoring you.', 'binb', who); + recipient.send('chatmsg', executor + ' is ignoring you.', 'binb', who); return callback(true, who); } callback(false); @@ -268,13 +268,17 @@ function Room(roomname) { // Kick a user this.kick = function(who, why, executor, callback) { - usersdb.hget('user:'+executor, 'role', function (err, role) { + usersdb.hget(['user:' + executor, 'role'], function(err, role) { + if (err) { + console.error(err.message); + return callback(true); + } if (role > 0) { // Check role if (usersData[who]) { if (why) { - why = ' ('+why+')'; + why = ' (' + why + ')'; } - var notice = 'you have been kicked by '+executor+why+'.'; + var notice = 'you have been kicked by ' + executor + why + '.'; var recipient = sparks[who]; recipient.send('chatmsg', notice, 'binb', who); recipient.end(); @@ -337,7 +341,11 @@ function Room(roomname) { // Extract a random track from the database and send the load event var sendLoadTrack = function() { var index = randInt(trackscount); - songsdb.zrange(roomname, index, index, function(err, res) { + songsdb.zrange([roomname, index, index], function(err, res) { + if (err) { + console.error(err.message); + process.exit(1); + } var id = res[0]; // Check if extracted track is in the list of already played tracks if (~playedtracks.indexOf(id)) { @@ -345,14 +353,18 @@ function Room(roomname) { } playedtracks.push(id); var args = [ - 'song:'+id + 'song:' + id , 'artistName' , 'trackName' , 'previewUrl' , 'artworkUrl60' , 'trackViewUrl' ]; - songsdb.hmget(args, function(e, replies) { + songsdb.hmget(args, function(err, replies) { + if (err) { + console.error(err.message); + process.exit(1); + } artistName = replies[0]; artist = artistName.toLowerCase(); trackName = replies[1]; @@ -413,29 +425,37 @@ function Room(roomname) { // A user is submitting a name this.setNickName = function(spark, nickname) { - var feedback = null; + var feedback; if (nickname === 'binb') { - feedback = 'That name is reserved.'; + feedback = 'That name ' + + 'is reserved.'; } else if (!isUsername(nickname)) { - feedback = 'Name must contain only '; - feedback += 'alphanumeric characters.'; + feedback = 'Name must ' + + 'contain only alphanumeric characters.'; } else if (sparks[nickname]) { - feedback = 'Name already taken.'; + feedback = 'Name ' + + 'already taken.'; } if (feedback) { return spark.send('invalidnickname', feedback); } - // Check if requested nickname belong to a registered user - var key = 'user:'+nickname; - usersdb.exists(key, function(err, resp) { - if (resp === 1) { - feedback = 'That name belongs '; - feedback += 'to a registered user.'; + // Check if requested nickname belongs to a registered user + var key = 'user:' + nickname; + usersdb.exists([key], function(err, exists) { + if (err) { + console.error(err.message); + feedback = 'Could not ' + + 'check name availability.'; + return spark.send('invalidnickname', feedback); + } + if (exists) { + feedback = 'That name ' + + 'belongs to a registered user.'; return spark.send('invalidnickname', feedback); } spark.nickname = nickname; @@ -455,7 +475,11 @@ function Room(roomname) { // Start the room this.start = function() { - songsdb.zcard(roomname, function(err, card) { + songsdb.zcard([roomname], function(err, card) { + if (err) { + console.error(err.message); + process.exit(1); + } trackscount = card; sendLoadTrack(); }); @@ -469,7 +493,7 @@ function Room(roomname) { this.unignore = function(who, executor) { if (usersData[who]) { // Inform the bad player that he/she is no longer ignored - var notice = executor+' has stopped ignoring you.'; + var notice = executor + ' has stopped ignoring you.'; var recipient = sparks[who]; recipient.send('chatmsg', notice, 'binb', who); } diff --git a/lib/stats.js b/lib/stats.js index 7e30939..e279549 100644 --- a/lib/stats.js +++ b/lib/stats.js @@ -5,61 +5,94 @@ var db = require('./redis-clients').users; /** - * Expose a function to collect user statistics. + * Update user statistics. */ -module.exports = function(username, stats) { - var key = 'user:'+username; +var updateStats = function(key, multi, username, stats) { if (stats.points) { // Update total points - db.hincrby(key, 'totpoints', stats.points); + multi.hincrby(key, 'totpoints', stats.points); // Update the score of the member in the sorted set - db.zincrby('users', stats.points, username); - } - if (stats.userscore) { - // Set personal best - db.hget(key, 'bestscore', function(err, res) { - if (res < stats.userscore) { - db.hset(key, 'bestscore', stats.userscore); - } - }); + multi.zincrby('users', stats.points, username); } if (stats.gold) { // Update the number of golds - db.hincrby(key, 'golds', 1); + multi.hincrby(key, 'golds', 1); } if (stats.silver) { - db.hincrby(key, 'silvers', 1); + multi.hincrby(key, 'silvers', 1); } if (stats.bronze) { - db.hincrby(key, 'bronzes', 1); - } - if (stats.guesstime) { - // Update the number of guessed tracks - db.hincrby(key, 'guessed', 1); - // Update total guess time - db.hincrby(key, 'totguesstime', stats.guesstime); - // Set best answer time - db.hget(key, 'bestguesstime', function(err, res) { - if (stats.guesstime < res) { - db.hset(key, 'bestguesstime', stats.guesstime); - } - }); - // Set worst answer time - db.hget(key, 'worstguesstime', function(err, res) { - if (stats.guesstime > res) { - db.hset(key, 'worstguesstime', stats.guesstime); - } - }); + multi.hincrby(key, 'bronzes', 1); } if (stats.firstplace) { // Update the number of first places - db.hincrby(key, 'victories', 1); + multi.hincrby(key, 'victories', 1); } if (stats.secondplace) { - db.hincrby(key, 'secondplaces', 1); + multi.hincrby(key, 'secondplaces', 1); } if (stats.thirdplace) { - db.hincrby(key, 'thirdplaces', 1); + multi.hincrby(key, 'thirdplaces', 1); + } + multi.exec(function(err, replies) { + if (err) { + err.forEach(function(err) { + console.error(err.message); + }); + } + }); +}; + +/** + * Expose a function to update user statistics. + */ + +module.exports = function(username, stats) { + var key = 'user:' + username + , multi = db.multi(); + if (stats.guesstime) { + var args = [ + key + , 'bestscore' + , 'bestguesstime' + , 'worstguesstime' + ]; + db.hmget(args, function(err, replies) { + if (err) { + return console.error(err.message); + } + if (stats.userscore > replies[0]) { + // Set personal best + multi.hset(key, 'bestscore', stats.userscore); + } + // Update the number of guessed tracks + multi.hincrby(key, 'guessed', 1); + // Update total guess time + multi.hincrby(key, 'totguesstime', stats.guesstime); + // Set best answer time + if (stats.guesstime < replies[1]) { + multi.hset(key, 'bestguesstime', stats.guesstime); + } + // Set worst answer time + if (stats.guesstime > replies[2]) { + multi.hset(key, 'worstguesstime', stats.guesstime); + } + updateStats(key, multi, username, stats); + }); + return; + } + if (stats.userscore) { + db.hget([key, 'bestscore'], function(err, bestscore) { + if (err) { + return console.error(err.message); + } + if (stats.userscore > bestscore) { + multi.hset(key, 'bestscore', stats.userscore); + } + updateStats(key, multi, username, stats); + }); + return; } + updateStats(key, multi, username, stats); }; diff --git a/package.json b/package.json index 7ee59c3..3489123 100644 --- a/package.json +++ b/package.json @@ -30,5 +30,5 @@ "start": "node app.js" }, "subdomain": "binb", - "version": "0.3.6-21" + "version": "0.4.0" } diff --git a/routes/site.js b/routes/site.js index d559f55..fe43ed4 100644 --- a/routes/site.js +++ b/routes/site.js @@ -17,8 +17,11 @@ var async = require('async') var subTask = function(genre) { return function(callback) { var index = randInt(rooms[genre].tracksCount()); - db.zrange(genre, index, index, function(err, res) { - db.hget('song:'+res[0], 'artworkUrl100', callback); + db.zrange([genre, index, index], function(err, res) { + if (err) { + return callback(err); + } + db.hget(['song:' + res[0], 'artworkUrl100'], callback); }); }; }; @@ -27,7 +30,7 @@ var subTask = function(genre) { * Extract at random in each room, some album covers and return the result as a JSON. */ -exports.artworks = function(req, res) { +exports.artworks = function(req, res, next) { var tasks = {}; config.rooms.forEach(function(room) { tasks[room] = function(callback) { @@ -39,7 +42,10 @@ exports.artworks = function(req, res) { }; }); async.parallel(tasks, function(err, results) { - res.json(results); + if (err) { + return next(err); + } + res.send(results); }); }; diff --git a/routes/user.js b/routes/user.js index 6d4c3d5..c7c033a 100644 --- a/routes/user.js +++ b/routes/user.js @@ -22,9 +22,15 @@ for (var i=0; i 180 || (by !== 'points' && by !== 'times')) { @@ -44,13 +50,19 @@ exports.sliceLeaderboard = function(req, res) { } var end = begin + 29; if (by === 'points') { - db.zrevrange('users', begin, end, 'withscores', function(err, results) { - res.json(results); + db.zrevrange(['users', begin, end, 'withscores'], function(err, results) { + if (err) { + return next(err); + } + res.send(results); }); return; } db.sort(utils.sortParams(begin), function(err, results) { - res.json(results); + if (err) { + return next(err); + } + res.send(results); }); }; @@ -88,10 +100,16 @@ exports.validateChangePasswd = function(req, res, next) { }; exports.checkOldPasswd = function(req, res, next) { - var key = 'user:'+req.session.user; - db.hmget(key, 'salt', 'password', function(err, data) { - var hash = crypto.createHash('sha256').update(data[0]+req.body.oldpassword).digest('hex'); - if (hash !== data[1]) { + var key = 'user:' + req.session.user; + db.hmget([key, 'salt', 'password'], function(err, data) { + if (err) { + return next(err); + } + var digest + , hash = crypto.createHash('sha256'); + hash.update(data[0] + req.body.oldpassword); + digest = hash.digest('hex'); + if (digest !== data[1]) { req.session.errors = {oldpassword: 'is incorrect'}; return res.redirect(req.url); } @@ -99,13 +117,19 @@ exports.checkOldPasswd = function(req, res, next) { }); }; -exports.changePasswd = function(req, res) { - var followup = ~safeurls.indexOf(req.query.followup) ? req.query.followup : '/' +exports.changePasswd = function(req, res, next) { + var digest + , followup = ~safeurls.indexOf(req.query.followup) ? req.query.followup : '/' , user = req.session.user - , key = 'user:'+user - , salt = crypto.randomBytes(6).toString('base64') - , password = crypto.createHash('sha256').update(salt+req.body.newpassword).digest('hex'); - db.hmset(key, 'salt', salt, 'password', password, function(err, data) { + , key = 'user:' + user + , hash = crypto.createHash('sha256') + , salt = crypto.randomBytes(6).toString('base64'); + hash.update(salt + req.body.newpassword); + digest = hash.digest('hex'); + db.hmset([key, 'salt', salt, 'password', digest], function(err, data) { + if (err) { + return next(err); + } // Regenerate the session req.session.regenerate(function() { req.session.cookie.maxAge = 604800000; // One week @@ -142,9 +166,12 @@ exports.validateLogin = function(req, res, next) { }; exports.checkUser = function(req, res, next) { - var key = 'user:'+req.body.username; - db.exists(key, function(err, data) { - if (data === 1) { + var key = 'user:' + req.body.username; + db.exists([key], function(err, exists) { + if (err) { + return next(err); + } + if (exists) { // User exists, proceed with authentication return next(); } @@ -153,11 +180,17 @@ exports.checkUser = function(req, res, next) { }); }; -exports.authenticate = function(req, res) { - var key = 'user:'+req.body.username; - db.hmget(key, 'salt', 'password', function(err, data) { - var hash = crypto.createHash('sha256').update(data[0]+req.body.password).digest('hex'); - if (hash === data[1]) { +exports.authenticate = function(req, res, next) { + var key = 'user:' + req.body.username; + db.hmget([key, 'salt', 'password'], function(err, data) { + if (err) { + return next(err); + } + var digest + , hash = crypto.createHash('sha256'); + hash.update(data[0] + req.body.password); + digest = hash.digest('hex'); + if (digest === data[1]) { var followup = ~safeurls.indexOf(req.query.followup) ? req.query.followup : '/'; // Authentication succeeded, regenerate the session req.session.regenerate(function() { @@ -229,9 +262,12 @@ exports.validateSignUp = function(req, res, next) { }; exports.userExists = function(req, res, next) { - var key = 'user:'+req.body.username; - db.exists(key, function(err, data) { - if (data === 1) { + var key = 'user:' + req.body.username; + db.exists([key], function(err, exists) { + if (err) { + return next(err); + } + if (exists) { // User already exists req.session.errors = {alert: 'A user with that name already exists.'}; return res.redirect(req.url); @@ -241,9 +277,12 @@ exports.userExists = function(req, res, next) { }; exports.emailExists = function(req, res, next) { - var key = 'email:'+req.body.email; - db.exists(key, function(err, data) { - if (data === 1) { + var key = 'email:' + req.body.email; + db.exists([key], function(err, exists) { + if (err) { + return next(err); + } + if (exists) { // Email already exists req.session.errors = {alert: 'A user with that email already exists.'}; return res.redirect(req.url); @@ -252,25 +291,35 @@ exports.emailExists = function(req, res, next) { }); }; -exports.createAccount = function(req, res) { - var userkey = 'user:'+req.body.username - , mailkey = 'email:'+req.body.email +exports.createAccount = function(req, res, next) { + var digest + , hash = crypto.createHash('sha256') + , mailkey = 'email:' + req.body.email , salt = crypto.randomBytes(6).toString('base64') - , hash = crypto.createHash('sha256').update(salt+req.body.password).digest('hex') - , date = new Date().toISOString() - , user = new User(req.body.username, req.body.email, salt, hash, date); + , userkey = 'user:' + req.body.username; + hash.update(salt + req.body.password); + digest = hash.digest('hex'); + var date = new Date().toISOString() + , user = new User(req.body.username, req.body.email, salt, digest, date); - // Add new user in the db - db.hmset(userkey, user); - db.set(mailkey, userkey); - db.zadd('users', 0, req.body.username); - db.sadd('emails', req.body.email); // Delete old fields values delete req.session.oldvalues; - res.render('login', { - followup: req.query.followup || '/', - slogan: utils.randomSlogan(), - success: 'You successfully created your account. You are now ready to login.' + + // Add new user in the db + var multi = db.multi(); + multi.hmset(userkey, user); + multi.set(mailkey, userkey); + multi.zadd('users', 0, req.body.username); + multi.sadd('emails', req.body.email); + multi.exec(function(err, replies) { + if (err) { + return next(err); + } + res.render('login', { + followup: req.query.followup || '/', + slogan: utils.randomSlogan(), + success: 'You successfully created your account. You are now ready to login.' + }); }); }; @@ -302,27 +351,34 @@ exports.validateRecoverPasswd = function(req, res, next) { next(); }; -exports.sendEmail = function(req, res) { - var key = 'email:'+req.body.email; - db.get(key, function(err, data) { - if (data !== null) { - // Email exists, generate a secure random token +exports.sendEmail = function(req, res, next) { + var key = 'email:' + req.body.email; + db.get([key], function(err, data) { + if (err) { + return next(err); + } + if (data) { delete req.session.captchacode; + delete req.session.oldvalues; + // Email exists, generate a secure random token var token = crypto.randomBytes(48).toString('hex'); // Token expires after 4 hours - db.setex('token:'+token, 14400, data, function(err, reply) { + db.setex(['token:' + token, 14400, data], function(err, reply) { + if (err) { + return next(err); + } mailer.sendEmail(req.body.email, token, function(err, response) { if (err) { console.error(err.message); } }); + res.render('recoverpasswd', { + followup: req.query.followup || '/', + slogan: utils.randomSlogan(), + success: true + }); }); - delete req.session.oldvalues; - return res.render('recoverpasswd', { - followup: req.query.followup || '/', - slogan: utils.randomSlogan(), - success: true - }); + return; } req.session.errors = {alert: 'The email address you specified could not be found'}; res.redirect(req.url); @@ -333,7 +389,7 @@ exports.sendEmail = function(req, res) { * Reset user password. */ -exports.resetPasswd = function(req, res) { +exports.resetPasswd = function(req, res, next) { if (req.body.password === undefined) { return res.send(400); } @@ -357,15 +413,22 @@ exports.resetPasswd = function(req, res) { return res.redirect(req.url); } - var key = 'token:'+req.query.token; - db.get(key, function(err, user) { - if (user !== null) { - // Delete the token - db.del(key); - // Update password - var salt = crypto.randomBytes(6).toString('base64'); - var password = crypto.createHash('sha256').update(salt+req.body.password).digest('hex'); - db.hmset(user, 'salt', salt, 'password', password, function(err, data) { + var key = 'token:' + req.query.token; + db.get([key], function(err, user) { + if (err) { + return next(err); + } + if (user) { + db.del(key); // Delete the token + var digest + , hash = crypto.createHash('sha256') + , salt = crypto.randomBytes(6).toString('base64'); + hash.update(salt + req.body.password); + digest = hash.digest('hex'); + db.hmset([user, 'salt', salt, 'password', digest], function(err, data) { + if (err) { + return next(err); + } res.render('login', { followup: '/', slogan: utils.randomSlogan(), @@ -383,23 +446,31 @@ exports.resetPasswd = function(req, res) { * Show user profile. */ -exports.profile = function(req, res) { - var key = 'user:'+req.params.username; - db.exists(key, function(err, data) { - if (data === 1) { - db.hgetall(key, function(e, obj) { - obj.bestguesstime = (obj.bestguesstime/1000).toFixed(1); - obj.joindate = utils.britishFormat(new Date(obj.joindate)); - if (obj.guessed !== '0') { - obj.meanguesstime = ((obj.totguesstime/obj.guessed)/1000).toFixed(1); +exports.profile = function(req, res, next) { + var key = 'user:' + req.params.username; + db.exists([key], function(err, exists) { + if (err) { + return next(err); + } + if (exists) { + db.hgetall([key], function(err, user) { + if (err) { + return next(err); + } + var joindate = new Date(user.joindate); + user.bestguesstime = (user.bestguesstime / 1000).toFixed(1); + user.joindate = utils.britishFormat(joindate); + if (user.guessed !== '0') { + user.meanguesstime = user.totguesstime / user.guessed; + user.meanguesstime = (user.meanguesstime / 1000).toFixed(1); } - obj.worstguesstime = (obj.worstguesstime/1000).toFixed(1); - delete obj.email; - delete obj.password; - delete obj.salt; - delete obj.totguesstime; + user.worstguesstime = (user.worstguesstime / 1000).toFixed(1); + delete user.email; + delete user.password; + delete user.salt; + delete user.totguesstime; res.locals.slogan = utils.randomSlogan(); - res.render('user', obj); + res.render('user', user); }); return; }