2018年4月17日 星期二

.Net Exceptions Best Practices

Exception is one of those constructs that is easy to misuse. This might include not throwing exception when one should or catching the exception without a good reason. Also there is the problem of throwing the wrong exception which not only doesn’t help us, but can confuse us. On the other hand there is the problem of properly handling the exception. Exception handling is even worse when it comes to improper use. So in this post I’m going to discuss some best practices regarding throwing and handling exceptions. I’ll show you how throwing the proper exception can save us a lot of headaches in terms of debugging. I also going to discuss how bad exception handling can be misleading when we want find bugs.

Throwing Exception

When To Throw Exception

There are many circumstances when throwing exception makes sense, in this section I’ll describe them and discuss why it’s a good idea to throw them. Please note that a lot of examples in this article is simplified to prove a point. For example one does not use a method to retrieve an array element. Or in some cases I didn’t use the previously advocated techniques to focus on the current point. So naturally examples doesn’t try to be a perfect form of exception handling in every way, because that introduce extra elements that could potentially distract the reader.

1 – Completing The Process And Giving Result Is Impossible (Fail Fast)

static void FindWinner(int[] winners)
{
if (winners == null)
{
throw new System.ArgumentNullException($"Parameter {nameof(winners)} cannot be null", nameof(winners));
}
OtherMethodThatUsesTheArray(winner);
}
Suppose we have the above method, here we throw an exception because getting result from this method without the existence of winner array is impossible. One other important point is the ease of use for the user of this method. Imagine we didn’t throw an exception and the array passed into OtherMethodThatUsesTheArray method and that method throws a NullReferenceException. By not throwing an exception debugging becomes much harder. Because the debugger of this code must first look into the OtherMethodThatUsesTheArray  method because that’s where the error originated form. Then he figures out that the winner argument is where this error is originated. When we throw an exception we know exactly where is error is originated from and we don’t have to chase it around the code base. Another problem is unnecessary use of resource, suppose before we reach the point that an exception happens by the framework, we do some expensive processing. Now if the method cannot give us our result without the dependent parameters or what have you, we wasted a lot of resource processing when in fact the method couldn’t possibly succeed in the first place. Remember we don’t throw exception when an error might happen, but we do when the error is blocking the process. Also sometimes we could avoid using exception and use try-stuff pattern like int.TryParse and not throwing exception. Note that exceptions are much more expensive than patterns like the one I just mentioned. Also take a look at fail-fast paradigm published by Martin Fowler (and written by Jim Shore).

2 – Calling an object’s member might produce invalid results given the object current state or might completely fail

void WriteLog(FileStream logFile)
{
if (!logFile.CanWrite)
{
throw new System.InvalidOperationException("Logfile cannot be read-only");
}
// Else write data to the log and return.
}
view rawWriteLog.cs hosted with ❤ by GitHub
Here the file stream that is passed to the WriteLog method crated in such a way that is not writable. In this instance we know that this method is not going to work. Because we can’t log to a file that is not writable. Another example is when we have a class that we expect to be in specific state when we receive it. We have again fail fast by throwing an exception and save resources and debugging time.

3 – Catching A Generic Non-Specific Exception And Throwing A More Specific One

static int GetValueFromArray(int[] array, int index)
{
try
{
return array[index];
}
catch (System.IndexOutOfRangeException ex)
{
throw new ArgumentOutOfRangeException("Index is out of range", "index", ex);
}
}
There is one rule of thumb when it comes to exceptions, the more specific exception our program produce, the easier it is to debug and maintain. In other word by doing this our program produce more accurate errors. So we should always strive to throw more specific exceptions as much as possible. That’s why throwing exceptions like System.ExceptionSystem.SystemExceptionSystem.NullReferenceException, or System.IndexOutOfRangeException is not a good idea. It’s better to not use them and see them as a signal that the error is generated by the framework and we’re going to have a tough time debugging. In the code above you see that we catch the IndexOutOfRangeException and throw a new ArgumentOutOfRangeException  that shows us where the actual error is originated from. We can also use it to catch the framework generated exception and throw a new custom exception. That allows us to add additional information or maybe handle it differently. Just make sure you pass the original exception into the custom exception as inner exception, otherwise the stacktrace will be lost.

4 – Throw Exception For Exceptional Cases

This one might sound obvious, but it can be tricky sometimes. There is somethings in our program that are expected to happen and we can’t count them as errors. So we’ll not throw an exception. For example a search query might return empty or the user login attempt might fail. In these circumstances it’s better to return some kind of meaningful message then throwing exception. As Steve McConnell puts it in Code Complete book, “Exceptions should be reserved for what’s truly exceptional” as opposed to expected ones.

Do Not Use Exceptions To Change The Flow Of The Program

