How using try catch for exception handling is best practice

Question

while maintaining my colleague's code from even someone who claims to be a senior developer, I often see the following code:

try
{
  //do something
}
catch
{
  //Do nothing
}

or sometimes they write logging information to log files like following try catch block

try
{
  //do some work
}
catch(Exception exception)
{
   WriteException2LogFile(exception);
}

I am just wondering if what they have done is the best practice? It makes me confused because in my thinking users should know what happens with the system.

Answer 1

My exception-handling strategy is:

• To catch all unhandled exceptions by hooking to the Application.ThreadException event, then decide:

  • For a UI application: to pop it to the user with an apology message (WinForms)
  • For a Service or a Console application: log it to a file (service or console)

Then I always enclose every piece of code that is run externally in try/catch :

• All events fired by the WinForms infrastructure (Load, Click, SelectedChanged...)
• All events fired by third party components

Then I enclose in 'try/catch'

• All the operations that I know might not work all the time (IO operations, calculations with a potential zero division...). In such a case, I throw a new ApplicationException("custom message", innerException) to keep track of what really happened

Additionally, I try my best to sort exceptions correctly. There are exceptions which:

• need to be shown to the user immediately

• require some extra processing to put things together when they happen to avoid cascading problems (ie: put .EndUpdate in the finally section during a TreeView fill)

• the user does not care, but it is important to know what happened. So I always log them:

• In the event log

• or in a .log file on the disk

It is a good practice to design some static methods to handle exceptions in the application top level error handlers.

I also force myself to try to:

• Remember ALL exceptions are bubbled up to the top level. It is not necessary to put exception handlers everywhere.
• Reusable or deep called functions does not need to display or log exceptions : they are either bubbled up automatically or rethrown with some custom messages in my exception handlers.

So finally:

Bad:

// DON'T DO THIS; ITS BAD
try
{
    ...
}
catch 
{
   // only air...
}

Useless:

// DON'T DO THIS; IT'S USELESS
try
{
    ...
}
catch(Exception ex)
{
    throw ex;
}

Having a try finally without a catch is perfectly valid:

try
{
    listView1.BeginUpdate();

    // If an exception occurs in the following code, then the finally will be executed
    // and the exception will be thrown
    ...
}
finally
{
    // I WANT THIS CODE TO RUN EVENTUALLY REGARDLESS AN EXCEPTION OCCURRED OR NOT
    listView1.EndUpdate();
}

What I do at the top level:

// i.e When the user clicks on a button
try
{
    ...
}
catch(Exception ex)
{
    ex.Log(); // Log exception

    -- OR --
    
    ex.Log().Display(); // Log exception, then show it to the user with apologies...
}

What I do in some called functions:

// Calculation module
try
{
    ...
}
catch(Exception ex)
{
    // Add useful information to the exception
    throw new ApplicationException("Something wrong happened in the calculation module:", ex);
}

// IO module
try
{
    ...
}
catch(Exception ex)
{
    throw new ApplicationException(string.Format("I cannot write the file {0} to {1}", fileName, directoryName), ex);
}

There is a lot to do with exception handling (Custom Exceptions) but those rules that I try to keep in mind are enough for the simple applications I do.

Here is an example of extensions methods to handle caught exceptions a comfortable way. They are implemented in a way they can be chained together, and it is very easy to add your own caught exception processing.

// Usage:

try
{
    // boom
}
catch(Exception ex)
{
    // Only log exception
    ex.Log();

    -- OR --

    // Only display exception
    ex.Display();

    -- OR --

    // Log, then display exception
    ex.Log().Display();

    -- OR --

    // Add some user-friendly message to an exception
    new ApplicationException("Unable to calculate !", ex).Log().Display();
}

// Extension methods

internal static Exception Log(this Exception ex)
{
    File.AppendAllText("CaughtExceptions" + DateTime.Now.ToString("yyyy-MM-dd") + ".log", DateTime.Now.ToString("HH:mm:ss") + ": " + ex.Message + "\n" + ex.ToString() + "\n");
    return ex;
}

internal static Exception Display(this Exception ex, string msg = null, MessageBoxImage img = MessageBoxImage.Error)
{
    MessageBox.Show(msg ?? ex.Message, "", MessageBoxButton.OK, img);
    return ex;
}

Answer 2

Best practice is that exception handling should never hide issues. This means that try-catch blocks should be extremely rare.

There are 3 circumstances where using a try-catch makes sense.

