Catching base Exception to preserve data integrity

Question

I know that often catching all exceptions (C#: catch(Exception exception){...}) is deemed bad practice.

However, I believe that there are situations where it is perfectly reasonable to do it. For example, if I have a number of operations to perform:

foreach(var operation in operationBatch)
    operation.Do();

and a batch should either succeed as a whole, or fail as a whole, does this seem reasonable?

var rollBackStack = new Stack<Operation>(operationBatch.Length);
try{
  foreach(var operation in operationBatch)
  {
      operation.Do();
      rollBackStack.Push(operation); // assuming operation is atomic and cannot both apply changes AND fail
  }
} catch(Exception exception) // whatever went wrong...
{
    foreach(var ranOperation in rollBackStack)
      ranOperation.Undo();

    throw; // or throw new MyDomainException("relevant message", exception);
}

I guess a generic version of the question would be: is catching all exceptions only to perform some action and rethrowing an acceptable practice, or is there another way that's recommended? I can think of something like

var completed = 0;
try{
  foreach(var operation in operationBatch)
  {
      operation.Do();
      ++completed;
  }
} finally {
    if(completed != operationBatch.Length)
        foreach(var ranOperation in operationBatch.Take(completed).Reverse())
          ranOperation.Undo();

    // no need to rethrow from finally
}

But this seems like simply unnecessarily more complex version of the first snippet.

Answer 1

You're OK.

From the documentation for CA1031:

CA1031: Do not catch general exception types

Cause

A general exception such as System.Exception or System.SystemException is caught in a catch statement, or a general catch clause such as catch() is used.

How to Fix Violations

To fix a violation of this rule, catch a more specific exception, or rethrow the general exception as the last statement in the catch block.

It seems the guidance on this particular rule is that it is OK to catch a general exception as long as you rethrow it. Your design is consistent with this guidance.

Note: There is a signficant difference between these two statements:

throw; 

throw new MyDomainException("relevant message", exception);

In most cases you should use the former, which preserves the stack trace and original exception type so that "outer" catch blocks can distinguish between a system error like out of memory (which they usually will not want to catch, since they can't really handle them) and domain exceptions (which they probably can).