Take the below code for example.
[HttpPost]
public ViewResult CreateProduct(CreateProductViewModel viewModel)
{
try
{
ValidateProductViewModel(viewModel);
CreateProduct(viewModel);
}
catch (ValidationException ex)
{
return View(viewModel);
}
}
view rawCreateProduct.cs hosted with ❤ by GitHub
This is a pattern that I see in some legacy code that I need to work on. As you can see the method ValidateProductViewModel throws an exception for some reason if the view model was not valid. Then if the view model was not valid it catches it and return the view with errors. We better change the code above to the code below?!
[HttpPost]
public ViewResult CreateProduct(CreateProduct viewModel)
{
bool viewModelIsValid = ValidateProductViewModel(viewModel);
if(!viewModelIsValid) return View(viewModel);
CreateProduct(viewModel);
return View(viewModel);
}
view rawCreateProduct.cs hosted with ❤ by GitHub
Here, the method that is responsible for validation returns a bool instead of throwing exception if the view model was not valid. Here’s a good question on Stackoverflow about this.

Do Not Return Error Code, Throw Exception Instead

Throwing exception is always safer than return an error code. The reason is what if the calling code forgot to check for or returned error code and went ahead with execution? But that can’t happen if we throw an exception.

Make Sure To Clean Up Any Side Effect If Exception Thrown

private static void MakeDeposit(Account account,decimal amount)
{
try
{
account.Deposit(amount);
}
catch
{
account.RollbackDeposit(amount);
throw;
}
}
view rawMakeDeposit.cs hosted with ❤ by GitHub
Here we know that an error might happen when calling the deposit method. We should make sure if an exception happens, any changes to the system is rolled back.
try
{
DBConnection.Save();
}
catch
{
// Roll back the DB changes so they aren't corrupted on ANY exception
DBConnection.Rollback();
// Re-throw the exception, it's critical that the user knows that it failed to save
throw;
}
view rawDBConnection.cs hosted with ❤ by GitHub
You can also use transaction scope instead of handling it this way. Remember you can do it in finally block too.

If You Catch An Exception And Cannot Handle It Properly, Make Sure To Re-Throw It

There are circumstances when we catch an exception but we don’t intend to handle it, maybe we just want to log it. Something Like:
try
{
conn.Close();
}
catch (InvalidOperationException ex)
{
_logger.LogError(ex.Message, ex);
//bad practice, stack trace is lost
//throw ex;
//good practice, keepi'n the stack trace
throw;
}
view rawReThrow.cs hosted with ❤ by GitHub
As you can see in the code above, not only we should re-throw the exception, but also we should re-throw it in a way that we don’t loose the stack trace. In this case if we use throw ex, we’ll lose the stacktrace, but if we use throw without an instace ex in front of it, we preserved the stackstrace.

Do Not Use Exceptions As Parameter Or Return Value

Using Exception as argument or return value doesn’t make sense most of the time. Maybe the only time that it might make sense is when we use it in an exception factory, something like.
Exception AnalyzeHttpError(int errorCode) {
if (errorCode < 400) {
throw new NotAnErrorException();
}
switch (errorCode) {
case 403:
return new ForbiddenException();
case 404:
return new NotFoundException();
case 500:
return new InternalServerErrorException();
default:
throw new UnknownHttpErrorCodeException(errorCode);
}
}
view rawAnalyzeHttpError.cs hosted with ❤ by GitHub

Prevent The Program From Throwing Exception If Possible, Exceptions Are Expensive

try
{
conn.Close();
}
catch (InvalidOperationException ex)
{
Console.WriteLine(ex.GetType().FullName);
Console.WriteLine(ex.Message);
}
view rawCatch.cs hosted with ❤ by GitHub
Take the above code for example, we put the code for closing the connection in try block. If the connection is already closed, it’ll throws an exception. But maybe we could achieve the same thing without causing the program to throw exception?
if (conn.State != ConnectionState.Closed)
{
conn.Close();
}
view rawWithoutException.cs hosted with ❤ by GitHub
As you can see the try block was unnecessary, it caused the program to throw an exception if the connection was already closed, and exception are more expensive than if check.

Create Your Own Exception Class

Not all exceptions that can happen are covered by the framework. Sometimes we need to create our own exception type. They can be defined as class, like any other classes in C#. We create custom exception classes usually because we want to handle that exception differently. Or that specific kind of exception is very critical to our application. To create a custom exception we need to create a class that is derived from System.Exception.
[Serializable()]
public class InvalidDepartmentException : System.Exception
{
public InvalidDepartmentException() : base() { }
public InvalidDepartmentException(string message) : base(message) { }
public InvalidDepartmentException(string message, System.Exception inner) : base(message, inner) { }
// A constructor is needed for serialization when an exception propagates from a remoting server to the client.
protected InvalidDepartmentException(SerializationInfo info, StreamingContext context) { }
}
Here the The derived class defined four constructors. one default constructor, one that sets the message property, and one that sets both the Message and InnerException. The fourth constructor is used to serialize the exception. Also note that exceptions should be serializable.

Handling (Catching) Exception

When To Catch Exception

Catching exception is even more misused than throwing it. But this kind of misuse can lead to a lot of pain when the application reached its maintenance phase. In the following section I’ll describe some best practices regarding handling exceptions.

