An incremental way of improving exception throwing in C#
There is one basic functionality of all main programming languages: throwing exceptions. Determining in code that something is wrong, one throws an exception of a certain type with extra messages and values. The problem this solves is breaking an execution flow that has entered an invalid state and being aware of what happened. Traditionally, errors are then caught in higher levels of the application and decisions are made: ignore the error, log it, encapsulate it into another exception with extra information, throw it as it is after some cleanup, etc.
But as the joke goes, now you have two problems. When developing .NET code you have to ask yourself what type of exception you are going to throw, what data to add to it and think of what will catch it above and how will it interpret what you sent. Some people create a different exception type for each little issue, in view of the multiple catch(SpecificExceptionType) functionality, so they can choose later what to do at a higher level. Others try to use the out of the box Microsoft exception types, a clear case of stuffing square pegs in round holes. Inevitably someone will just give up in frustration and throw a new Exception("Something went wrong!"); and be done with it. And recently, in order to solve the problems with the above approaches, I envisioned (with full documentation and implementation) a dependency injected IExceptionFactory which I thought was the greatest invention since fire only to discover it was so unwieldy to use that I despaired and deleted the entire thing.
Discussing with friends about this deceptively complicated problem, I think I found a solution that covers all major scenarios. But before doing that, I can just feel that some of you thought "Hey! I am doing that and there is nothing wrong with it!", so let's discuss what's wrong with the approaches above. If you want to skip this, go to The Solution section.
Extending Exception is not simple. There are four constructors and I dare you to say out the top of your head what Exception(SerializationInfo, StreamingContext) is and where it is used. There are numerous code analyzers that spew a lot of warnings about how Exceptions should be implemented correctly. That's another story: here is a nice article about it. More importantly, doing all this work for every possible exception takes time and effort and duplicated code. In the end, you will get to the next scenario, but with a larger set of hole shapes.
Also, the try/catch block in C# 6.0 has been updated with the catch when syntax, so you can have multiple catch blocks with the same Exception type and different conditions.
If you get an empty string from a method and you expected something there, you should throw an exception, but which one? The value is not null, so ArgumentNullException is kind of not applicable. Is ArgumentOutOfRangeException better? I mean, empty string is not in the range of accepted values for the parameter, maybe it could be it. Or is it just ArgumentException? You decide on ArgumentException and you smugly add the name of the variable with nameof(yourLocalVariable), because you are knowledgeable in the ways of code... and you get a warning that yourLocalVariable is not the name of any parameter of the method you are in. That's right, the value was invalid, but ArgumentExceptions are used specifically for the current method arguments.
You don't want to use multiple custom exception types, because you've read this post and abandoned it half way, but you agreed with the first point. Or maybe you are just lazy. You ignore the warning, you use ArgumentException anyway. Later on you are reading the logs and are trying to remember where in the code you used yourLocalVariable and why does it matter it's empty.
Admit it, the Microsoft exception types were not really meant to help you throw exceptions, they are there for Microsoft's internal code and use. Most of the few cases when the exception type is spot on are probably not what the makers of that exception type envisioned when they made it.
You are done with pointless standards. You just use throw new Exception($"This really specific thing happened with variable {yourVariable}"); and let God sort them out! You can use catch when to look into the string and parse it for information and make decisions on it. It actually works, you're rightly satisfied with yourself. You've showed them all how it's done. Boom! And then a junior developer comes along and decides your wording it not quite right for a native English speaker and changes the string. Suddenly everything literally goes boom, as exceptions get where they shouldn't and flows change unexpectedly.
After warning the entire team to never change the exception strings as they are used in the functionality of the application and you even consider creating a resource system for Exception strings so that it can be used for decision making regardless of content (and inevitably hate the way you need to store format strings and remember what value goes where), a member of the UI team comes and says "Hey, I need to get the reason the flow failed to the user. And I need to translate it to their language". And you despair.
A slight variation on all of the points above, this involves creating only one type of custom exception, add to it whatever is needed to determine flow, string resource ids, etc. This is actually a pretty decent idea, as it puts the control back into the developer's hands. Why depend on Microsoft types or parsing strings. Context is for kings and you are a king amongst kings.
However, whenever you want to change something, like add a value to an enum that defines the type of the exception, change the way in which a certain exception is handled, you have to change all the code that uses that exception. It's a single point of use, but not a single point of change.
Moreover, other devs in the team think it is cumbersome to work with it. The exception type is stored in the basest of libraries and they all want to add something to it. It becomes bloated and soon enough it creeps into a huge mess that is handled differently in different code and is not easy to maintain, understand or use.
So why not use an exception factory? Everything else in your code works on the premise that "if you want something, you inject an ISomething in the constructor and worry about the implementation never". Why not inject IExceptionFactory everywhere where you need exceptions, then do something magic with it? The result of the operation is determined by the implementation, too. If you want another implementation, you just inject something else. It's genius!
Only then you have to use it. How do you inject the factory in static methods, extension methods, stuff so basic that it used as utilities classes all over the code and now you have to add an extra dependency to everything that uses those classes? Everybody hates you, hates having to add an extra constructor parameter, an extra field, then throw exceptions with something like throw _exceptionFactory.New("Something went wrong!",new ParameterEmptyExceptionData(nameof(localVariable), localVariable)); while adding a dependency to the logging library that the factory uses to log generated exceptions.
Oh, it's just crap!
Let's start from an existing piece of code: throw new ArgumentException("{localVariable} is null or empty");. Optimally, we would just want to change this code slightly to solve several issues:
A tall order, especially since we've already decided that we don't want to use the factory idea. Some of the issues above are also non-issues in most cases. What if I don't care about the language of the message or if it is a resource or not, it's something used internally in our code. localVariable is empty, I don't need its value. The context is clear from the Stack trace. The exception is meaningful enough as an ArgumentException. In other words, we need to solve one more issue: all of the issues above are optional.
The software pattern that covers this scenario and has been used extensively by library makers is the build pattern. For the sake of exploration, let's see how this would work:
This looks promising, considering that every method above is optional, except the builder instantiation and the build at the end, but it's still too close to the factory idea above. Why use new in a project that is based on dependency injection? Why use .Build() everywhere where you need to throw an exception. Where does the logger come from?
So here is the solution I am proposing, using several resources we have at our disposal in C#:
The code will look like this:
Each method above is an extension method on the Exception type. Any of them can decide to return the original object or a different one, but they all return an instance that extends Exception. The information attaching methods use the Data property to hold the information. The Build method is designed to take every information attached to an Exception and perform more complex actions, like logging or constructing a completely different object to be returned, however that step is also optional.
And here is the source for an ExceptionBuilder static class that acts as both container for the more common extension methods as well as the point where dependencies are being registered:
Note a few things:
Finally, there is one design decision that I am not sure about: to use throw exception, or to use exception.Throw()? The former is natural to all devs, but it needs special catch blocks to be able to resume execution; whatever the builder returns, it will always throw something. The latter needs a change in all code that throws exceptions, but it could handle the decision to whether to throw anything at all without recompile.
I lean on the first, just because changes in an existing code base can be done incrementally and the code can be understood by all devs, regardless of seniority.
I find this to be a wonderful idea, clear, useful and flexible. I hope you do, too!
But as the joke goes, now you have two problems. When developing .NET code you have to ask yourself what type of exception you are going to throw, what data to add to it and think of what will catch it above and how will it interpret what you sent. Some people create a different exception type for each little issue, in view of the multiple catch(SpecificExceptionType) functionality, so they can choose later what to do at a higher level. Others try to use the out of the box Microsoft exception types, a clear case of stuffing square pegs in round holes. Inevitably someone will just give up in frustration and throw a new Exception("Something went wrong!"); and be done with it. And recently, in order to solve the problems with the above approaches, I envisioned (with full documentation and implementation) a dependency injected IExceptionFactory which I thought was the greatest invention since fire only to discover it was so unwieldy to use that I despaired and deleted the entire thing.
Discussion
Discussing with friends about this deceptively complicated problem, I think I found a solution that covers all major scenarios. But before doing that, I can just feel that some of you thought "Hey! I am doing that and there is nothing wrong with it!", so let's discuss what's wrong with the approaches above. If you want to skip this, go to The Solution section.
Multiple Exception implementations
Extending Exception is not simple. There are four constructors and I dare you to say out the top of your head what Exception(SerializationInfo, StreamingContext) is and where it is used. There are numerous code analyzers that spew a lot of warnings about how Exceptions should be implemented correctly. That's another story: here is a nice article about it. More importantly, doing all this work for every possible exception takes time and effort and duplicated code. In the end, you will get to the next scenario, but with a larger set of hole shapes.
Also, the try/catch block in C# 6.0 has been updated with the catch when syntax, so you can have multiple catch blocks with the same Exception type and different conditions.
Using existing exception types
If you get an empty string from a method and you expected something there, you should throw an exception, but which one? The value is not null, so ArgumentNullException is kind of not applicable. Is ArgumentOutOfRangeException better? I mean, empty string is not in the range of accepted values for the parameter, maybe it could be it. Or is it just ArgumentException? You decide on ArgumentException and you smugly add the name of the variable with nameof(yourLocalVariable), because you are knowledgeable in the ways of code... and you get a warning that yourLocalVariable is not the name of any parameter of the method you are in. That's right, the value was invalid, but ArgumentExceptions are used specifically for the current method arguments.
You don't want to use multiple custom exception types, because you've read this post and abandoned it half way, but you agreed with the first point. Or maybe you are just lazy. You ignore the warning, you use ArgumentException anyway. Later on you are reading the logs and are trying to remember where in the code you used yourLocalVariable and why does it matter it's empty.
Admit it, the Microsoft exception types were not really meant to help you throw exceptions, they are there for Microsoft's internal code and use. Most of the few cases when the exception type is spot on are probably not what the makers of that exception type envisioned when they made it.
Using Exception and a meaningful message
You are done with pointless standards. You just use throw new Exception($"This really specific thing happened with variable {yourVariable}"); and let God sort them out! You can use catch when to look into the string and parse it for information and make decisions on it. It actually works, you're rightly satisfied with yourself. You've showed them all how it's done. Boom! And then a junior developer comes along and decides your wording it not quite right for a native English speaker and changes the string. Suddenly everything literally goes boom, as exceptions get where they shouldn't and flows change unexpectedly.
After warning the entire team to never change the exception strings as they are used in the functionality of the application and you even consider creating a resource system for Exception strings so that it can be used for decision making regardless of content (and inevitably hate the way you need to store format strings and remember what value goes where), a member of the UI team comes and says "Hey, I need to get the reason the flow failed to the user. And I need to translate it to their language". And you despair.
Using a single type of Exception that has everything you need
A slight variation on all of the points above, this involves creating only one type of custom exception, add to it whatever is needed to determine flow, string resource ids, etc. This is actually a pretty decent idea, as it puts the control back into the developer's hands. Why depend on Microsoft types or parsing strings. Context is for kings and you are a king amongst kings.
However, whenever you want to change something, like add a value to an enum that defines the type of the exception, change the way in which a certain exception is handled, you have to change all the code that uses that exception. It's a single point of use, but not a single point of change.
Moreover, other devs in the team think it is cumbersome to work with it. The exception type is stored in the basest of libraries and they all want to add something to it. It becomes bloated and soon enough it creeps into a huge mess that is handled differently in different code and is not easy to maintain, understand or use.
Another layer of indirection
So why not use an exception factory? Everything else in your code works on the premise that "if you want something, you inject an ISomething in the constructor and worry about the implementation never". Why not inject IExceptionFactory everywhere where you need exceptions, then do something magic with it? The result of the operation is determined by the implementation, too. If you want another implementation, you just inject something else. It's genius!
Only then you have to use it. How do you inject the factory in static methods, extension methods, stuff so basic that it used as utilities classes all over the code and now you have to add an extra dependency to everything that uses those classes? Everybody hates you, hates having to add an extra constructor parameter, an extra field, then throw exceptions with something like throw _exceptionFactory.New("Something went wrong!",new ParameterEmptyExceptionData(nameof(localVariable), localVariable)); while adding a dependency to the logging library that the factory uses to log generated exceptions.
Oh, it's just crap!
The Solution
Let's start from an existing piece of code: throw new ArgumentException("{localVariable} is null or empty");. Optimally, we would just want to change this code slightly to solve several issues:
- formalize that it is an argument empty exception
- make it clear it's localVariable that was empty
- maybe add the actual value of localVariable
- declare the context in which the exception was thrown
- declare the message that should be used in the exception
- throw a meaningful exception type
- decide if this exception should be ignored or thrown
- log the exception
- minimize developer effort
- minimize dependencies
- use a solution that is closed for modification, but open for extension
A tall order, especially since we've already decided that we don't want to use the factory idea. Some of the issues above are also non-issues in most cases. What if I don't care about the language of the message or if it is a resource or not, it's something used internally in our code. localVariable is empty, I don't need its value. The context is clear from the Stack trace. The exception is meaningful enough as an ArgumentException. In other words, we need to solve one more issue: all of the issues above are optional.
The software pattern that covers this scenario and has been used extensively by library makers is the build pattern. For the sake of exploration, let's see how this would work:
var exception = new ArgumentException("{localVariable} is null or empty");
var builder = new ExceptionBuilder(ex, logger)
.SetError(Error.EmptyValue)
.SetName(nameof(localVariable))
.AddValue(localVariable)
.SetOrigin("Getting the localVariable in order to save the world")
.SetMessageId(Messages.EmptyWorldNameWhenTryingToSaveIt)
.ShouldBeIgnored();
throw builder.Build(); // this also logs and returns an exception of a type the builder decides relevant
This looks promising, considering that every method above is optional, except the builder instantiation and the build at the end, but it's still too close to the factory idea above. Why use new in a project that is based on dependency injection? Why use .Build() everywhere where you need to throw an exception. Where does the logger come from?
So here is the solution I am proposing, using several resources we have at our disposal in C#:
- the Exception type has a Data Dictionary property for additional data
- extension methods can be defined in multiple places for the same type
- there is no need for an instance of a builder when throwing an exception or Build
The code will look like this:
throw new ArgumentException("{localVariable} is null or empty")
.SetError(Error.EmptyValue)
.SetName(nameof(localVariable))
.AddValue(localVariable)
.SetOrigin("Getting the localVariable in order to save the world")
.SetMessageId(Messages.EmptyWorldNameWhenTryingToSaveIt)
.ShouldBeIgnored()
.Build();
Each method above is an extension method on the Exception type. Any of them can decide to return the original object or a different one, but they all return an instance that extends Exception. The information attaching methods use the Data property to hold the information. The Build method is designed to take every information attached to an Exception and perform more complex actions, like logging or constructing a completely different object to be returned, however that step is also optional.
And here is the source for an ExceptionBuilder static class that acts as both container for the more common extension methods as well as the point where dependencies are being registered:
/// <summary>
/// Add data to exceptions, then build a Custom exception
/// using registered <see cref="IExceptionBuildHandler"/> and optional logging
/// </summary>
public static class ExceptionBuilder
{
private const string CustomPrefix = "Custom.";
private static readonly List<IExceptionBuildHandler> _handlers = new List<IExceptionBuildHandler>();
private static ICustomLogger _logger;
#region Extended Data
/// <summary>
/// Attaches a custom <see cref="Error"/> to the exception
/// </summary>
/// <param name="ex"></param>
/// <param name="error"></param>
/// <returns></returns>
public static Exception SetError(this Exception ex, Error error)
{
_logger?.LogTrace($"Setting error {error} in exception {ex}");
return ex.SetData(nameof(Error), error);
}
/// <summary>
/// Attaches an object as the exception origin to the exception
/// </summary>
/// <param name="ex"></param>
/// <param name="origin"></param>
/// <returns></returns>
public static Exception SetOrigin(this Exception ex, object origin)
{
_logger?.LogTrace($"Setting exception origin {origin} in exception {ex}");
return ex.SetData("origin", origin);
}
/// <summary>
/// Attaches a name parameter to the exception
/// </summary>
/// <param name="ex"></param>
/// <param name="name"></param>
/// <returns></returns>
public static Exception SetName(this Exception ex, string name)
{
_logger?.LogTrace($"Setting exception name {name} in exception {ex}");
return ex.SetData("name", name);
}
/// <summary>
/// Declare an exception as not breaking the execution flow.
/// Implement catch blocks for exceptions like this to support this scenario.
/// </summary>
/// <param name="ex"></param>
/// <returns></returns>
public static Exception TryToIgnore(this Exception ex)
{
_logger?.LogTrace($"Declaring exception {ex} as not breaking execution flow");
return ex.SetData("tryToIgnore", true);
}
/// <summary>
/// True if this exception is declared as not breaking execution flow
/// </summary>
/// <param name="ex"></param>
/// <returns></returns>
public static bool ShouldBeIgnored(this Exception ex)
{
return object.Equals(ex.GetData("tryToIgnore"), true);
}
/// <summary>
/// Attaches a type to the exception
/// </summary>
/// <param name="ex"></param>
/// <param name="type"></param>
/// <returns></returns>
public static Exception AddType(this Exception ex, Type type)
{
_logger?.LogTrace($"Attaching type {type} in exception {ex}");
return ex.AddData("types", type);
}
/// <summary>
/// Attaches a value to the exception
/// </summary>
/// <param name="ex"></param>
/// <param name="value"></param>
/// <returns></returns>
public static Exception AddValue(this Exception ex, object value)
{
_logger?.LogTrace($"Attaching type {value} in exception {ex}");
return ex.AddData("values", value);
}
/// <summary>
/// Gets data from exception based on key.
/// Returns null if not found.
/// </summary>
/// <param name="ex"></param>
/// <param name="key"></param>
/// <returns></returns>
public static object GetData(this Exception ex, string key)
{
key = $"{CustomPrefix}{key}";
if (ex.Data?.Contains(key)!=true)
{
return null;
}
return ex.Data[key];
}
/// <summary>
/// Attaches an object to exception data replacing any previous one with the same key
/// </summary>
/// <typeparam name="T"></typeparam>
/// <param name="ex"></param>
/// <param name="key"></param>
/// <param name="value"></param>
/// <returns></returns>
public static Exception SetData<T>(this Exception ex, string key, T value)
{
key = $"{CustomPrefix}{key}";
var result = ex.AsCustomException();
ex.Data[key] = value;
return result;
}
/// <summary>
/// Adds an object to a list that resides in the exception data at the given key
/// </summary>
/// <typeparam name="T"></typeparam>
/// <param name="ex"></param>
/// <param name="key"></param>
/// <param name="value"></param>
/// <returns></returns>
public static Exception AddData<T>(this Exception ex, string key, T value)
{
key = $"{CustomPrefix}{key}";
var result = ex.AsCustomException();
var alreadyExists = ex.Data.Contains(key);
if (!alreadyExists || !(ex.Data[key] is List<T> list))
{
if (alreadyExists)
{
_logger?.LogWarning($"Overwriting data {ex.Data[key]} with key {key} with an empty list of {typeof(T).Name} in exception {ex}.");
_logger?.LogWarning($"Are you using Add* and Set* builder methods at the same time or adding objects of different types?");
}
list = new List<T>();
ex.Data[key] = list;
}
lock (list)
{
list.Add(value);
}
return result;
}
#endregion Extended Data
/// <summary>
/// Builds the exception from the Data and the provided base exception
/// </summary>
/// <param name="ex"></param>
/// <returns></returns>
public static CustomException Build(this Exception ex)
{
var CustomException = ex.AsCustomException();
lock (_handlers)
{
for (var index = _handlers.Count - 1; index >= 0; index--)
{
var handler = _handlers[index];
var result = handler.Build(CustomException, _logger);
if (result != null)
{
CustomException = result.AsCustomException();
break;
}
}
}
_logger?.LogTrace($"Built exception {CustomException}");
return CustomException;
}
#region Registration
/// <summary>
/// Register an <see cref="IExceptionBuildHandler"/>. The last handler to be added will take precedence.
/// </summary>
/// <param name="handler"></param>
public static void RegisterBuildHandler(IExceptionBuildHandler handler)
{
lock (_handlers)
{
_logger?.LogTrace($"Registering exception build handler {handler}");
_handlers.Add(handler);
}
}
/// <summary>
/// Register a logger
/// </summary>
/// <param name="logger"></param>
public static void RegisterLogger(ICustomLogger logger)
{
_logger = logger;
_logger?.LogTrace($"Registered logger in the exception builder");
}
#endregion Registration
private static CustomException AsCustomException(this Exception ex)
{
return ex is CustomException CustomException
? CustomException
: new CustomException(ex);
}
}
Note a few things:
- All of the extension methods are returning the same object, with the exception of Build, which returns a CustomException that maybe writes the extra Data values in ToString
- The external dependencies are being registered via methods. In the class I use, I even replaced those methods with a RegisterServiceProvider method that sets up everything it needs, including the list of handlers, from dependency injection
- One doesn't need to call Build and every extension method just naturally continues a normal existing code like throw new WhateverException();
- When using Build, though, you can change the exception object that is being thrown just by injecting another instance of IExceptionBuildHandler
- In my project, I've devised a method of injecting code via a text configuration file. That means that you can change what happens when an exception if being thrown without recompiling your existing code.
Finally, there is one design decision that I am not sure about: to use throw exception, or to use exception.Throw()? The former is natural to all devs, but it needs special catch blocks to be able to resume execution; whatever the builder returns, it will always throw something. The latter needs a change in all code that throws exceptions, but it could handle the decision to whether to throw anything at all without recompile.
I lean on the first, just because changes in an existing code base can be done incrementally and the code can be understood by all devs, regardless of seniority.
I find this to be a wonderful idea, clear, useful and flexible. I hope you do, too!
Comments
Be the first to post a comment