Take the tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

controllers/users.js

var usersModel = require('../models/users.js');

users.create = function(req, res){
  usersModel.create(req.data.username, req.data.password, function(error, user){
    if(error){
      res.statusCode = 500;
      res.end('server-error');
      return;
    }

    res.statusCode = 201;
    res.end(user._id);
  });
};

models/users.js

var db = require('../mongodb.js');

module.exports.create = function(username, password, callback){
  db.collection('main', 'users', function(error, collection){
    if(error){
      callback(error);
      return;
    }

    collection.insert({
      username: username,
      password: password,
      createdOn: new Date()
    }, callback);
  });
};

mongodb.js

var mongo = require('mongodb')
  , mongodb = module.exports
  , connections = {}
  , config = {
    main: {
      name: 'application-main',
      server: new mongo.Server('127.0.0.1', 27017, { auto_reconnect: true })
    }
  };

mongodb.database = function(key, callback){
  if(connections[key]){
    callback(null, connections[key]);
    return;
  }

  var db = mongo.Db(config[key].name, config[key].server);

  db.open(function(error, connection){
    if(error){
      callback(error);
      return;
    }

    connection.on('close', function(){
      delete(connections[key]);
    });

    connections[key] = connection;
    callback(null, connection);
  }
};

mongodb.collection = function(dbKey, collName, callback){
  mongodb.database(dbKey, function(error, connection){
    if(error){
      callback(error);
      return;
    }

    connection.collection(collName, function(error, collection){
      if(error){
        callback(error);
        return;
      }

      callback(null, collection);
    });
  });
};

As you can see, the errors are passed through every callback, but infact, it might be better to throw an error at higher levels for example, look at the mongodb.js database function, it passes through an error in the callback, however if there was an error at that level it might be better to deal with it at that point and just fall back on procces.on('uncaughtException')

It is alot of extra code each time, dealing with errors in callbacks. And I am wonder if there is a better way around it.

share|improve this question
add comment

1 Answer

up vote 5 down vote accepted

First, there is a principle you should be reminded: Never deal with the error you don't know, just pop it up.

Which means, if you can't deal with or don't know about the error passed in the callback, you just pass it to next callback, or throw it(A pattern of CHAIN OF RESPONSIBILITY)!

For the errors you know how to deal with it, you can define a module for error handling and deal with it according to the type of error and log the error for debugging, like the following way.

error-handler.js

var log = require('./log'); // Log module(local or third-party);

function handler(error){
  if(error instanceof error1){
    // log it
    // handler for error1
  }else if(error instanceof error2){
    // log it 
    // handler for error2
  }else if ....
};

module.exports = handler;

logic.js

var errorHandler = require('./error-handler');

db.open(function(err, connection){
  if(err){ return errorHandler(err); }
});

For good modules like mongoose will throw their customized Errors.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.