Proper error handling in ASP.NET MVC2

Question

I have an override OnException(ExceptionContext filterContext) in my base controller to catch the app during any errors, and then log them. The problem I'm getting in my app is this particular method is fired off four times for certain errors. I'll walk you through a scenario:

Let's say i navigate to: http://localhost:180/someController/someAction?someId=XX

And I have poor object handling in my code. The Id passed in is an invalid one, and it retrieves some null object, I then, bc of my bad object handling, try to operate on a null object. I get an exception.

BaseController's OnException is fired off here.

That null object is still returned out to the view, where the view tries to bind it to something, or what have you.

BaseController's OnException is fired off here again, for the error in the view.

Essentially, only one error is important to me, but the trickle up effect is causing more errors to fire off, and spam my inbox :-/.

What is the correct way to catch an error in MVC2 and not have this happen to me?

Answer 1

First to explain why you are getting multiple errors. First error will be from trying to operate on a null object most likely in your model or controller. Your then probably getting a 2nd exception when the view is trying to bind to a null object when it is expecting an object to exist. Not exactly sure why you are getting 4 errors but could be because the code is trying to operate on an object that is currently null.

My first suggestion would be have your OnException code redirect the application to a friendly error page. Your probably just eating up each new exception and not letting the web.config handle the error pages properly if you have that setup to display an error page.

My second suggestion would be to add some code to check for null objects before you operate on them. These are commonly called Guard Clauses, and are very helpful and useful to implement. You can then determine a nice friendly way to handle errors without always logging an exception if you don't need to and to also display a friendly message to a user besides a generic "An Error has occured." message.

For example in your Controller you could check for a null object and pass an alternate view to the user if that object is null

Function Example As ActionResult

    dim obj as Object = GetObject

    If obj is Nothing Then

        Return View("FriendlyNoObjectView")

    Else

        Return View(obj)

    End If

End Function

I know this is vb (Sorry I know that better then c#) but the idea is the same. If you wanted you could still log that as an error, but you would then prevent the error from happening many times. It's always good practice to handle the error when it occurs and try not to let it float all the way to the top of the stack and cause multiple other errors.

Hope this helps these were just my quick thoughts from reading your question.

Answer 2

I would recommend you inheriting from the HandleError attribute and rolling your exception handling in there. Overriding the OnException on a single controller means you either have a lot of exception handling code in a lot of controllers or your inherit from a base one, which due to the MVC pipeline is not really necessary in either case.

By using the attribute, you should have one occurrence of an error per action executed, and once the error is handled it won't fire again. Hopefully this will cut down on repeat exception messages.

I personally use attributes for the exception handling cause it's cleaner and more reusable and get's rid of a lot of noise within my actions.