1. Always deal with known exceptions as low-down as you can. However, if you're expecting an exception it's usually better practice to test for it first. For instance parse, formatting and arithmetic exceptions are nearly always better handled by logic checks first, rather than a specific try-catch.

2. If you need to do something on an exception (for instance logging or roll back a transaction) then re-throw the exception.

3. Always deal with unknown exceptions as high-up as you can - the only code that should consume an exception and not re-throw it should be the UI or public API.

Suppose you're connecting to a remote API, here you know to expect certain errors (and have things to in those circumstances), so this is case 1:

try 
{
    remoteApi.Connect()
}
catch(ApiConnectionSecurityException ex) 
{
    // User's security details have expired
    return false;
}

return true;

Note that no other exceptions are caught, as they are not expected.

Now suppose that you're trying to save something to the database. We have to roll it back if it fails, so we have case 2:

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;
}

Note that we re-throw the exception - the code higher up still needs to know that something has failed.

Finally we have the UI - here we don't want to have completely unhandled exceptions, but we don't want to hide them either. Here we have an example of case 3:

try
{
    // Do something
}
catch(Exception ex) 
{
    // Log exception for developers
    WriteException2LogFile(ex);

    // Display message to users
    DisplayWarningBox("An error has occurred, please contact support!");
}

However, most API or UI frameworks have generic ways of doing case 3. For instance ASP.Net has a yellow error screen that dumps the exception details, but that can be replaced with a more generic message in the production environment. Following those is best practice because it saves you a lot of code, but also because error logging and display should be config decisions rather than hard-coded.

This all means that case 1 (known exceptions) and case 3 (one-off UI handling) both have better patterns (avoid the expected error or hand error handling off to the UI).

Even case 2 can be replaced by better patterns, for instance transaction scopes (using blocks that rollback any transaction not committed during the block) make it harder for developers to get the best practice pattern wrong.

For instance suppose you have a large scale ASP.Net application. Error logging can be via ELMAH, error display can be an informative YSoD locally and a nice localised message in production. Database connections can all be via transaction scopes and using blocks. You don't need a single try-catch block.

TL;DR: Best practice is actually to not use try-catch blocks at all.

Answer 3

An exception is a blocking error.

First of all, the best practice should be don't throw exceptions for any kind of error, unless it's a blocking error.

If the error is blocking, then throw the exception. Once the exception is already thrown, there's no need to hide it because it's exceptional; let the user know about it (you should reformat the whole exception to something useful to the user in the UI).

Your job as software developer is to endeavour to prevent an exceptional case where some parameter or runtime situation may end in an exception. That is, exceptions mustn't be muted, but these must be avoided.

For example, if you know that some integer input could come with an invalid format, use int.TryParse instead of int.Parse. There is a lot of cases where you can do this instead of just saying "if it fails, simply throw an exception".

Throwing exceptions is expensive.

If, after all, an exception is thrown, instead of writing the exception to the log once it has been thrown, one of best practices is catching it in a first-chance exception handler. For example:

• ASP.NET: Global.asax Application_Error
• Others: AppDomain.FirstChanceException event.

My stance is that local try/catches are better suited for handling special cases where you may translate an exception into another, or when you want to "mute" it for a very, very, very, very, very special case (a library bug throwing an unrelated exception that you need to mute in order to workaround the whole bug).

For the rest of the cases:

• Try to avoid exceptions.
• If this isn't possible: first-chance exception handlers.
• Or use a PostSharp aspect (AOP).

Answering to @thewhiteambit on some comment...

@thewhiteambit said:

Exceptions are not Fatal-Errors, they are Exceptions! Sometimes they are not even Errors, but to consider them Fatal-Errors is completely false understanding of what Exceptions are.

First of all, how an exception can't be even an error?

• No database connection => exception.
• Invalid string format to parse to some type => exception
• Trying to parse JSON and while input isn't actually JSON => exception
• Argument null while object was expected => exception
• Some library has a bug => throws an unexpected exception
• There's a socket connection and it gets disconnected. Then you try to send a message => exception
• ...

We might list 1k cases of when an exception is thrown, and after all, any of the possible cases will be an error.

An exception is an error, because at the end of the day it is an object which collects diagnostic information -- it has a message and it happens when something goes wrong.

No one would throw an exception when there's no exceptional case. Exceptions should be blocking errors because once they're thrown, if you don't try to fall into the use try/catch and exceptions to implement control flow they mean your application/service will stop the operation that entered into an exceptional case.

