Refactor error handling

d-pac · Aug 13, 2014 · b1d862f · b1d862f
1 parent 772a30e
commit b1d862f
Show file tree

Hide file tree

Showing 3 changed files with 61 additions and 33 deletions.
diff --git a/routes/api/middleware.js b/routes/api/middleware.js
@@ -1,12 +1,12 @@
 'use strict';
 
 var errors = require( 'errors' );
-var debug = require('debug')('dpac:api.middleware');
+var debug = require( 'debug' )( 'dpac:api.middleware' );
 
 exports.initAPI = function initAPI( req,
                                     res,
                                     next ){
-  debug('initAPI');
+  debug( 'initAPI' );
   res.apiResponse = function( status ){
     if( req.query.callback ){
       res.jsonp( status );
@@ -30,7 +30,7 @@ exports.initAPI = function initAPI( req,
 exports.requireUser = function( req,
                                 res,
                                 next ){
-  debug('requireUser');
+  debug( 'requireUser' );
   if( !req.user ){
     res.apiError( new errors.Http401Error() );
   }else{
@@ -41,16 +41,28 @@ exports.requireUser = function( req,
 exports.methodNotAllowed = function( req,
                                      res,
                                      next ){
-  res.apiError( new errors.Http406Error() );
+  return res.apiError( new errors.Http406Error() );
 };
 
-exports.requireAdmin = function(req,
-  res,
-  next){
-  debug('requireAdmin');
-  if( !req.user.isAdmin){
-    res.apiError( new errors.Http401Error() );
+exports.requireAdmin = function( req,
+                                 res,
+                                 next ){
+  debug( 'requireAdmin' );
+  if( !req.user.isAdmin ){
+    return res.apiError( new errors.Http401Error() );
   }else{
     next();
   }
 };
+
+exports.handleError = function( err,
+                                req,
+                                res,
+                                next ){
+  console.error( err );
+  if( (err.name && 'CastError' === err.name) && (err.path && '_id' === err.path ) ){
+    return res.apiError( new errors.Http404Error() );
+  }else{
+    return res.apiError( new errors.Http500Error() );
+  }
+};
diff --git a/routes/api/users/controller.js b/routes/api/users/controller.js
@@ -2,45 +2,58 @@
 var keystone = require( 'keystone' ),
   async = require( 'async' );
 var errors = require( 'errors' );
-var debug =require('debug')('dpac:api.users.controller');
+var debug = require( 'debug' )( 'dpac:api.users.controller' );
 var User = keystone.list( 'User' ).model;
 
-module.exports.list = function(req,
-  res,
-  next){
-  debug('list');
-  User.find().exec(function(err, users){
-    var results={};
-    users.forEach(function(user){
+module.exports.list = function( req,
+                                res,
+                                next ){
+  debug( 'list' );
+  User.find().exec( function( err,
+                              users ){
+    if( err ){
+      return next( err );
+    }
+    if( !users ){
+      return res.apiError( new errors.Http404Error() );
+    }
+    var results = { };
+    users.forEach( function( user ){
       results[user.id] = user;
-    });
-    res.apiResponse(users);
-  });
+    } );
+    res.apiResponse( users );
+  } );
 };
 
 module.exports.retrieve = function( req,
                                     res,
                                     next ){
-  debug('retrieve');
-  User.findById( res.locals.user.id, function( err,
-                                                     user ){
-    if( err || !user ){
+  debug( 'retrieve' );
+  User.findById( res.locals.user.id ).exec( function( err,
+                                                      user ){
+    if( err ){
+      return next( err );
+    }
+    if( !user ){
       return res.apiError( new errors.Http404Error() );
     }
-    return res.apiResponse( user.toJSON() );
+    return res.apiResponse( user );
   } );
 };
 
 module.exports.update = function( req,
                                   res,
                                   next ){
-  debug('update');
-  User.findOneAndUpdate(
-    { _id : res.locals.user.id },
-    req.body,
-    function( err,
-              user ){
-      return res.apiResponse( user.toJSON() );
+  debug( 'update' );
+  User.findByIdAndUpdate( res.locals.user.id, req.body ).exec( function( err,
+                                                                         user ){
+      if( err ){
+        return next( err );
+      }
+      if( !user ){
+        return res.apiError( new errors.Http404Error() );
+      }
+      return res.apiResponse( user );
     }
   );
 };
diff --git a/routes/index.js b/routes/index.js
@@ -56,6 +56,9 @@ exports = module.exports = function( app ){
   app.patch( '/api/users/:id', api.users.controller.update );
 
   app.all( '/api/users*', api.middleware.methodNotAllowed );
+
+  app.all( '/api*', api.middleware.handleError );
+
   // NOTE: To protect a route so that only admins can see it, use the requireUser middleware:
   // app.get('/protected', middleware.requireUser, routes.views.protected);