Should you implement IDisposable.Dispose() so that it never throws?
Asked Answered
F

8

57

For the equivalent mechanism in C++ (the destructor), the advice is that it should usually not throw any exceptions. This is mainly because by doing so you might terminate your process, which is only very rarely a good strategy.

In the equivalent scenario in .NET ...

  1. A first exception is thrown
  2. A finally block is executed as a result of the first exception
  3. The finally block calls a Dispose() method
  4. The Dispose() method throws a second exception

... your process does not terminate immediately. However, you lose information because .NET unceremoneously replaces the first exception with the second one. A catch block somewhere up the call stack will therefore never see the first exception. However, one is usually more interested in the first exception because that normally gives better clues as to why things started to go wrong.

Since .NET lacks a mechanism to detect whether code is being executed while an exception is pending, it seems there are really only two choices how IDisposable can be implemented:

  • Always swallow all exceptions that occur inside Dispose(). Not good as you might also end up swallowing OutOfMemoryException, ExecutionEngineException, etc. which I'd usually rather let tear down the process when they occur without another exception already pending.
  • Let all exceptions propagate out of Dispose(). Not good as you might lose information about the root cause of a problem, see above.

So, which is the lesser of the two evils? Is there a better way?

EDIT: To clarify, I'm not talking about actively throwing exceptions from Dispose() or not, I'm talking about letting exceptions thrown by methods called by Dispose() propagate out of Dispose() or not, for example:

using System;
using System.Net.Sockets;

public sealed class NntpClient : IDisposable
{
    private TcpClient tcpClient;

    public NntpClient(string hostname, int port)
    {
        this.tcpClient = new TcpClient(hostname, port);
    }

    public void Dispose()
    {
        // Should we implement like this or leave away the try-catch?
        try
        {
            this.tcpClient.Close(); // Let's assume that this might throw
        }
        catch
        {
        }
    }
}
Falcon answered 23/2, 2009 at 13:24 Comment(1)
For context: WCF is notorious for this...Falconet
F
16