1 – Beware Of Pokemon Exception Handling

I saw in a lot of applications, the try block is used to suppress the exception. But that’s not what’s try-catch block is made for.  By using try like this, we change the behavior of our system and making finding bugs harder than it should be. That so common that we have a term for its misuse, namely Pokemon exception handling. Most of the time the thing that happens is that the try-catch block swallows the error and the error end-up somewhere else in our application than its original location. The real pain is that most of the time the error message doesn’t make any sense at all, because it’s not where the original error is coming from. This make for a frustrating debugging experience.

2 – Use Try Block When You can actually handle the exception and recover from it

One good example of this scenario is when the program prompt user for a path to a file and file doesn’t exist. We can recover from this if the application throws an error, maybe by catching the exception and asking the user to enter another file path. For this reason you should order catch block from the most specific to the least specific one.
public void OpenFile(string filePath)
{
try
{
File.Open(path);
}
catch (FileNotFoundException ex)
{
Console.WriteLine("The specified file path not found, please enter another path.");
PromptUserForAnotherFilePath();
}
}
view rawOpenFile.cs hosted with ❤ by GitHub
One other thing you can use is exception filter. Exception filter works like if statements for a catch block. If the check evaluate to true, the catch block executed, otherwise the catch block is ignored.
private static void Filter()
{
try
{
A();
}
catch (OperationCanceledException exception) when (string.Equals(nameof(ExceptionFilter), exception.Message, StringComparison.Ordinal))
{
}
}
view rawFilter.cs hosted with ❤ by GitHub

3 – You Want To Catch A Generic Exception And Throw A More Specific One With More Context

int GetInt(int[] array, int index)
{
try
{
return array[index];
}
catch(System.IndexOutOfRangeException e)
{
throw new System.ArgumentOutOfRangeException("Parameter index is out of range.", e);
}
}
view rawGetInt.cs hosted with ❤ by GitHub
Take the above code for instance. Here we catch the IndexOutOfBound exception and throw ArgumentOutOfRangeException. By doing this we have more context about where the error originated from and can find the source of our problem more quickly.

4 – You Want To Partially Handle The Exception And Pass It Up For Further Processing

try
{
// Open File
}
catch (FileNotFoundException e)
{
_logger.LogError(e);
// Re-throw the error.
throw;
}
In the above example, we catch the exception, we log the information that we want about it and then we re-throw the exception.

5 – Swallow Exception Only At The Highest Layer Of Your Application

In every application, there is some point when exception should be swallowed. For example most of the time in web applications we don’t want our user to see the exception error. We want to show something generic to the user and keep the exception information for debugging purposes. What we can do in these circumstances is that we wrap the code block in try-catch block. If an error happens, we catch the exception, we log the error and return some generic information to the user, something like the code below.
[HttpPost]
public async Task<IActionResult> UpdatePost(PostViewModel viewModel)
{
try
{
_mediator.Send(new UpdatePostCommand{ PostViewModel = viewModel});
return View(viewModel);
}
catch (Exception e)
{
_logger.LogError(e);
return View(viewModel);
}
}
view rawUpdatePost.cs hosted with ❤ by GitHub
Notice that in these circumstances we still log the error, but the error cannot bubble up the layer. Because this is the last layer, so there is no layer above it actually. In other word the only code that should consume an exception and not re-throw it should be the UI or public API. Some developer prefer to configure some global way of handling exceptions that occurring in this layer. You can take a look at my previous post that uses such a technique. The most important things when it comes to exception handling is that exception handling should never hide issues.

Finally The Finally Block

Finally block is used for cleaning up any remaining resources used in try block without waiting for the garbage collector in runtime to finalize the object. We can use it to close database connection, file streams etc. Notice that we first check to see if FileStream object is null before calling close. Without doing this the finally block could throw its own exception which is not a good thing at all.
FileStream file = null;
var fileinfo = new FileInfo("C:\\file.txt");
try
{
file = fileinfo.OpenWrite();
file.WriteByte(0xF);
}
finally
{
// Check for null because OpenWrite might have failed.
if (file != null)
{
file.Close();
}
}
view rawFinally.cs hosted with ❤ by GitHub
We can use it with or without catch block, the important point is finally block is always going to be executed, regardless of whether or not an exception happened. Another important point is that because resources like database connection is so expensive, it’s better to close them in finally block as soon as possible, as opposed to waiting for garbage collector to do it for us. We can also use the using statement since the FileStream is Implementing IDisposable. Worth to note that using statement is just a syntactic sugar that translates to try and finally block and is more readable and overall is a better choice.

Summary

In this post, I described some best practices regarding exceptions. I discussed in what situations it’s a good idea to throw exception and also described some useful guideline for doing it. We also saw how to handle exception and when and why to handle them. We also saw how misusing exception can negatively effect the ability to find and track bugs in our application.

from : http://hamidmosalla.com/2018/02/28/net-exception-best-practices/

沒有留言:

張貼留言