Handle Error with NLog and Try Catch

8
votes

I log errors in my Actions using NLog to store errors with additional information, for example:

using NLog;

private static Logger _logger = LogManager.GetCurrentClassLogger();

public virtual ActionResult Edit(Client client)
{
  try
  {
        // FORCE ERROR
        var x = 0;

        x /= x;

        return RedirectToAction(MVC.Client.Index());
  }
  catch (Exception e)
  {
    _logger.Error("[Error in ClientController.Edit - id: " + client.Id + " - Error: " + e.Message + "]");
  }
}

And I have Error handling configured in Web.config:

<customErrors mode="On" />

But I don't get redirected to the Error.cshtml when I execute the Action (the page remains in the same place), why?

Can I use Elmah to do the same thing? (logging additional information like client Id)

3
c#asp.net-mvcerror-handlingnlogelmah
Very simple, you don't get redirected to error.cshtml because, for the ASP.NET pipeline, no error occurred. You'll have to rethrow the exception or throw a new one. - Camilo Terevinto
Hi thanks, is this make sense to do? or can I do diferently? - Patrick
Yes, that's standard logging in ASP.NET if I remember correctly - Camilo Terevinto
Maybe I could redirect to the Error view instead of rethrow the error, no? - Patrick
That's another option. But, if you redirect you'll get a status code of Redirect; if you throw an error, you'll get a 50x (can't remember the exact number, I think it's 500 but too sleepy) - Camilo Terevinto

3 Answers

4
votes

First of all, most people solve this error by not catching the exception. This way, the exception propagates to ASP.NET, which displays a "500 Internal Error" webpage, and all the pertinent information is logged.

  • If your server is configured for production, the error page will just say "an error occurred, details were logged."

  • If the server is configured for development, then you will get the famous yellow page with the exception type, the message, and the stack trace.

Swallowing the exception and manually redirecting to an error page is a bad practice because it hides errors. There are tools that examine your logs and give you nice statistics, for example about percentages of successful/failed requests, and these won't work any more.

So, not swallowing the exception is what people do, and at the very least, it solves your problem.

Now, I find this very clunky, because I do not like manually looking for the source files mentioned in the yellow page and manually going to the mentioned line numbers. I practically have no use for the yellow page, it might just as well just say "an error occurred, cry me a river, nah-nah-nah." I don't read the yellow page.

Instead, I do like to log exceptions on my own, and I have my logger begin each line with full-path-to-source-filename(line):, so that every line on the debug log in visual studio is clickable, and clicking on a line automatically opens up the right source file, and scrolls to the exact line that issued the log message. If you want this luxury, then go ahead and catch the exception, but right after logging the exception you have to rethrow it, so that things can follow their normal course.

Amendment

Here is some information that was added in comments:

So, you can do the following:

try
{
   ...
}
catch (Exception e)
{
    log( "information" );
    throw; //special syntax which preserves original stack trace
}

Or

try
{
   ...
}
catch (Exception e)
{
    throw new Exception( "information", e ); //also preserves original stack trace
}

Do not do this: catch( Exception e ) { log( "information" ); throw e; } because it loses the original stack trace information of e.

1
votes

In your code, error occur at the division portion(x/=x) so no execution of redirect line(index page) and jump to catch portion executing the logger. You have to define the redirect to Error.cshtml in catch portion also.

Note: when you use try catch block error will not occur at ASP.NET level resulting no redirect to Error.cshtml page 

using NLog;

private static Logger _logger = LogManager.GetCurrentClassLogger();

public virtual ActionResult Edit(Client client)
{
  try
  {
        // FORCE ERROR
        var x = 0;

        x /= x; /// error occur here

        return RedirectToAction(MVC.Client.Index()); /// no execution of this line
  }
  catch (Exception e)
  {
    _logger.Error("[Error in ClientController.Edit - id: " + client.Id + " - Error: " + e.Message + "]");
    /// add redirect link here 
     return RedirectToAction(MVC.Client.Error()); /// this is needed since the catch block execute mean no error at ASP.net level resulting no redirect to default error page

  }
}
0
votes

This will streamline your exception handling and allow you to manage the process more succinctly. Create an attribute like this:

 public class HandleExceptionAttribute : System.Web.Mvc.HandleErrorAttribute
    {
        // Pass in necessary data, etc
        private string _data;
        public string Data
        {
            get { return _data; }
            set { _data = value; }
        }

        public override void OnException(System.Web.Mvc.ExceptionContext filterContext)
        {            
            // Logging code here
            // Do something with the passed-in properties (Data in this code)
            // Use the filterContext to retrieve all sorts of info about the request
            // Direct the user

            base.OnException(filterContext);
        }
    }

Now you can use it on a controller or method level with an attribute like this:

[HandleException(Data="SomeValue", View="Error")]

Or, register it globally (global.asax) like this:

GlobalFilters.Filters.Add(new HandleExceptionAttribute());