ASP.Net MVC Try Catch Best Practice

Question

Could anyone please guide me on what's the best practice to handle exception in ASP.NET MVC?

1. Controller?
2. Model?
3. Model (Contains EF logics i.e. save, update etc) throw any exception and catch in Controller?

Answer 1

In any method or block of code where you may expect specific exceptions (interacting with a database or an external service which may or may not not be available, etc.) wrap that code in a Try/Catch to catch the specific exception(s) in question. You'd want to know exactly what kind of exceptions occurred to handle them properly. (Naturally, use a Finally block to dispose of any open resources.) How to properly handle them is up to you and how you want your application to behave.

You should definitely have a global exception handling to catch anything unexpected which falls through the cracks. At no point should an unhandled exception bubble up to the user. The global exception handler should just present the user with a friendly error message and should log the exception and notify you of what happened. Generally, a good goal is to identify the exception and add error handling code to catch it in its localized state before it bubbles up to global. The goal, over time, should be to have as few global exceptions as possible and to have any potentially-exception-generating code have its own error handling to guard against those cases.

An example of the latter could be something as simple as a particular method receiving null arguments that you want to check for before using those arguments. One thing you want to avoid, however, is using exception handling for logic flow. For example...

Let's say you have a method which takes a custom object as an argument:

public MyMethod(MyObject obj)
{
  // ...
}

Your original code assumes that obj will always have a value. However, after some time of production use, you discover that obj is sometimes null and that it's throwing a NullReferenceException from within that method, which is being caught by the global exception handler.

You may be tempted to just wrap the code in MyMethod in its own Try/Catch block to catch that exception. This isn't necessarily a good idea (though there may be cases where it is). Instead, you'd want to check for null at the start of the method:

public MyMethod(MyObject obj)
{
  if (obj == null) throw new ArgumentNullException("obj can not be NULL");
  // ...
}

This encapsulates the method better and allows it to throw controlled exceptions. Exceptions aren't bad things, only unexpected exceptions. Note that the above will still throw the exception which will still bubble up to the global handler. Thus, it's also a good idea to wrap the calls to this method in a Try/Catch block in order to catch the ArgumentNullException and handle it accordingly. Perhaps the code which calls this method can fix the null reference and try again, perhaps it can try something else, etc. You still don't want it to bubble up to the global handler if possible, since this has become an "expected" exception and can be handled accordingly.

Naturally, you still want to avoid the throwing/catching of the expected exceptions in the above example, so similar checks for null should happen before calling the method so that the method isn't even called.

Maybe if the object is null you can directly present the user with an error message and log the error and notify you of as much information about the state of things as possible so that you can research why it's null and fix it. Maybe being null is a perfectly acceptable state of that object at that time according to the logic of the application. If so, check if it's null and don't bother calling the method. Just carry on as normal.

It's a lot of error-checking and handling code, but that's a good thing.

Edit: Another thing to note about exception handling is that you should catch the exception only if you can actually handle it at that time. If the method can't handle it internally, let the exception bubble up from the method to the calling method, and so on. Thus, the only exceptions which should reach the global handler are exceptions which you can't actually handle anywhere in the code (which is why it's a good goal to fix and prevent global exceptions).

"Handle" in this case would mean being able to actually do something about it. Maybe that something is to just log it and carry on, maybe that something is to perform some specific logic, etc. But to catch, log, and re-throw is bad design. If you can't recover from the exception, let it bubble up.

Answer 2

The best way is to catch the exception globally, log it using Elmah. Otherwise you will have to put all your exceptions in your every controller code and that would be a lot of repetition especially for CRUD operations