Let's build an error handling aspect: part 7
I'm starting to wind down this blog post series. A couple more tweaks and you'll be all caught up with how I'm using this aspect in a system that I'm actively working on right now.
But first, let me address some of the criticism that I've received in comments, reddit, and so on.
Mainly, the criticism about my aspect making the return value be an empty collection (instead of null) if the return type is a collection. I totally expected this criticism, and I definitely think it's fair. I pondered this idea a lot myself before I came to the conclusion that I did, and it's entirely possible that I'll change my mind later. So, if you are following along, and thinking about creating an aspect like this, you should definitely think carefully about doing this; don't just copy and paste my aspect and call it a day.
I think it comes down to a choice: do I want to always be checking for nulls, or when there's a collection can I just assume that it will (at worst) be empty? I based my decision somewhat on the Framework Design Guidelines - my API should return an empty collection (or array) instead of nulls. My thought process was also that this aspect is responsible for the return value being null in the first place (because it's handling the exception and not bubbling it up), and therefore it should make a reasonable attempt at not disrupting normal program flow by returning an empty collection.
This is an opinionated approach, and it may not mesh with what you are trying to do, you or your team's coding style, and where you intend to use this aspect.
Okay, on with the show.
The aspect I've written is able to send an error message up through the stack and eventually to the UI (in my application right now, that's either MVC ModelState or a display element in my Telerik Reporting reports, but it could easily be other UI elements too). Here's the relevant bit of code:
While I'm developing, that's great. I get the exception message and it helps me pinpoint the error. There's no stack trace, but you could add that too if you wanted (or check it out on Exceptionless logs). But there's a problem: I do not want my users (especially any malicious ones) to see detailed error messages like that. So, just add in some configuration and logic to show (or not show) a verbose error message. It's a piece of cake, and it really isn't something that's specific to AOP (other than to show that AOP code can be configured just like any other part of your code). I also want to refactor this to only use Exceptionless when in production: I don't need to log any exceptions that I create locally when I'm in the process of writing code.
Notice that I kinda hand-waved the configuration stuff away into the staic class/method IsThisSite.RunningLocally? The RunningLocally code is simply a check of Web.config (or App.config) using ConfigurationManager.AppSettings. How you manage your configurations is up to you and your team.
Also notice that GetErrorMessage could be customized to show different error messages depending on the exception (or not).
Okay, so I think this aspect is in pretty good shape. It's helpful, cuts down on boilerplate, and implements a very important cross-cutting concern requirement (error handling/logging). But, a) you shouldn't have to take my word for it that it works, and b) what if I want to refactor it? It should be unit tested (which is the next and final post).
