Prerequisites
To get the most out of this post, it's best if you are familiar with Object Oriented Programming, C# (or other C family language) and the basics of ASP.NET Core and MVC architecture.
If you're unfamiliar with any of these topics, but want to learn more here are some fantastic pluralsight courses:
If you don't have a pluralsight membership, they offer a 10-day free trial. If you want my two-cents: I've tried Treehouse, Udemy and others and Pluralsight is my clear winnner. Whether you're a code newbie or a seasoned engineer it's well worth the monthly fee.
Why I dislike dealing with exceptions
Throwing Exceptions is Dishonest
Consider the following code snippet:
public decimal SquareRootOf(int x)
{
if(x < 0)
throw new ArgumentException("x is negative");
// Calculate the square root
}
We see code like this all the time, and it's not a terrible practice. It's a good defensive programming practice to validate data that is passed into functions. But this function lied. It said it was going to return a decimal when passed an int, and it might, but it might also return an ArgumentException. There are a couple ways this code might be made more honest, but one appropriate fix is to be more honest about what type of parameters you can accept. If this function was declared that x was of type uint, then no check is needed and it can honestly say that yes, uint's always get decimals's back.
Exception Handling is a Poor Way to Control the Flow of Execution
Consider the following controller action for ASP.NET Core
[HttpDelete("{id}"]
public IActionResult DeletePost(int id)
{
try
{
var post = _postRepository.GetPostById(id);
var currentUser = _userService.GetCurrentUser();
if(post.Author.Id == currentUser.Id)
{
_postRepository.Delete(post);
return NoContent();
}
return Forbid();
}
catch (NotFoundException nfe)
{
return NotFound();
}
catch (AuthorizationException ae)
{
return Unauthorized();
}
catch
{
return StatusCodeResult(500);
}
}
Sure looks like resilient code. It checks for authorizations, returns custom exceptions that map nicely to the appropriate HTTP result. But it looks a bit jarring, and the control flow is routed mostly by exceptions. Additionally, we have some validation logic in here, and I prefer to keep the single responsibility of my controllers confined to orchestration.
Life Without Exception Handling
This is not a post about railway programming.
Imagine, if objects honestly told you they could return a few different types? What if a repository method could tell you "I'm going to return either a Post or an Exception"? Wouldn't that be nice.
Well there is a concept borrowed from functional programming, call railway oriented programming that is just that... but I won't be talking about it here. Personally, I've walked down this road before, and once you correctly deal with Asynchronous controllers... the readability (IMHO) just vanishes, and I suspect that C#, while multi-purpose, just isn't quite there yet to support paradigms so closely tied to functional programming. I prefer a more Object oriented approach. I will however leave this link for the interested.
Note: This post details railway oriented programming in F#, a proper functional language that is much more suited to this task than C#, but it explains the concept. For information on how it's done in C#, Zoran Horvat has this great course (yes, on pluralsight) about making your C# code more functional, and it has a module on railway oriented programming.
An old favorite, the Result class
I really have only one question when I ask some service method to do something, or get me something. Did it work? IF so, I've got the thing, and I can move on. IF something when wrong, I should be able to fail fast and return whatever error occurred further up the chain (As we are used to doing with exceptions)
There's an old pattern that I've used in many languages and frameworks, where a wrapper class holds both the data, and the success/failure information of the call.
Since we're confining ourself to the domain of ASP.NET Core controllers, we get a big break. Something that works can return the object we wanted. Something that fails can return an ActionResult. Since the class doing the actual work knows more about exactly what went wrong than the controller, it can return the correct ActionResult. This way, the controller is now relieved of the job of mapping exceptions to status codes, and can focus entirely on it's single responsibility of orchestration.
Let's look at some code, first a result implementation for void methods
public abstract class Result
{
public ActionResult ErrorResult { get; private set; }
public bool Errored => ErrorResult != null;
protected Result()
{
ErrorResult = null;
}
protected Result(ActionResult errorResult)
{
ErrorResult = errorResult;
}
private class Success : Result { }
private class Error : Result
{
public Error(ActionResult errorResult) : base(errorResult) { }
}
public static Result WithoutError => new Success();
public static implicit operator Result(ActionResult errorResult) => new Error(errorResult);
}
Let's unpack this a bit here.
First, I like making result class abstract because I don't want people using the constructor. There's a static factory property, and an implicit conversion we'll talk about later.
The one field of the Result is the ActionResult ErrorResult. Since ActionResult implements IActionResult and ASP.NET Core controllers usually return IActionResult, we can take any Result that Errored, and just return it's ErrorResult property.
Then I define two private implementations of Result: Success and Error. I don't want any users to actually know about these types, they are just implementations for me to use when I need to instantiate a Result object
Next we have a way to indicate the successful run of a void method a static property that returns a new instance of Success, encapsulated behind the Result type.
Then we have an implicit conversion from any ActionResult to a Result that instantiates a new Error (again, encapsulated)
So then we can return from "void" methods the following way
public Result DoSomething()
{
// Do Something
return Result.WithoutError;
}
public Result DoSomethingThatFails()
{
// Do Something That Fails
return new InternalServerError();
}
public Result DoSomethingThatWorks()
{
// Do Some Stuff
Result otherMethodResult = Other.MethodThatWorks();
// Fail Fast
if(otherMethodResult.Errored)
return otherMethodResult.ErrorResult;
// Do Some More Stuff
return Result.WithoutError;
}
So we see how to return without error, return ActionResult objects and have them converted to Result by the compiler, and easily return errors from other Result types. Works well for commands, but this doesn't give us data back from queries.
Getting something back, with Result
So what about methods that were supposed to return me some data? How do I get it? Well, we have another subclass of Result that can handle this
public class Result<T> : Result
{
public T Value { get; private set; }
private Result(T value)
{
Value = value;
}
private Result(ActionResult errorResult)
: base(errorResult) { }
public static implicit operator Result<T>(T value) => new Result<T>(value);
public static implicit operator Result<T>(ActionResult errorResult) =>
new Result<T>(errorResult);
}
So now I've added a field for the data we hope to return, and made the class generic so we can make results of any type. I've kept the constructors private because I want people to use the implicit conversions. So methods that return Result can either return the object of type T, or any ActionResult that represents an error.
Side Note: Yes, you can make an object with a method that returns a Result<ActionResult>... or in other words always returns some ActionResult, for both successes and errors, which seems a little confusing... And it certainly would not play nice with the constructors and implicit conversions we've set up... but there is already an abstraction for objects that always return an HTTP results whether the action was a success or not. They're called controllers! It probably doesn't make sense for a controller to be making queries to some object that is essentially another controller... so I think returning Result<ActionResult> is a pretty pungent code smell. Moving on...
Let's revisit our delete post action, where we can see that the controller has reduced it's responsibility to nothing more than orchestrating the process for resolving a request, and it fails fast as soon as some part of that process returns an error.
[HttpDelete("{id}")]
public IActionResult DeletePost(int id)
{
Result<User> getUser = _userService.GetCurrentUser();
// The user might be unauthenticated, the user
// service is now responsible for detecting that
// logging it, responding with challenge etc.
if (getUser.Errored)
return getUser.ErrorResult
Result deletion = _postRepository.DeletePost(id, getUser.Value);
// Was the post not found?
// Was the user unauthorized?
// Did we lose our database connection?
// We don't know, we don't care.
if(deletion.Errored)
return deletion.ErrorResult;
// All must have went well.
return NoContent();
}
Whew! This was a long post, and thanks for making it all the way to the end! If you have any questions or comments, feel free to leave them here, or if they come to you later, tweet them may way @sam_ferree! Like the idea? Fork me on GitHub
Top comments (21)
In all the asp.net apis I have written I have adopted the global exception handler approach to returning errors.
It gives me a central location for logging and an easy way for nested services to fail fast.
Do you find all the
if (result.errored)checks everywhere add a lot more conditional paths to your code?Where are the sharp edges, in your experience, with this approach?
How do you feel about using an app framework type (ActionResult) throughout your business logic? Do all your libs take a dependency on Asp.net now?
Thanks for the interesting post.
If you do a good job of applying the single responsibility principle, in my experience the controller doesn’t usually have more than a handful of calls to make.
The
if (action.Errored)are used more like guard clauses with early exits so it doesn’t feel to me like it gets too busy, could just be a preference though.The biggest problem is constructors. I would love for new T() to return a Result with a bad request error if the validation failed. I end up moving this logic into a TFactory and then I’m left with anemic data models which doesn’t make my inner programmer happy... an object should be able to enforce it’s own rules about data validity and not offload that to some other class. I’m playing with a few solutions, but my favorite one won’t have the necessary language features (interfaces and abstract classes that can define static methods) until C# 8.0 (or even later :( )
ActionResult is used as the error type because it’s so handy in ASP.NET Core, i’ve Used other failure types, and sometimes my own depending on the context I’m working in.
If I need to keep web client framework code out of my domain layer, I’d probably have my domain code throw appropriate exceptions, and then apply a facade in me .net Core App that catches those exceptions and maps them to ActionResults. My main goal here is to avoid catching and handling exceptions in my controllers. Some other web framework might have a really elegant way of handling exceptions, and while I dislike it, it is a pattern than a lot of other developers understand.
You got it wrong (about exception handling and flow control). The way you use Result is correct and when you can do that instead of throwing exceptions - you should go for it. Exception handling is much slower and it's not meant to replace the normal workflow. The whole idea of throwing exceptions is, that you may have several method calls nested (even recursively) and you may handle the exceptions way up in the stack calls. If you do this using Result - the whole process turns into a lot of result checking and returning errors back to the caller, which is obviously worse and makes the code hard to read just like it's harder to read when you use exceptions as flow control.
Things get a lot better when you can use Railway Oriented Programming but C# isn't built for it. There is certainly a trade-off: explicit function contracts or nicer nesting. At this point in my career I choose the former and then lean toward languages that support both (F#).
Ah functional programming... you know what they say - it's like communism - most arguments against communism can be applied to functional programming as well:
You seem like a nice person.
This is an understatement :)
That's a great point I hadn't considered.
This is the pattern I've been using in C# too. I've tried functional concepts like "either" but the language fights you and it's an added complexity. Nice idea using implicit operators. I'm stealing that.
Isn't exception itself Railway Oriented Programming? I guess that pattern was originally meant to be for "C" like programming where exception wasn't the language feature.
If you look at how the control flows, exception path is a separate path created by language to follow everything correctly.
Multiple nested Try/Catch has huge performance cost. Not only this, too much branching is debugging nightmare, and it slows down CPU instruction caching.
And a biggest issue is, if someone eats up exception and re throws wrong one, you could spend days trying to find bug.
Why not just use xmldocs with
Could this be applied to async code? Was having some issues figuring out how.
This is some older code, and I've played with a few versions of result now.
but I did often want to take a Result of Task of T and await it to get Result of T
Essentially to make anything awaitable you need to have a method that
returns a TaskAwaiter of T, or in our case Result of Task of T, which we can do with an extension method.
This is based off a different Result class I've been working with, but the basic idea should work.
Awesome, thank you for sharing. If you are willing, could you also add your newer approach to your Github? Would like to check that out too!
I don't see a lot of exception handling code because I believe in writing preventive code whenever in the stack that makes sense.
Yup. That’s mostly my intent here. Services with preventative code. In the event of some inability to complete the request, the services know more about what kind of failure happened than the controller, so I use the this enable them to dictate the correct HTTP Response.
Nice! seems quite useful in general for error handling, I might put this to use in some node side projects I have
thanks for the post!
I think you might need to come to the XXI century, to a more elegant, robust error handling in where we got rid of ResultXXX objects
Hey, I read your comment and was curious about what you think is a more elegant, robust error handling approach, but no details were listed in your comment. Would you expand on your thoughts here?
I use the following library to handle all exceptions without any try..catch in controllers. github.com/khellang/Middleware