Also, I suggest everyone to check the fail-fast paradigm published by Martin Fowler (and written by Jim Shore). This is how I always understood how to handle exceptions, even before I got to this document some time ago.

[...] consider them Fatal-Errors is completely false understanding of what exceptions are.

Usually exceptions cut some operation flow and they're handled to convert them to human-understandable errors. Thus, it seems like an exception actually is a better paradigm to handle error cases and work on them to avoid an application/service complete crash and notify the user/consumer that something went wrong.

More answers about @thewhiteambit concerns

For example in case of a missing Database-Connection the program could exceptionally continue with writing to a local file and send the changes to the Database once it is available again. Your invalid String-To-Number casting could be tried to parse again with language-local interpretation on Exception, like as you try default English language to Parse("1,5") fails and you try it with German interpretation again which is completely fine because we use comma instead of point as separator. You see these Exceptions must not even be blocking, they only need some Exception-handling.

1. If your app might work offline without persisting data to database, you shouldn't use exceptions, as implementing control flow using try/catch is considered as an anti-pattern. Offline work is a possible use case, so you implement control flow to check if database is accessible or not, you don't wait until it's unreachable.

2. The parsing thing is also an expected case (not EXCEPTIONAL CASE). If you expect this, you don't use exceptions to do control flow!. You get some metadata from the user to know what his/her culture is and you use formatters for this! .NET supports this and other environments too, and an exception because number formatting must be avoided if you expect a culture-specific usage of your application/service.

An unhandled Exception usually becomes an Error, but Exceptions itself are not codeproject.com/Articles/15921/Not-All-Exceptions-Are-Errors

This article is just an opinion or a point of view of the author.

Since Wikipedia can be also just the opinion of articule author(s), I wouldn't say it's the dogma, but check what Coding by exception article says somewhere in some paragraph:

[...] Using these exceptions to handle specific errors that arise to continue the program is called coding by exception. This anti-pattern can quickly degrade software in performance and maintainability.

It also says somewhere:

Incorrect exception usage

Often coding by exception can lead to further issues in the software with incorrect exception usage. In addition to using exception handling for a unique problem, incorrect exception usage takes this further by executing code even after the exception is raised. This poor programming method resembles the goto method in many software languages but only occurs after a problem in the software is detected.

Honestly, I believe that software can't be developed don't taking use cases seriously. If you know that...

• Your database can go offline...
• Some file can be locked...
• Some formatting might be not supported...
• Some domain validation might fail...
• Your app should work in offline mode...
• whatever use case...

...you won't use exceptions for that. You would support these use cases using regular control flow.

And if some unexpected use case isn't covered, your code will fail fast, because it'll throw an exception. Right, because an exception is an exceptional case.

In the other hand, and finally, sometimes you cover exceptional cases throwing expected exceptions, but you don't throw them to implement control flow. You do it because you want to notify upper layers that you don't support some use case or your code fails to work with some given arguments or environment data/properties.

Answer 4

The only time you should worry your users about something that happened in the code is if there is something they can or need to do to avoid the issue. If they can change data on a form, push a button or change a application setting in order to avoid the issue then let them know. But warnings or errors that the user has no ability to avoid just makes them lose confidence in your product.

Exceptions and Logs are for you, the developer, not your end user. Understanding the right thing to do when you catch each exception is far better than just applying some golden rule or rely on an application-wide safety net.

Mindless coding is the ONLY kind of wrong coding. The fact that you feel there is something better that can be done in those situations shows that you are invested in good coding, but avoid trying to stamp some generic rule in these situations and understand the reason for something to throw in the first place and what you can do to recover from it.

Answer 5

I know this is an old question, but nobody here mentioned the MSDN article, and it was the document that actually cleared it up for me, MSDN has a very good document on this, you should catch exceptions when the following conditions are true:

• You have a good understanding of why the exception might be thrown, and you can implement a specific recovery, such as prompting the user to enter a new file name when you catch a FileNotFoundException object.

• You can create and throw a new, more specific exception.

int GetInt(int[] array, int index)
{
    try
    {
        return array[index];
    }
    catch(System.IndexOutOfRangeException e)
    {
        throw new System.ArgumentOutOfRangeException(
            "Parameter index is out of range.");
    }
}

• You want to partially handle an exception before passing it on for additional handling. In the following example, a catch block is used to add an entry to an error log before re-throwing the exception.

    try
{
    // Try to access a resource.
}
catch (System.UnauthorizedAccessException e)
{
    // Call a custom error logging procedure.
    LogError(e);
    // Re-throw the error.
    throw;     
}

