What is the best practice in C# to propagate an exception thrown in a finally block without losing an exception from a catch block?

Question

When an exception is possible to be thrown in a finally block how to propagate both exceptions - from catch and from finally?

As a possible solution - using an AggregateException:

internal class MyClass
{
    public void Do()
    {
        Exception exception = null;
        try
        {
            //example of an error occured in main logic
            throw new InvalidOperationException();
        }
        catch (Exception e)
        {
            exception = e;
            throw;
        }
        finally
        {
            try
            {
                //example of an error occured in finally
                throw new AccessViolationException();
            }
            catch (Exception e)
            {
                if (exception != null)
                    throw new AggregateException(exception, e);
                throw;
            }
        }
    }
}

These exceptions can be handled like in following snippet:

private static void Main(string[] args)
{
    try
    {
        new MyClass().Do();
    }
    catch (AggregateException e)
    {
        foreach (var innerException in e.InnerExceptions)
            Console.Out.WriteLine("---- Error: {0}", innerException);
    }
    catch (Exception e)
    {
        Console.Out.WriteLine("---- Error: {0}", e);
    }

    Console.ReadKey();
}

I can't think of a meaningful scenario where it would be necessary to know both exceptions. Your solution would definitely work, but handling it in the caller would be awfully complex. — Sergey Kalinichenko, Oct 20 '13 at 15:55
I think that the re-throw at the end (in the finally catch block) will never be reached, since if an exception occurs the object *e* is never *null*. — keenthinker, Oct 20 '13 at 16:00
If you're catching an exception, you're saying "I know how to fix this" - if it turns out that you **don't** know how to fix this, then you need to do as little as possible to obscure the error situation. — Damien_The_Unbeliever, Oct 20 '13 at 16:01
@SergeySmolnikov if that is all you are doing then you should use the [AppDomain.UnhandledException](http://msdn.microsoft.com/en-us/library/system.appdomain.unhandledexception.aspx) event. — Scott Chamberlain, Oct 20 '13 at 16:37
Good point, but what if an application supposed not to be restarted in such cases as in the example? — Sergey S, Oct 20 '13 at 16:43
@dasblinkenlight: The problem with only propagating one exception is that a caller who expects one exception may make assumptions about the system state which don't hold. For example, a method which is supposed to copy data from a plug-in sensor to a file may expect that someone may unplug the sensor, and intend that the file be a valid representation of everything that was read before it was unplugged. If an attempt to close the file fails with the last record partially written, that would represent a condition the application really should know about. — supercat, Dec 13 '13 at 18:08
@dasblinkenlight: If the application, without knowing about the failed write, tries to grab data from another sensor and put it in the file, that attempt may appear to "succeed" (more disk space may have become available, for example) but in fact be writing data which, because of the left-over half-record, ends up being illegible. Exceptions in `finally` blocks generally indicate icky situations, but in some cases the only correct way to handle them would be to somehow propagate both exceptions, which is somewhat possible but is at best extremely awkward. — supercat, Dec 13 '13 at 18:14

sandimschuh · Answer 1 · 2018-08-27T13:04:19.717

I regularly come into the same situation and have not found a better solution yet. But I think the solution suggested by the OP is eligible.

Here's a slight modification of the original example:

internal class MyClass
{
    public void Do()
    {
        bool success = false;
        Exception exception = null;
        try
        {
            //calling a service that can throw an exception
            service.Call();
            success = true;
        }
        catch (Exception e)
        {
            exception = e;
            throw;
        }
        finally
        {
            try
            {
                //reporting the result to another service that also can throw an exception
                reportingService.Call(success);
            }
            catch (Exception e)
            {
                if (exception != null)
                    throw new AggregateException(exception, e);
                throw;
            }
        }
    }
}

IMHO it will be fatal to ignore one or the other exception here.

Another example: Imagin a test system that calibrates a device (DUT) and therefore has to control another device that sends signals to the DUT.

internal class MyClass
{
    public void Do()
    {
        Exception exception = null;
        try
        {
            //perform a measurement on the DUT
            signalSource.SetOutput(on);
            DUT.RunMeasurement();
        }
        catch (Exception e)
        {
            exception = e;
            throw;
        }
        finally
        {
            try
            {
                //both devices have to be set to a valid state at end of the procedure, independent of if any exception occurred
                signalSource.SetOutput(off);
                DUT.Reset();
            }
            catch (Exception e)
            {
                if (exception != null)
                    throw new AggregateException(exception, e);
                throw;
            }
        }
    }
}

In this example, it is important that all devices are set to a valid state after the procedure. But both devices also can throw exceptions in the finally block that must not get lost or ignored.

Regarding the complexity in the caller, I do not see any problem there either. When using System.Threading.Tasks the WaitAll() method, for example, can also throw AgregateExceptions that have to be handled in the same way.

One more note regarding @damien's comment: The exception is only caught to wrap it into the AggregateException, in case that the finally block throws. Nothing else is done with the exception nor is it handled in any way.

For those who want to go this way you can use a little helper class I created recently:

public static class SafeExecute
{
    public static void Invoke(Action tryBlock, Action finallyBlock, Action onSuccess = null, Action<Exception> onError = null)
    {
        Exception tryBlockException = null;

        try
        {
            tryBlock?.Invoke();
        }
        catch (Exception ex)
        {
            tryBlockException = ex;
            throw;
        }
        finally
        {
            try
            {
                finallyBlock?.Invoke();
                onSuccess?.Invoke();
            }
            catch (Exception finallyBlockException)
            {
                onError?.Invoke(finallyBlockException);

                // don't override the original exception! Thus throwing a new AggregateException containing both exceptions.
                if (tryBlockException != null)
                    throw new AggregateException(tryBlockException, finallyBlockException);

                // otherwise re-throw the exception from the finally block.
                throw;
            }
        }
    }
}

and use it like this:

public void ExecuteMeasurement(CancellationToken cancelToken)
{
    SafeExecute.Invoke(
        () => DUT.ExecuteMeasurement(cancelToken),
        () =>
        {
            Logger.Write(TraceEventType.Verbose, "Save measurement results to database...");
            _Db.SaveChanges();
        },
        () => TraceLog.Write(TraceEventType.Verbose, "Done"));
}

score 2 · Accepted Answer · answered Oct 20 '13 at 16:48

2

As the comments have suggested this may indicate "unfortunately" structured code. For example if you find yourself in this situation often it might indicate that you are trying to do too much within your method. You only want to throw and exception if there is nothing else you can do (your code is 'stuck' with a problem you can't program around. You only want to catch an exception if there is a reasonable expectation you can do something useful. There is an OutOfMemoryException in the framework but you will seldom see people trying to catch it, because for the most part it means you're boned :-)

If the exception in the finally block is a direct result of the exception in the try block, returning that exception just complicates or obscures the real problem, making it harder to resolve. In the rare case where there is a validate reason for returning such as exception then using the AggregateException would be the way to do it. But before taking that approach ask yourself if it's possible to separate the exceptions into separate methods where a single exception can be returned and handled (separately).

answered Oct 20 '13 at 16:48

Dweeberly

4,668
2
22
41

Agree, the design of code should be revised before using such solution. I try to find more save way than to use finally blocks with "try-catch with empty catch block". – Sergey S Oct 20 '13 at 18:39
To add to @Dweeberly's answer, typically use the finally block to do any resource cleanup. Well structured code will not attempt to do any recovery or further processing in the finally block. finally is provided as a last ditch effort for your code to have a one last shot before the exception is propagated up the stack. As the adage goes " with great power comes great responsibility", use "finally" wisely – Abhijeet Patel Oct 20 '13 at 18:50
@AbhijeetPatel: It is normal for some kinds of classes to enter into states whose cleanup will require actions that might fail (e.g. one could construct a `File` class where every write-data method would ensure data was written before returning, but performance would be horrible compared with a class that used deferred writes). Often, failures in `finally` are more serious than those in `try`, but there's seldom any nice way to know whether that will be true in any particular situation. – supercat Dec 13 '13 at 18:16
2

I strongly disagree this indicates a poorly structured code. A simple example is when you need to dispose something in the finally block and you want to make sure that the disposal itself also went fine, no matter if there were any exceptions before that block. However you structure your code, a requirement to "do something and dispose critical resources no matter what happens" is difficult to separate into different scopes without having to deal with try/catch/finally in the end. So, why should we run from the problem, when we're going to face it anyway. – Mladen B. Jul 02 '20 at 06:45

What is the best practice in C# to propagate an exception thrown in a finally block without losing an exception from a catch block?

2 Answers2