07 December, 2023

Beware of secondary exceptions

We were contacted by a client who complained that EurekaLog was generating an error report in the wrong place. In fact, the client had an expected exception that he wanted to hide by showing a simple message instead. The client kindly showed his code:
try
  Query.Delete; // - an exception is raised here
except
  Query.Transaction.Rollback;
  ShowMessage('Sorry, could not delete the report');
  Exit;
end;
What's happening? Does EurekaLog really ignore user code?

If the client code were written like this:
try
  Query.Delete; // - an exception is raised here
except
  Exit;
end;
then there would be no problem: Query.Delete raises an exception, the exception is caught by the except block, which does nothing, and execution continues further, EurekaLog does not react (*).

However, the client code looks different: its except block does some (complex) exception recovery.

In any case, such exceptions are called handled because they are handled by the user's (client's) code. A handled exception is handled by cleanup code, which performs some recovery/rollback actions, after which program execution continues and the exception itself is removed.

In general, any cleanup code (be it a destructor or a rollback/recovery code) should be written in a such way so it will not raise exceptions. It seems to me that the logic here is obvious: if your normal code raises an exception, then you can always suggest a possible recovery actions (the so-called “Plan B”). For example, if an exception occurs when opening a document - then the document needs to be closed; If an exception occurs while accessing the printer - the print job must be canceled. If there is an error opening a file - you can try to open it again after a pause or user's action. And so on.

But what will you do if an error occurs in the “Plan B” itself? You wanted to cancel the print job, but it... is not cancelled. So what now? You will not be able to offer any reasonable recovery actions. Because you don't know what happened. Your variables may be corrupted. Hell, you might not even be able to allocate a block of memory right now! The only possible clean way is to kill/restart the application.

This is why the cleanup code must NOT raise exceptions. Because you can't do anything meaningful with these exceptions. In other words: the cleanup code must handle all exceptions that it knows how to handle.

But in practice, things may not be the same as in an ideal world.

In this case, the client's code raises a second exception inside Query.Transaction.Rollback. It means that the execution of the except block is interrupted and execution moves to the exception handler higher up the stack. It turns out that there are no more try blocks higher up the stack in the client's code, so execution is transferred to the global exception handler, which calls EurekaLog to generate a bug report.

Correcting the code will involve following the rule “cleanup code must not raise exceptions” (“cleanup code must handle known exceptions”). For example, as follows:
try
  Query.Delete; // - an exception is raised here
except
  try
    Query.Transaction.Rollback;
  except
    on EDatabaseError do; // - ignore database errors
  end;
  ShowMessage('Sorry, could not delete the report');
  Exit;
end;
In this case, the second exception will be handled in place and will not "bubble up" to the next exception handler (be it a try block or a global exception handler). It is important to write the correct code - i.e. so that it doesn't block exceptions that you don't know how to handle. For example, the following code is incorrect:
  try
    Query.Transaction.Rollback;
  except
    // - ignore ALL errors
  end;
It is incorrect because you do not know how to properly handle/rollback/recover from, for example, an EAccessViolation exception. It is best to write the code so that it takes into account the most narrow conditions for exceptions - only those that you know exactly how to handle. For example, if you know a specific type/code of the error when canceling a transaction - then write like this:
  try
    Query.Transaction.Rollback;
  except
    on E: ESomeSpecificDatabaseException do
      if E.ErrorCode = 1234 then
      begin
        // - ignore only this specific error
      end
      else
        raise;
  end;



Remarks:
Tags: , , , ,