I'd suggest reading the entire "Exceptions and Exception Handling" section and also Best Practices for Exceptions.

Answer 6

The better approach is the second one (the one in which you specify the exception type). The advantage of this is that you know that this type of exception can occur in your code. You are handling this type of exception and you can resume. If any other exception came, then that means something is wrong which will help you find bugs in your code. The application will eventually crash, but you will come to know that there is something you missed (bug) which needs to be fixed.

Answer 7

With Exceptions, I try the following:

First, I catch special types of exceptions like division by zero, IO operations, and so on and write code according to that. For example, a division by zero, depending the provenience of the values I could alert the user (example a simple calculator in that in a middle calculation (not the arguments) arrives in a division by zero) or to silently treat that exception, logging it and continue processing.

Then I try to catch the remaining exceptions and log them. If possible allow the execution of code, otherwise alert the user that a error happened and ask them to mail a error report.

In code, something like this:

try{
    //Some code here
}
catch(DivideByZeroException dz){
    AlerUserDivideByZerohappened();
}
catch(Exception e){
    treatGeneralException(e);
}
finally{
    //if a IO operation here i close the hanging handlers for example
}

Answer 8

The second approach is a good one.

If you don't want to show the error and confuse the user of application by showing runtime exception(i.e. error) which is not related to them, then just log error and the technical team can look for the issue and resolve it.

try
{
  //do some work
}
catch(Exception exception)
{
   WriteException2LogFile(exception);//it will write the or log the error in a text file
}

I recommend that you go for the second approach for your whole application.

Answer 9

Leave blank catch block is the worse thing to do. If there is an error the best way to handle it is to:

1. Log it into file\database etc..
2. Try to fix it on the fly (maybe trying alternative way of doing that operation)
3. If we cannot fix that, notify the user that there is some error and of course abort the operation

Answer 10

To me, handling exception can be seen as business rule. Obviously, the first approach is unacceptable. The second one is better one and it might be 100% correct way IF the context says so. Now, for example, you are developing an Outlook Addin. If you addin throws unhandled exception, the outlook user might now know it since the outlook will not destroy itself because of one plugin failed. And you have hard time to figure out what went wrong. Therefore, the second approach in this case, to me, it is a correct one. Beside logging the exception, you might decide to display error message to user - i consider it as a business rule.

Answer 11

Best practice is to throw an Exception when the error occurs. Because an error has occurred and it should not be hidden.

But in real life you can have several situations when you want to hide this

1. You rely on third party component and you want to continue the program in case of error.
2. You have a business case that you need to continue in case of error

Answer 12

You should consider these Design Guidelines for Exceptions

• Exception Throwing
• Using Standard Exception Types
• Exceptions and Performance

https://docs.microsoft.com/en-us/dotnet/standard/design-guidelines/exceptions

Answer 13

The catch without any arguments is simply eating the exception and is of no use. What if a fatal error occurs? There's no way to know what happened if you use catch without argument.

A catch statement should catch more specific Exceptions like FileNotFoundException and then at the very end you should catch Exception which would catch any other exception and log them.

Answer 14

Sometimes you need to treat exceptions which say nothing to users.

My way is:

• To catch uncaughted exceptions on application level (ie. in global.asax) for critical exceptions (application can not be useful). These exeptions I am not catching on the place. Just log them on app level and let system do its job.
• Catch "on place" and show some useful info to user (entered wrong number, can't parse).
• Catch on place and do nothing on marginal problems like "I will check for update info on the background, but the service is not running".

It definitely does not have to be best practice. ;-)

Answer 15

I can tell you something:

Snippet #1 is not acceptable because it's ignoring exception. (it's swallowing it like nothing happened).

So do not add catch block that do nothing or just rethrows.

Catch block should add some value. For example output message to end user or log error.

Do not use exception for normal flow program logic. For example:

e.g input validation. <- This is not valid exceptional situation, rather you should write method IsValid(myInput); to check whether input item is valid or not.

Design code to avoid exception. For example:

int Parse(string input);

If we pass value that cannot be parsed to int, this method would throw and exception, instead of that we might write something like this:

bool TryParse(string input,out int result); <- this method would return boolean indicating if parse was successfull.

Maybe this is little bit out of scope of this question, but I hope this will help you to make right decisions when it's about try {} catch(){} and exceptions.