From bf5e9016a4cb7889b3a9e39b90eb35cb8f7f9ec8 Mon Sep 17 00:00:00 2001 From: Tyler Renelle Date: Wed, 5 Feb 2014 17:26:09 -0700 Subject: [PATCH] fix(errors): `return next(err)` when experiencing errors, instead of res.json(500,{err:err}). Let the top-level error handler handle this (needed for upcoming versionerror discarding) --- src/controllers/user.js | 14 +++++++------- src/utils.js | 30 +++++++++++++++--------------- 2 files changed, 22 insertions(+), 22 deletions(-) diff --git a/src/controllers/user.js b/src/controllers/user.js index a44f5937da..45b093e85e 100644 --- a/src/controllers/user.js +++ b/src/controllers/user.js @@ -81,7 +81,7 @@ api.score = function(req, res, next) { var delta = user.ops.score({params:{id:task.id, direction:direction}}); user.save(function(err,saved){ - if (err) return res.json(500, {err: err}); + if (err) return next(err); // TODO this should be return {_v,task,stats,_tmp}, instead of merging everything togther at top-level response // However, this is the most commonly used API route, and changing it will mess with all 3rd party consumers. Bad idea :( res.json(200, _.extend({ @@ -193,7 +193,7 @@ api.update = function(req, res, next) { }); user.save(function(err) { if (!_.isEmpty(errors)) return res.json(401, {err: errors}); - if (err) return res.json(500, {err: err}); + if (err) return next(err); res.json(200, user); }); }; @@ -235,7 +235,7 @@ api.cron = function(req, res, next) { api['delete'] = function(req, res) { res.locals.user.remove(function(err){ - if (err) return res.json(500,{err:err}); + if (err) return next(err); res.send(200); }) } @@ -258,7 +258,7 @@ api.addTenGems = function(req, res) { var user = res.locals.user; user.balance += 2.5; user.save(function(err){ - if (err) return res.json(500,{err:err}); + if (err) return next(err); res.send(204); }) } @@ -381,7 +381,7 @@ api.cast = function(req, res) { var done = function(){ var err = arguments[0]; var saved = _.size(arguments == 3) ? arguments[2] : arguments[1]; - if (err) return res.json(500, {err:err}); + if (err) return next(err); res.json(saved); } @@ -445,12 +445,12 @@ _.each(shared.wrap({}).ops, function(op,k){ res.locals.user.ops[k](req,function(err, response){ // If we want to send something other than 500, pass err as {code: 200, message: "Not enough GP"} if (err) { - if (!err.code) return res.json(500,{err:err}); + if (!err.code) return next(err); if (err.code >= 400) return res.json(err.code,{err:err.message}); // In the case of 200s, they're friendly alert messages like "You're pet has hatched!" - still send the op } res.locals.user.save(function(err){ - if (err) return res.json(500,{err:err}); + if (err) return next(err); res.json(200,response); }) }) diff --git a/src/utils.js b/src/utils.js index 1fb45587af..a4a44d52c2 100644 --- a/src/utils.js +++ b/src/utils.js @@ -49,21 +49,21 @@ module.exports.setupConfig = function(){ }; module.exports.crashWorker = function(server,mongoose) { - return function(err, req, res, next) { - if (!cluster.isMaster) { - // make sure we close down within 30 seconds - var killtimer = setTimeout(function() { - process.exit(1); - }, 30000); - // But don't keep the process open just for that! - killtimer.unref(); - // stop taking new requests. - server.close(); - mongoose.connection.close(); - cluster.worker.disconnect(); - } - next(err); - }; + return function(err, req, res, next) { + if (!cluster.isMaster) { + // make sure we close down within 30 seconds + var killtimer = setTimeout(function() { + process.exit(1); + }, 30000); + // But don't keep the process open just for that! + killtimer.unref(); + // stop taking new requests. + server.close(); + mongoose.connection.close(); + cluster.worker.disconnect(); + } + next(err); + }; }