user3265963
user3265963

Reputation: 273

Logging Visual Studio MVC5

I am working on a project that is a web application. The application should have a single button on the page that redirects the user back to the same page. In 1 of every 5 (or thereabout) occasions when you press the button the program should throw some exception and log it.

It should catch the exception and do three things: Send an email, write the error in a file and write it to debug window.

My controller so far:

 public class HomeController : Controller
    {
        public ActionResult Index()
        {
            try
            {
                RedirectToAction("Index");
            }
            catch (ArgumentException e)
            {

            }
            Random RandNumber = new Random();
            int rand = RandNumber.Next(1000);
            if(rand % 5 == 0)
            {
                throw new System.ArgumentException("This is a random excpetion");
            }
            return View();
        }

    }

The idea is the to have the class Logger that declares a collection of the class LogMedia and loops through all the instances.

Class Logger:

  public class Logger
    {
        List<LogMedia> m_loggers = new List<LogMedia>();
        void LogException(Exception ex)
        {
            foreach(var i in m_loggers)
            {
                //Loop through all Log Media instances
            }
        }

Class LogMedia

    class LogMedia
    {
        public virtual void LogMessage(string Message); //virtual function that doesen't do anything

    }
    class OutputWindowLogMedia:LogMedia
    {
        public override void LogMessage(string Message)
        {
            System.Diagnostics.Debug.WriteLine(Message);
        }
    }
    class TextFileLogMedia : LogMedia
    {
        public override void LogMessage(string Message)
        {
            File.AppendAllText("c:\\Temp\\Log.txt", Message);
        }
    }
    class EmailLogMedia : LogMedia
    {
        public override void LogMessage(string Message)
        {
            //send email
        }

    }
}

My questions to you are the following:

Will my controller work as it stands now with what I am trying to implement? I am especially skeptical of my try catch..

What is the best way to loop though these three instances?

Do I have the right class declerations for Log Media, i.e. should I create new classes(that inherits LogMedia) for each instance and then a function that overrides the virtual function in LogMedia.

Upvotes: 0

Views: 234

Answers (1)

maf748
maf748

Reputation: 788

It's unclear if you want this behavior on this particular action, the controller, or the entire app. That said, unless there is some specific recovery code you want to build into your logic, I wouldn't pollute my action code with the try-catch.

There are two options the MVC framework provides to handle errors:

First, you can override OnException in a specific controller:

protected override void OnException(ExceptionContext filterContext)
{
    // do your logging here
    // set this flag if you want to stop the exception from bubbling
    filterContext.ExceptionHandled = true;
}

Second, you can create an error handling filter:

public class MyExceptionFilterAttribute :
    System.Web.Mvc.FilterAttribute, 
    System.Web.Mvc.IExceptionFilter
{
    public void OnException(System.Web.Mvc.ExceptionContext filterContext)
    {
        // same error handling logic as the controller override
    }
}

A filter can either be added to the global filters list, or applied to an action method like this:

[MyExceptionFilter]
public ActionResult Index()
{
}

Edit: forgot to mention your logger structure. Your approach of having Logger loop over multiple instances of LogMedia is a good, common approach to supporting multiple logging mechanisms with one common interface (log4net appenders for example). That said, have you considered using an existing, proven framework for your logging needs? You get a thoroughly tested framework to run, and a skill that will carry over to future endeavours.

Edit2: after your comment, I took a closer look at your code instead of your exception trapping approach. In your Index action, you're redirecting to Index with no condition checking. This is going to end up as a constant redirect loop (until IIS stops you). Since this is an assignment, I don't want to give too much away, but you will need some logic on the server side to detect a button click and redirect back to your Index. Consider another Index action method that accepts HttpPost (your current action would be the HttpGet handler).

Upvotes: 1

Related Questions