Historically, GDNet's error tracking has been... well, basically, if there's an error showing up, we've relied on one of the staff noticing it, or somebody posting about it on the forum, or somebody telling me about it on IRC. That's a decidedly poor situation, and something I've been determined to improve in V5.
Loosely speaking, errors can be split into one of two categories. There are errors that are 'user-facing,' that is, errors that the user has caused or should be aware of, such as getting their password wrong when logging in. Then there are errors which are internal, which arise from incorrect or incomplete code, and that the user should never see. (Or more accurately, that should never happen). The former category need to be reported back to users in neatly formatted ways, with appropriate annotations - for example, the "Incorrect Password" error needs to also display the 'Forgot your password?' link. The latter need to be recorded for investigation by the dev team in our bug tracker, JIRA, and the user should generally not see any details.
Errors under .NET are generally represented as exceptions, which is eons ahead of how error handling was performed under classic ASP. An exception can be any arbitrary object, it can pack in as much data as is useful to describe the problem, and it can be passed around fairly arbitrarily, caught and rethrown at will.
Now, not every exception is necessarily an error that needs to be reported to the user or logged - when an exception is thrown it's entirely possible that the code is about to catch it and deal with it. Exceptions that are thrown, caught, and handled purely internally are not interesting to us. The only exceptions we're interested in are those that have bubbled up out of that internal code, and are going to be passed through code that is not going to handle them. So, this presents us with the first question: at what point has an exception bubbled up far enough to become 'noteworthy?'
The neatest place to draw the line is at the service boundaries. Any exception that reaches the service boundary for any of the web-facing services will, at the very least, need to be neatly formatted for the user. The service boundaries of internal services are less mandatory but they're still convenient, especially due to the way WCF works. Specifically, WCF allows you to install error handler modules that catch any exceptions thrown by service methods - i.e. exceptions that reach the service boundary.
We still have to answer the second question: how do we differentiate between user-facing errors and internal ones? This, ultimately, is something that can only be answered by the code that is throwing the exception. The exception needs to be 'stamped' as user-facing, and it needs to carry that stamp throughout its lifetime.
There are two approaches to this: one, we could insist that all user-facing exceptions derive from a UserFacingException class. That's reasonably nice, but means that we can't reuse any of the standard framework exceptions. The other is that we use the Exception.Data property, which is an opaque map of strings to objects. By picking a suitably obscure key - such as "dontLogToJira" - we can store our stamp in that dictionary. Add a couple of extension methods for convenience:
public static class DontLogToJiraExtension{ public const string ignoreKey = "dontLogToJira"; public static Exception DontLogToJira(this Exception e) { e.Data[ignoreKey] = true; return e; } public static bool ShouldLogToJira(this Exception e) { return e.Data == null || !e.Data.Contains(ignoreKey); }}and then whenever we throw an exception, we can stamp it very quickly and easily:
throw new BadCredentialsFault().DontLogToJira();Now that our exceptions are suitably categorized and we've decided where our error handlers will be, we can move on to looking at the error handlers themselves.
A WCF error handler is simply one that supports the IErrorHandler behavior. IErrorHandler requires two methods: HandleError, which is given the exception and returns a boolean to indicate whether WCF should continue passing the exception to other handlers, and ProvideFault, which is used to transform the fault message in whatever way you see fit. Ideally WCF will send the error back to the client somehow, so it wraps it up in a response message; by altering this message, we can control exactly what is sent back to the client. That's how we'll do our user-facing errors, but more on that in a minute.
The log-to-JIRA handler does all of its work in HandleError, and it's fairly straightforward. Behavior of note:
public bool HandleError(Exception error){ // Use the extension method to test that the exception hasn't been stamped if (ShouldIgnoreException(error)) return true; // Chop the stack trace down to remove all the WCF parts. IEnumerable trace = error.StackTrace.Split('\n').Reverse().SkipWhile((s => !s.Contains(CallstackFilter))).Reverse(); // If there are no GDNet parts left, don't log the exception. if (trace.Count() == 0) return true; /* ... */ // Work out a title for this report, using a hash of the stack trace. // We'll then test that an issue by this title doesn't already exist; // that should stop the same exception from getting loads of duplicate // reports. string issueTitle = string.Format("{0} {1} ({2:X})", exceptionTypeName, trace.First(), exceptionHash); /* ... */ // Allow other handlers to do their thing. return true;}The code that does the actual creation of the issue in JIRA just uses error.ToString(), so it contains all the data available. One of the ways I'm considering extending this later will be to have already-reported exceptions get filed as comments on the existing exceptions; presently they're just discarded. We'll see; hopefully it won't be necessary.
Out of interest, the LogExceptionsToJira class implements not only the IErrorHandler interface, but also the IServiceBehavior interface. Simply creating an error handler object isn't enough; you also need to bind it to the appropriate channel dispatchers, and using a service or endpoint behavior is the easiest way of doing this. It allows me to add or remove the error handler just by editing the XML config file for the service.
Let's look at the other error handler, the RenderWebVisibleExceptions handler. This handler does nothing in its HandlerError method, instead focusing on the ProvideFault method. Its intention is to generate an HTTP message containing a pretty XHTML page. Obtaining the XHTML is the interesting part - the rest is just WCF wrapping - so we'll focus on that.
So, here's a funny thing. When you see an error page, it should obey the same rules as the rest of the site, right? It should be using your custom theme, and display the correct data about who's logged in and so on. To do this, we actually use the full Renderer service that gets used for regular pages; it takes care of things like theming and session-specific data. And if the renderer throws an exception while rendering the exception page? You'll get the wonderful and simple error page that says "An error occurred when trying to generate an error message."
Rendering these generic errors is very simple:
var xml = new StringBuilder();XmlWriter wr = XmlWriter.Create(xml);wr.WriteStartElement("defaultLayout", "https://www.gamedev.net/");wr.WriteAttributeString("title", "An error has occurred");{ wr.WriteElementString("p", "http://www.w3.org/1999/xhtml", "Sorry, an internal error has occurred in the site; it has been recorded."); wr.WriteStartElement("p", "http://www.w3.org/1999/xhtml"); { wr.WriteString("If you like, you could report it to the webmaster at "); wr.WriteStartElement("a", "http://www.w3.org/1999/xhtml"); wr.WriteAttributeString("href", "mailto:webmaster@gamedev.net"); { wr.WriteString("webmaster@gamedev.net"); } wr.WriteEndElement(); wr.WriteString("."); } wr.WriteEndElement(); if (ShowExceptionDetail) { wr.WriteElementString("p", "http://www.w3.org/1999/xhtml", "Exception detail follows:"); wr.WriteElementString("p", "http://www.w3.org/1999/xhtml", error.ToString()); }}wr.WriteEndElement();wr.Close();using (var rc = new Rendering.RendererClient()){ return rc.Render(xml.ToString(), null);}I'll probably turn that into a resource sometime soon, instead of having it all get pushed through the XmlWriter.
Now, while that's all well and good for generic errors, there are sometimes errors that should have a more custom display. For example, when you get your password wrong, the 'Forgot your password' link. When you try browsing to the profile of a user who doesn't exist, maybe there should be a user search form alongside the 'can't find that user' message. For these cases, exceptions must implement the IWebVisibleException interface:
public interface IWebVisibleException{ HttpStatusCode HttpStatusCode { get; } string HttpStatusDescription { get; } String GetPresentableXhtml();}It's up to WebVisibleExceptions to figure out their own XHTML, though most of them will also use the renderer service. You can see that they're also free to specify their own HTTP status code and description - for example, the 'user not found' exception will return status 404 with description "User not found":
class UserNotFoundException : Exception, IWebVisibleException{ public string UserName { get; set; } public UserNotFoundException(string username) { UserName = username; } public string GetPresentableXhtml() { var xml = new StringBuilder(); var wr = XmlWriter.Create(xml); wr.WriteStartElement("defaultLayout", "https://www.gamedev.net/"); wr.WriteAttributeString("title", "User not found"); wr.WriteString(String.Format("User {0} could not be found.", UserName)); wr.WriteEndElement(); wr.Close(); using (var rc = new Rendering.RendererClient()) { return rc.Render(xml.ToString(), null); } } public HttpStatusCode HttpStatusCode { get { return HttpStatusCode.NotFound; } } public string HttpStatusDescription { get { return "User not found"; } }}That pretty much wraps up V5's error-handling architecture. I'm not 100% happy with how it's done - the way that exceptions get 'stamped' as user-facing rankles somewhat - but it'll do for now. The important thing is to keep going!
