3
\$\begingroup\$

I have this C# production code for handling errors on a web application. There are currently two applications that use the class. In each application I initialize the class within the Page_Init method in a base class that is used on every page. Some methods are used to actually display the message whereas other methods will just email the message.

I wonder if maybe I should refactor it into two separate classes and/or use an interface to improve its scalability. Any thoughts?

using System;
using System.Web;
using System.Web.Configuration;
using System.Web.UI.HtmlControls;
using System.Data.Entity.Validation;

/// <summary>
/// Handles displaying messages and sending notifications
/// </summary>
public class ExceptionHandler
{
    private SessionManager session;
    private EmailProvider email;
    private EnvironmentProvider environmentProvider;
    private AppLocations appLocation;

    public ExceptionHandler(SessionManager userSession = null)
    {
        environmentProvider = new EnvironmentProvider();
        appLocation         = environmentProvider.GetLocation();
        session             = userSession;
        email               = new EmailProvider(session);
    }

    public void ShowMessage(Exception ex, HtmlGenericControl errorPanel, HttpRequest request, HttpServerUtility server, string messageKey = "")
    {
        SetMessage(errorPanel, server, messageKey);
        if (ex.IsNotNull())
        {
            email.SendExceptionDetails(GetEmailDetails(ex, request));
        }
    }

    public void ShowSessionMessage(HtmlGenericControl errorPanel, HttpRequest request, HttpServerUtility server, MessageKeyGroup sessionMessageKeys)
    {
        bool isGeneric = false;
        string message = sessionMessageKeys.MessageKey;
        if (message.IsNullOrEmpty())
        {
            message   = GetHtmlDecodedMessage(server, "GenericErrorMessage");
            isGeneric = true;
        }

        string loginUrl = GetLoginURL();
        if (loginUrl.IsNullOrEmpty())
        {
            email.SendExceptionDetails(GetEmailDetails(new NullReferenceException("Login URL is null."), request));
            message = GetHtmlDecodedMessage(server, isGeneric ? "GenericErrorMessageWithoutButton" : sessionMessageKeys.AlternateMessageKey);
            errorPanel.InnerHtml = GetHtmlDecodedMessage(server, message);
            errorPanel.Visible   = true;
            return;
        }
        errorPanel.InnerHtml = String.Format(GetHtmlDecodedMessage(server, message), loginUrl);
        errorPanel.Visible   = true;
    }

    public void ShowExistingClaimMessage(HtmlGenericControl errorPanel, HttpRequest request, HttpServerUtility server, string messageKey)
    {
        errorPanel.InnerHtml = GetHtmlDecodedMessage(server, messageKey);
        errorPanel.Visible = true;
    }

    public void SendExceptionInfo(Exception ex, HttpRequest request)
    {
        if (ex is DbEntityValidationException)
        {
            DbEntityValidationException dbEx = (DbEntityValidationException)ex;
            email.SendExceptionDetails(GetEFValidationExceptionDetails(dbEx, request));
            return;
        }
        email.SendExceptionDetails(GetEmailDetails(ex, request));
    }

    public void SendExceptionInfo(Exception ex)
    {
        email.SendException(ex);
    }

    private ExceptionEmailDetails GetEmailDetails(Exception ex, HttpRequest request)
    {
        string serverName = request.Url.GetLeftPart(UriPartial.Authority);
        string serverHost = request.ServerVariables["REMOTE_HOST"];
        return new ExceptionEmailDetails()
        {
            SystemException = ex,
            ServerName = serverName,
            ServerHost = serverHost
        };
    }

    private ExceptionEmailDetails GetEFValidationExceptionDetails(DbEntityValidationException ex, HttpRequest request)
    {
        string msg = string.Empty;
        foreach (var validationErrors in ex.EntityValidationErrors)
        {
            foreach (var validationError in validationErrors.ValidationErrors)
            {
                msg += String.Format("Property: {0} Error: {1} {2}", validationError.PropertyName, validationError.ErrorMessage, Environment.NewLine);
                //Debug.WriteLine(msg);
                //Trace.TraceInformation(msg);
            }
        }
        ExceptionEmailDetails details = GetEmailDetails(ex, request);
        details.BodyPrefix = msg;
        return details;
    }

    private void SetMessage(HtmlGenericControl errorPanel, HttpServerUtility server, string companyName, string messageKey = "")
    {
        string message = GetHtmlDecodedMessage(server, "ClaimErrorMessage");
        if (messageKey.IsNotNullOrEmpty())
        {
            message = GetHtmlDecodedMessage(server, messageKey);
        }
        if (companyName.IsNull())
        {
            message = GetHtmlDecodedMessage(server, "GenericPolicyErrorMessage");
        }
        errorPanel.InnerHtml = companyName.IsNull() ? message : String.Format(message, companyName);
        errorPanel.Visible = true;
    }

    public string GetHtmlDecodedMessage(HttpServerUtility server, string messageKey)
    {
        return server.HtmlDecode(WebConfigurationManager.AppSettings[messageKey]);
    }

    private string GetLoginURL()
    {
        switch (appLocation)
        {
            case AppLocations.Development:
                return WebConfigurationManager.AppSettings["LoginURLforDev"];
            case AppLocations.Staging:
                return WebConfigurationManager.AppSettings["LoginURLforStaging"];
            case AppLocations.Production:
                return WebConfigurationManager.AppSettings["LoginURLforProd"];
        }
        return string.Empty;
    }

    public void SendErrorMessage(string subject, string body)
    {
        email.SendMessage(body, subject, null, null);
    }
}
\$\endgroup\$
  • \$\begingroup\$ You could use Elmah or NLog from Nuget. It stored to DB , sneds emails and Elmah streamlines nicely in MVC.. and even WebForms. \$\endgroup\$ – Piotr Kula Oct 24 '14 at 17:45
  • \$\begingroup\$ I know about ELMAH and plan to integrate it into our architecture at some point in the future, but that wont happen anytime soon. Thanks for the suggestion. \$\endgroup\$ – Necromancer Oct 28 '14 at 18:31
2
\$\begingroup\$
  1. Your class has implicit dependencies which should be passed in because otherwise unit testing can become painful. Those are EnvironmentProvider and EmailProvider.

    Now while looking at these it becomes obvious that EnvironmentProvider is purely used to retrieve the appLocation and it should therefore be removed as class member.

    Checking further it seems that appLocation is purely used to obtain a loginUrl. Hence the class can be simplified by just passing the login URL via the constructor and let something else worry about to obtain it. This removes a concern from the class which it really shouldn't have to deal with. You could even make that a parameter to ShowSessionMessage which is the only method which cares about it and remove that dependency from the class all together.

  2. This code seems slightly weird:

    message = GetHtmlDecodedMessage(server, isGeneric ? "GenericErrorMessageWithoutButton" : sessionMessageKeys.AlternateMessageKey);
    errorPanel.InnerHtml = GetHtmlDecodedMessage(server, message);
    

    Looks like it decodes the message twice. It might not matter or might even be required but still looks strange.

\$\endgroup\$
  • \$\begingroup\$ Thanks Chris, your comments were very helpful as they forced me to look at some possible issues. Your suggestions for improvement sound logical. \$\endgroup\$ – Necromancer Oct 28 '14 at 18:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.