8

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)

Julian
  • 33,915
  • 22
  • 119
  • 174
Patrick
  • 2,995
  • 14
  • 64
  • 125
  • 1
    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 Dec 05 '15 at 04:35
  • Hi thanks, is this make sense to do? or can I do diferently? – Patrick Dec 05 '15 at 04:36
  • Yes, that's standard logging in ASP.NET if I remember correctly – Camilo Terevinto Dec 05 '15 at 04:37
  • Maybe I could redirect to the Error view instead of rethrow the error, no? – Patrick Dec 05 '15 at 04:38
  • 2
    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 Dec 05 '15 at 04:39
  • Yes but at this time I had received the error detail by email, so I can move on with sending the feedback to the user – Patrick Dec 05 '15 at 04:40
  • @Patrick...You can create a custom attribute that handles your errors in one place. If you're interested, I'll post an example. – Big Daddy Dec 07 '15 at 12:20
  • @BigDaddy Hi, Thanks. And will I be able to log aditional information regarding the action request like client id? – Patrick Dec 07 '15 at 12:22

3 Answers3

6

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.

Mike Nakis
  • 56,297
  • 11
  • 110
  • 142
  • Hi thanks! And how do you catch errors in production environment? I'm trying to create a shortcut so when I receive a email with the error from NLog, I know exactly which clientId for example I have to debug. – Patrick Dec 14 '15 at 14:21
  • 1
    I suppose that if you decide to catch, log, and rethrow, then you can probably include the clientid in the logged text, or you can do anything else you want to do instead of logging. For example, you can add the client id to some table in the database. But all this is unrelated to whether you will redirect to an error page. I am suggesting that once you have recorded whatever information you need to record, then instead of doing your own redirection you should just rethrow and let the server handle the exception depending on how it has been configured. – Mike Nakis Dec 14 '15 at 14:45
  • And "rethrowing" the error is a good practice in your opinion? or you would do differently? How can I rethrow the error in my case? Thanks – Patrick Dec 14 '15 at 14:47
  • 1
    Yes, rethrowing the error is a very good practice, and C# even supports special syntax for it. Check it out: http://stackoverflow.com/questions/881473/why-catch-and-rethrow-an-exception-in-c – Mike Nakis Dec 14 '15 at 14:52
  • So I just do "throw;" after logging the error, correct? What do you thing about Duncan's comment "And yes, the try...catch is doing nothing useful (apart from lose the call stack - so it's actually worse - unless for some reason you didn't want to expose this information)." ? – Patrick Dec 14 '15 at 14:54
  • 1
    Yes, throw after logging the error. If you really care about the stack trace included with the original exception, (and you usually do,) Duncan suggests to throw a new exception and pass the old exception as an argument to it. – Mike Nakis Dec 14 '15 at 14:57
  • 1
    Thanks Mike for your help! I have check your help has correct ;) Thank you. – Patrick Dec 14 '15 at 14:59
1

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

  }
}
Dipitak
  • 97
  • 1
  • 1
  • 3
  • Hi thanks, that's the solution I have right now in the project, but it came with a problem, I never get in development environment the "Yellow page of death" because I always redirect. I always have to check the email or log to get details regarding the error :( – Patrick Dec 09 '15 at 14:32
0

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());
Big Daddy
  • 5,160
  • 5
  • 46
  • 76
  • Hi, thanks! But this means that I can only log one value and if I want to log 2 values I need another attribute? – Patrick Dec 07 '15 at 19:33
  • Based on a string build based on several values? – Patrick Dec 08 '15 at 02:06
  • You can log anything from the request or create additional properties on the custom exception attribute and hydrate them from the annotation. – Big Daddy Dec 08 '15 at 12:01
  • And the "SomeValue" can access values inside the action like a clientId that passes has a parameter? – Patrick Dec 08 '15 at 12:44