Lets say i am using Express to build a simple API to POST and GET customer objects.
(Please ignore syntax, or anything Express specific. This is just free form written in SublimeText to explain my question.)
In my route, i call a method that looks something like this:
// Main Express index.js server file.
function getCustomer(req, res, next) {
var q = request.query;
Customer.findByCustomerId(q.customerId, function(error, customer) {
if (error) { // No idea if this is a Mongo Error, NotFound Error, InactiveError, something else from some other part of the app!
res.send("??????");
return next();
}
res.send(201, customer.JSONResponse());
return next();
});
}
// Customer.js Mongoose Model in separate file.
Customer.statics.findByCustomerId = function(customerId, callback) {
this.findOne({customerId : customerId}, function(error, foundCustomer) {
if (error) { // MONGO ERROR, Something is really broken.
return callback(error, null);
}
if (!foundCustomer) { // Didn't find a customer so lets send that back.
return callback(new Error("No customer found"), null);
}
if (foundCustomer.status == "inactive") { // Lets just say a customer being inactive is error worthy.
return callback(new Error("The customer you have specified is inactive"), null);
}
return callback(null, foundCustomer);
});
};
Several things could happen here:
A valid customer exists and is returned with null error.
MongoDB has an error and sends it to the callback with null customer.
My custom customer finder by id method has an error:
a. No customer was found.
b. The customer was found but is not active.
How do i handle sending a response with the correct error? (Without a switch or if mess.)
If it's a mongo error, i want to send a generic 500.
If the customer is not found, i want to send a 404 with a friendly message.
If a customer is found but is not active, i want to send some other error response.
Is there a general guideline or best practice for this sitauation?
Thank you in advance!
So, yes. In general, the best way to handle this is to bubble custom errors up and then have some Express middleware that handles those errors in whatever way is appropriate.
So for example, rather than new Error("No customer found");, you could create a RecordNotFoundError that you then instantiate and throw. This makes it easier for the error handling middleware. For example, your Error subclass could implement a code or status property that maps directly to the HTTP status code you want sent (for consistent middleware handling).
I would also make sure you're encapsulating your logic the right way. To me, the 'active' flag on the customer should just be a filter criteria. If it's the default case that you only want to see active folks, then that should be no different than a Record not found situation, to me. That said, if you really want it to be its own error for some reason, then make a CustomerNotActiveError subclass and decide what HTTP status code you think is appropriate.
The different cases will still need to be handled like you did, but the nice thing is that your controller logic gets simpler, you just either render / send the data from the Customer, or you callback with the error that was sent. As I said, a generic error handler middleware can then receive those errors and check their name or status to decide how to handle them if you want, though in my experience the easiest thing to do is to have each error define it's own HTTP status code, and that way you don't need the switch or if mess like you describe. Or if you do have an if you can at least simplify and use it to just decide whether to log the error or not (most you will, but some you might not).
One observation, you're setting status to 201 in your example code, but if you're just doing a lookup the correct code is 200; 201 would be correct if you're actually creating a new thing.