I would argue that swallowing is the lesser of the two evils in this scenario, as it is better to raise the original Exception - caveat: unless, perhaps the failure to cleanly dispose is itself pretty darned critical (perhaps if a TransactionScope couldn't dispose, since that could indicate a rollback failure).

See here for more thoughts on this - including a wrapper/extension method idea:

using(var foo = GetDodgyDisposableObject().Wrap()) {
   foo.BaseObject.SomeMethod();
   foo.BaseObject.SomeOtherMethod(); // etc
} // now exits properly even if Dispose() throws

Of course, you could also do some oddity where you re-throw a composite exception with both the original and second (Dispose()) exception - but think: you could have multiple using blocks... it would quickly become unmanageable. In reality, the original exception is the interesting one.

Falconet answered 23/2, 2009 at 13:29 Comment(2)
I agree it can become unmanageable, but an important point is that the original exception is the interesting one unless there is no original exception. I went ahead and wrote up the oddity and am pretty happy with it, though in my particular case I ended up just swallowing the exception within Dispose (since I could modify = the code and did not want to have to visit every user of the class)Deterioration
This is true and it highlights a weak point in the design of TransactionScope. On the other hand, a transaction cannot fail to rollback because that is the default action even if there's a power loss. TransactionScope should just swallow exceptions during Dispose. Then again, Dispose actually does the work to commit whereas Commit() only marks to commit but does not actually do so. Maybe another design error.Kiri
A
38

The Framework Design Guidelines (2nd ed) has this as (§9.4.1):

AVOID throwing an exception from within Dispose(bool) except under critical situations where the containing process has been corrupted (leaks, inconsistent shared state, etc.).

Commentary [Edit]:

  • There are guidelines, not hard rules. And this is an "AVOID" not a "DO NOT" guideline. As noted (in comments) the Framework breaks this (and other) guidelines in places. The trick is knowing when to break a guideline. That, in many ways, is the difference between a Journeyman and a Master.
  • If some part of the clean-up could fail then should provide a Close method that will throw exceptions so the caller can handle them.
  • If you are following the dispose pattern (and you should be if the type directly contains some unmanaged resource) then the Dispose(bool) may be called from the finaliser, throwing from a finaliser is a bad idea and will block other objects from being finalised.

My view: exceptions escaping from Dispose should only be those, as in the guideline, that as sufficiently catastrophic that no further reliable function is possible from the current process.

Aground answered 23/2, 2009 at 13:41 Comment(10)
Thanks for the pointer. Problem is that the framework itself does not follow this guideline, see Marc's answer.Falcon
LINQ breaks the guidelines, and this is stated in the commentary. Remember guidelines are not unbreakable rules, and this is an AVOID guideline rather than a DO NOT.Aground
Ok, then what do you recommend? Catch all or catch none?Falcon
By default; let none escape. If an operation included in the finaliser has a significant prospect of failing (e.g. closing a network resource), then provide a separate Close method.Aground
I love the idea of putting a Close method that throws and Dispose not throwing. +2 if I could.Lomax
The big problem here is that AVOID is effectively meaningless, as there's no way to know in advance whether Dispose will throw or not, so one has to assume, in principle, that it will. Various network-related .NET classes throw exception on dispose.Eyot
@Eyot AVOID isn't meaningless, most types follow this. If you have types that do not (specific examples would help) check for a Close method that could be used to perform clean up in a more controlled way.Aground
The question talks about Dispose(). The guideline is about Dispose(bool) - Difference!Jen
@MartinBa When following the pattern, Dispose just calls Dispose(bool): it does apply.Aground
@Richard, yes, but the pattern does only apply if the class is itself managing native resources. If the class is only forwarding to IDisposable members, then you do not write a Dispose(bool) method.Jen
F
16

I would argue that swallowing is the lesser of the two evils in this scenario, as it is better to raise the original Exception - caveat: unless, perhaps the failure to cleanly dispose is itself pretty darned critical (perhaps if a TransactionScope couldn't dispose, since that could indicate a rollback failure).

See here for more thoughts on this - including a wrapper/extension method idea:

using(var foo = GetDodgyDisposableObject().Wrap()) {
   foo.BaseObject.SomeMethod();
   foo.BaseObject.SomeOtherMethod(); // etc
} // now exits properly even if Dispose() throws

Of course, you could also do some oddity where you re-throw a composite exception with both the original and second (Dispose()) exception - but think: you could have multiple using blocks... it would quickly become unmanageable. In reality, the original exception is the interesting one.

Falconet answered 23/2, 2009 at 13:29 Comment(2)
I agree it can become unmanageable, but an important point is that the original exception is the interesting one unless there is no original exception. I went ahead and wrote up the oddity and am pretty happy with it, though in my particular case I ended up just swallowing the exception within Dispose (since I could modify = the code and did not want to have to visit every user of the class)Deterioration
This is true and it highlights a weak point in the design of TransactionScope. On the other hand, a transaction cannot fail to rollback because that is the default action even if there's a power loss. TransactionScope should just swallow exceptions during Dispose. Then again, Dispose actually does the work to commit whereas Commit() only marks to commit but does not actually do so. Maybe another design error.Kiri
F
8

Dispose should be designed to do its purpose, disposing the object. This task is safe and does not throw exceptions most of the time. If you see yourself throwing exceptions from Dispose, you should probably think twice to see if you are doing too much stuff in it. Beside that, I think Dispose should be treated like all other methods: handle if you can do something with it, let it bubble if you can't.

EDIT: For the specified example, I would write the code so that my code does not cause an exception, but clearing the TcpClient up might cause an exception, which should be valid to propagate in my opinion (or to handle and rethrow as a more generic exception, just like any method):

public void Dispose() { 
   if (tcpClient != null)
     tcpClient.Close();
}

However, just like any method, if you know tcpClient.Close() might throw an exception that should be ignored (doesn't matter) or should be represented by another exception object, you might want to catch it.

Fashion answered 23/2, 2009 at 13:31 Comment(2)
Bubbling loses the actual thing that broke in the first place; re the pattern... better tell the WCF team ;-pFalconet
I clarified the question. Clearly, calling TcpClient.Close() doesn't qualify as doing to much, right?Falcon
S
4

Releasing resources should be a "safe" operation - after all how can I recover from not being able to release a resource? so throwing an exception from Dispose just doesn't make sense.

However, if I discover inside Dispose that program state is corrupted it's better to throw the exception then to swallow it, its better to crush now then to continue running and produce incorrect results.

Swithbart answered 23/2, 2009 at 13:53 Comment(7)
If an attempt to Dispose a TCP connection succeeds, code is entitled to assume that everything which has been sent has reached the other end. If the attempt fails, code should not proceed on the assumption that the information has been delivered, but that doesn't mean the entire system state is corrupt.Bodoni
@Bodoni - I wrote that releasing a resource SHOULD be a safe operation unless the system state is corrupted (and, according to the accepted answer the framework design guidelines agree with me) I never said all the classes in .net follow this guideline (some don't). I personally think the behavior you describe for TCP connections is wrong (because "the information has been delivered" means nothing, if the server crashes right after sending the TCP ACK the data is just as lost as if it wasn't sent at all), also, I can't find this behavior in the TcpClient MSDN documentation.Swithbart
TCP wasn't a perfect example, but it's familiar to many people; perhaps a better example would be trying to save a file to a removable drive which might not be needed for any other purpose. If someone yanks out a USB stick before a file is written, any resulting exception shouldn't be stifled, but the removal of the stick shouldn't disrupt parts of the application which weren't using it [among other things, since the user's document wasn't saved, the user will probably want to try again to save it!]Bodoni
@Bodoni - if yanking a USB drive caused an error in Dispose because Dispose was trying to flush previously written data to the drive than the system state just became sort-of corrupt (those Write calls you did earlier and returned success - they really failed and anything you did under the assumption they succeeded might be invalid), if yanking the drive caused an error in Dispose because Dispose does file operations than it shouldn't, Dispose is for cleaning up and freeing resources (and sometimes aborting) not for saving things.Swithbart
It is normal for streams to defer write requests until a stream is flushed or closed, with the success of that operation implying success of writing the data. If an execution scope that creates stream gets exited via an exception, the stream would have no reason to write the data prior to that, and no sensible place to write the data other than during the Dispose.Bodoni
@Bodoni - if an execution scope that created a stream is exited via an exception raising yet another exception in Dispose is not helpful - the new exception will mask the original exception so now you don't know why the code failed (except it's not IO, if it was IO we wouldn't be flushing in Dispose) or if the system is in a consistent state. blindly retrying operations where you don't even have access to the failure reason is not a good way to build robust applications...Swithbart
@Bodoni ... and even then your basic assumption that the file is actually saved if you didn't get an exception is wrong, because of OS and hardware caches you can't be sure the data was written unless you use FlushFileBuffers or write-through, FlushFileBuffers is a system-wide operation that effects the performance of every process, not something you want to do automatically on Dispose and if you use write-through you care too much about when data is written to use automatic flushing.Swithbart
B
3

It's too bad Microsoft didn't provide an Exception parameter to Dispose, with the intention that it be wrapped as an InnerException in case disposal itself throws an exception. To be sure, effective use of such a parameter would require use of an exception-filter block, which C# doesn't support, but perhaps the existence of such a parameter could have motivated the C# designers into providing such a feature? One nice variation I'd like to see would be the addition of an Exception "parameter" to a Finally block, e.g.

  finally Exception ex: // In C#
  Finally Ex as Exception  ' In VB

which would behave like a normal Finally block except that 'ex' would be null/Nothing if the 'Try' ran to completion, or would hold the thrown exception if it did not. Too bad there's no way to make existing code use such a feature.

Bodoni answered 16/1, 2011 at 23:2 Comment(0)
C
2

There are various strategies for propagating or swallowing exceptions from the Dispose method, possibly based on whether an unhanded exception was also thrown from the main logic. The best solution would be to leave the decision up to the caller, depending on their specific requirements. I have implemented a generic extension method that does this, offering:

  • the default using semantics of propagating Dispose exceptions
  • Marc Gravell's suggestion of always swallowing Dispose exceptions
  • maxyfc's alternative of only swallowing Dispose exceptions when there is an exception from the main logic that would otherwise be lost
  • Daniel Chambers's approach of wrapping multiple exceptions into an AggregateException
  • a similar approach that always wraps all exceptions into an AggregateException (like Task.Wait does)

This is my extension method:

/// <summary>
/// Provides extension methods for the <see cref="IDisposable"/> interface.
/// </summary>
public static class DisposableExtensions
{
    /// <summary>
    /// Executes the specified action delegate using the disposable resource,
    /// then disposes of the said resource by calling its <see cref="IDisposable.Dispose()"/> method.
    /// </summary>
    /// <typeparam name="TDisposable">The type of the disposable resource to use.</typeparam>
    /// <param name="disposable">The disposable resource to use.</param>
    /// <param name="action">The action to execute using the disposable resource.</param>
    /// <param name="strategy">
    /// The strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method.
    /// </param>
    /// <exception cref="ArgumentNullException"><paramref name="disposable"/> or <paramref name="action"/> is <see langword="null"/>.</exception>
    public static void Using<TDisposable>(this TDisposable disposable, Action<TDisposable> action, DisposeExceptionStrategy strategy)
        where TDisposable : IDisposable
    {
        ArgumentValidate.NotNull(disposable, nameof(disposable));
        ArgumentValidate.NotNull(action, nameof(action));
        ArgumentValidate.IsEnumDefined(strategy, nameof(strategy));

        Exception mainException = null;

        try
        {
            action(disposable);
        }
        catch (Exception exception)
        {
            mainException = exception;
            throw;
        }
        finally
        {
            try
            {
                disposable.Dispose();
            }
            catch (Exception disposeException)
            {
                switch (strategy)
                {
                    case DisposeExceptionStrategy.Propagate:
                        throw;

                    case DisposeExceptionStrategy.Swallow:
                        break;   // swallow exception

                    case DisposeExceptionStrategy.Subjugate:
                        if (mainException == null)
                            throw;
                        break;    // otherwise swallow exception

                    case DisposeExceptionStrategy.AggregateMultiple:
                        if (mainException != null)
                            throw new AggregateException(mainException, disposeException);
                        throw;

                    case DisposeExceptionStrategy.AggregateAlways:
                        if (mainException != null)
                            throw new AggregateException(mainException, disposeException);
                        throw new AggregateException(disposeException);
                }
            }

            if (mainException != null && strategy == DisposeExceptionStrategy.AggregateAlways)
                throw new AggregateException(mainException);
        }
    }
}

These are the implemented strategies:

/// <summary>
/// Identifies the strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method
/// of an <see cref="IDisposable"/> instance, in conjunction with exceptions thrown by the main logic.
/// </summary>
/// <remarks>
/// This enumeration is intended to be used from the <see cref="DisposableExtensions.Using"/> extension method.
/// </remarks>
public enum DisposeExceptionStrategy
{
    /// <summary>
    /// Propagates any exceptions thrown by the <see cref="IDisposable.Dispose"/> method.
    /// If another exception was already thrown by the main logic, it will be hidden and lost.
    /// This behaviour is consistent with the standard semantics of the <see langword="using"/> keyword.
    /// </summary>
    /// <remarks>
    /// <para>
    /// According to Section 8.10 of the C# Language Specification (version 5.0):
    /// </para>
    /// <blockquote>
    /// If an exception is thrown during execution of a <see langword="finally"/> block,
    /// and is not caught within the same <see langword="finally"/> block, 
    /// the exception is propagated to the next enclosing <see langword="try"/> statement. 
    /// If another exception was in the process of being propagated, that exception is lost. 
    /// </blockquote>
    /// </remarks>
    Propagate,

    /// <summary>
    /// Always swallows any exceptions thrown by the <see cref="IDisposable.Dispose"/> method,
    /// regardless of whether another exception was already thrown by the main logic or not.
    /// </summary>
    /// <remarks>
    /// This strategy is presented by Marc Gravell in
    /// <see href="http://blog.marcgravell.com/2008/11/dontdontuse-using.html">don't(don't(use using))</see>.
    /// </remarks>
    Swallow,

    /// <summary>
    /// Swallows any exceptions thrown by the <see cref="IDisposable.Dispose"/> method
    /// if and only if another exception was already thrown by the main logic.
    /// </summary>
    /// <remarks>
    /// This strategy is suggested in the first example of the Stack Overflow question
    /// <see href="https://mcmap.net/q/95036/-swallowing-exception-thrown-in-catch-finally-block/1149773">Swallowing exception thrown in catch/finally block</see>.
    /// </remarks>
    Subjugate,

    /// <summary>
    /// Wraps multiple exceptions, when thrown by both the main logic and the <see cref="IDisposable.Dispose"/> method,
    /// into an <see cref="AggregateException"/>. If just one exception occurred (in either of the two),
    /// the original exception is propagated.
    /// </summary>
    /// <remarks>
    /// This strategy is implemented by Daniel Chambers in
    /// <see href="http://www.digitallycreated.net/Blog/51/c%23-using-blocks-can-swallow-exceptions">C# Using Blocks can Swallow Exceptions</see>
    /// </remarks>
    AggregateMultiple,

    /// <summary>
    /// Always wraps any exceptions thrown by the main logic and/or the <see cref="IDisposable.Dispose"/> method
    /// into an <see cref="AggregateException"/>, even if just one exception occurred.
    /// </summary>
    /// <remarks>
    /// This strategy is similar to behaviour of the <see cref="Task.Wait()"/> method of the <see cref="Task"/> class 
    /// and the <see cref="Task{TResult}.Result"/> property of the <see cref="Task{TResult}"/> class:
    /// <blockquote>
    /// Even if only one exception is thrown, it is still wrapped in an <see cref="AggregateException"/> exception.
    /// </blockquote>
    /// </remarks>
    AggregateAlways,
}

Sample use:

new FileStream(Path.GetTempFileName(), FileMode.Create)
    .Using(strategy: DisposeExceptionStrategy.Subjugate, action: fileStream =>
    {
        // Access fileStream here
        fileStream.WriteByte(42);
        throw new InvalidOperationException();
    });   
    // Any Dispose() exceptions will be swallowed due to the above InvalidOperationException

Update: If you need to support delegates that return values and/or are asynchronous, then you could use these overloads:

/// <summary>
/// Provides extension methods for the <see cref="IDisposable"/> interface.
/// </summary>
public static class DisposableExtensions
{
    /// <summary>
    /// Executes the specified action delegate using the disposable resource,
    /// then disposes of the said resource by calling its <see cref="IDisposable.Dispose()"/> method.
    /// </summary>
    /// <typeparam name="TDisposable">The type of the disposable resource to use.</typeparam>
    /// <param name="disposable">The disposable resource to use.</param>
    /// <param name="strategy">
    /// The strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method.
    /// </param>
    /// <param name="action">The action delegate to execute using the disposable resource.</param>
    public static void Using<TDisposable>(this TDisposable disposable, DisposeExceptionStrategy strategy, Action<TDisposable> action)
        where TDisposable : IDisposable
    {
        ArgumentValidate.NotNull(disposable, nameof(disposable));
        ArgumentValidate.NotNull(action, nameof(action));
        ArgumentValidate.IsEnumDefined(strategy, nameof(strategy));

        disposable.Using(strategy, disposableInner =>
        {
            action(disposableInner);
            return true;   // dummy return value
        });
    }

    /// <summary>
    /// Executes the specified function delegate using the disposable resource,
    /// then disposes of the said resource by calling its <see cref="IDisposable.Dispose()"/> method.
    /// </summary>
    /// <typeparam name="TDisposable">The type of the disposable resource to use.</typeparam>
    /// <typeparam name="TResult">The type of the return value of the function delegate.</typeparam>
    /// <param name="disposable">The disposable resource to use.</param>
    /// <param name="strategy">
    /// The strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method.
    /// </param>
    /// <param name="func">The function delegate to execute using the disposable resource.</param>
    /// <returns>The return value of the function delegate.</returns>
    public static TResult Using<TDisposable, TResult>(this TDisposable disposable, DisposeExceptionStrategy strategy, Func<TDisposable, TResult> func)
        where TDisposable : IDisposable
    {
        ArgumentValidate.NotNull(disposable, nameof(disposable));
        ArgumentValidate.NotNull(func, nameof(func));
        ArgumentValidate.IsEnumDefined(strategy, nameof(strategy));

#pragma warning disable 1998
        var dummyTask = disposable.UsingAsync(strategy, async (disposableInner) => func(disposableInner));
#pragma warning restore 1998

        return dummyTask.GetAwaiter().GetResult();
    }

    /// <summary>
    /// Executes the specified asynchronous delegate using the disposable resource,
    /// then disposes of the said resource by calling its <see cref="IDisposable.Dispose()"/> method.
    /// </summary>
    /// <typeparam name="TDisposable">The type of the disposable resource to use.</typeparam>
    /// <param name="disposable">The disposable resource to use.</param>
    /// <param name="strategy">
    /// The strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method.
    /// </param>
    /// <param name="asyncFunc">The asynchronous delegate to execute using the disposable resource.</param>
    /// <returns>A task that represents the asynchronous operation.</returns>
    public static Task UsingAsync<TDisposable>(this TDisposable disposable, DisposeExceptionStrategy strategy, Func<TDisposable, Task> asyncFunc)
        where TDisposable : IDisposable
    {
        ArgumentValidate.NotNull(disposable, nameof(disposable));
        ArgumentValidate.NotNull(asyncFunc, nameof(asyncFunc));
        ArgumentValidate.IsEnumDefined(strategy, nameof(strategy));

        return disposable.UsingAsync(strategy, async (disposableInner) =>
        {
            await asyncFunc(disposableInner);
            return true;   // dummy return value
        });
    }

    /// <summary>
    /// Executes the specified asynchronous function delegate using the disposable resource,
    /// then disposes of the said resource by calling its <see cref="IDisposable.Dispose()"/> method.
    /// </summary>
    /// <typeparam name="TDisposable">The type of the disposable resource to use.</typeparam>
    /// <typeparam name="TResult">The type of the return value of the asynchronous function delegate.</typeparam>
    /// <param name="disposable">The disposable resource to use.</param>
    /// <param name="strategy">
    /// The strategy for propagating or swallowing exceptions thrown by the <see cref="IDisposable.Dispose"/> method.
    /// </param>
    /// <param name="asyncFunc">The asynchronous function delegate to execute using the disposable resource.</param>
    /// <returns>
    /// A task that represents the asynchronous operation. 
    /// The task result contains the return value of the asynchronous function delegate.
    /// </returns>
    public static async Task<TResult> UsingAsync<TDisposable, TResult>(this TDisposable disposable, DisposeExceptionStrategy strategy, Func<TDisposable, Task<TResult>> asyncFunc)
        where TDisposable : IDisposable
    {
        ArgumentValidate.NotNull(disposable, nameof(disposable));
        ArgumentValidate.NotNull(asyncFunc, nameof(asyncFunc));
        ArgumentValidate.IsEnumDefined(strategy, nameof(strategy));

        Exception mainException = null;

        try
        {
            return await asyncFunc(disposable);
        }
        catch (Exception exception)
        {
            mainException = exception;
            throw;
        }
        finally
        {
            try
            {
                disposable.Dispose();
            }
            catch (Exception disposeException)
            {
                switch (strategy)
                {
                    case DisposeExceptionStrategy.Propagate:
                        throw;

                    case DisposeExceptionStrategy.Swallow:
                        break;   // swallow exception

                    case DisposeExceptionStrategy.Subjugate:
                        if (mainException == null)
                            throw;
                        break;    // otherwise swallow exception

                    case DisposeExceptionStrategy.AggregateMultiple:
                        if (mainException != null)
                            throw new AggregateException(mainException, disposeException);
                        throw;

                    case DisposeExceptionStrategy.AggregateAlways:
                        if (mainException != null)
                            throw new AggregateException(mainException, disposeException);
                        throw new AggregateException(disposeException);
                }
            }

            if (mainException != null && strategy == DisposeExceptionStrategy.AggregateAlways)
                throw new AggregateException(mainException);
        }
    }
}
Colored answered 24/2, 2016 at 21:41 Comment(1)
I like this a lot, but it can be improved in C# 6 using exception filtering. In the outer try you can use catch (Exception exception) when (null == (mainException = exception)) {} to grab the main exception, catch (Exception exception) when (strategy == DisposeExceptionStrategy.AggregateAlways) to aggregate the main exception, in the inner try catch when (strategy == DisposeExceptionStrategy.Swallow || (strategy == DisposeExceptionStrategy.Subject && mainException != null)) {} etc. Then all your stack traces will include the actual calls to action and/or Dispose, not the rethrows.Eudosia
L
1

I would probably use logging to capture detail about the first exception, then allow the second exception to be raised.

Luzon answered 23/2, 2009 at 13:40 Comment(0)
D
0

Here is a way to fairly cleanly grab any exceptions thrown by the contents of the using or the Dispose.

Original code:

using (var foo = new DisposableFoo())
{
    codeInUsing();
}

Then here is code that will throw if either codeInUsing() throws or foo.Dispose() throws or both throw, and let you see the first exception (sometimes wrapped as an InnerExeption, depending):

var foo = new DisposableFoo();
Helpers.DoActionThenDisposePreservingActionException(
    () =>
    {
        codeInUsing();
    },
    foo);

It's not great but not too bad.

Here is the code to implement this. I have it set so that it only works as described when the debugger is not attached, because when the debugger is attached I am more concerned that it will break in the right place on the first exception. You could modify as needed.

public static void DoActionThenDisposePreservingActionException(Action action, IDisposable disposable)
{
    bool exceptionThrown = true;
    Exception exceptionWhenNoDebuggerAttached = null;
    bool debuggerIsAttached = Debugger.IsAttached;
    ConditionalCatch(
        () =>
        {
            action();
            exceptionThrown = false;
        },
        (e) =>
        {
            exceptionWhenNoDebuggerAttached = e;
            throw new Exception("Catching exception from action(), see InnerException", exceptionWhenNoDebuggerAttached);
        },
        () =>
        {
            Exception disposeExceptionWhenExceptionAlreadyThrown = null;
            ConditionalCatch(
                () =>
                {
                    disposable.Dispose();
                },
                (e) =>
                {
                    disposeExceptionWhenExceptionAlreadyThrown = e;
                    throw new Exception("Caught exception in Dispose() while unwinding for exception from action(), see InnerException for action() exception",
                        exceptionWhenNoDebuggerAttached);
                },
                null,
                exceptionThrown && !debuggerIsAttached);
        },
        !debuggerIsAttached);
}

public static void ConditionalCatch(Action tryAction, Action<Exception> conditionalCatchAction, Action finallyAction, bool doCatch)
{
    if (!doCatch)
    {
        try
        {
            tryAction();
        }
        finally
        {
            if (finallyAction != null)
            {
                finallyAction();
            }
        }
    }
    else
    {
        try
        {
            tryAction();
        }
        catch (Exception e)
        {
            if (conditionalCatchAction != null)
            {
                conditionalCatchAction(e);
            }
        }
        finally
        {
            if (finallyAction != null)
            {
                finallyAction();
            }
        }
    }
}
Deterioration answered 9/4, 2013 at 16:17 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.