Handling errors on a web application

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;

///
/// Handles displaying messages and sending notifications
///
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())

-
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.

-
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.

StackExchange Code Review Q#67733, answer score: 2