Suggestions for improving error handling

Question

I am interested in finding out what is the correct way of implementing error handling for this situation in C#.

The system tries to do an operation. If the operation succeeded (returns a non-error code), the system logs in a log database that the operation was succesfull. If the operation returns a error code or something caused an exception before the operation, the system will log an error. However, if the an error is caught when logging, the system shouldn't log an error in the database.

I am using C#. The following code is what I have used until now, but I don't know if it is the best practice in this situation.

int response = -1;  //some error code
try
{
    //some code to prepare the operation - may cause exceptions
    response = DoOperation();
    //some code to clean after the operation - may cause exceptions
}
catch
{
    //error handling
}

try
{
    if (response > 0) //non-error code
        //log event in database
    else
        //log event error in database
}
catch
{
    //logging error handling
}

Do you have any suggestions for improving my code?

Note: The catch blocks include in the original code handling specific errors, I just used a general catch blocks for code simplicity in my question.

Answer 1

You could combine the try-catch blocks:

int response = -1;  //some error code
var logging = false;
try
{
    //some code to prepare the operation - may cause exceptions
    response = DoOperation();
    //some code to clean after the operation - may cause exceptions

    logging = true;
    if (response > 0) //non-error code
        //log event in database
    else
        //log event error in database
}
catch
{
    if(logging)
    //logging error handling
    else
    //error handling
}

Even better, if you know the logging could throw a particular type of exception that the other operations will not, then catch just that exception and handle it as a logging exception, and let all others trickle down to a general catch:

int response = -1;  //some error code

try
{
    //some code to prepare the operation - may cause exceptions
    response = DoOperation();
    //some code to clean after the operation - may cause exceptions

    if (response > 0) //non-error code
        //log event in database
    else
        //log event error in database
}
catch(LoggingSpecificException)
{
    //logging error handling
}
catch
{
    //error handling
}

As far as the general strategy, it doesn't look too bad. Getting a "return status code" from a method is more than a little outdated (try-throw-catch was designed to replace this style of method return), but there's a lot of "legacy code" out there, and some built-in methods still return values that indicate failure. The return value of DoOperation(), if any, should be its conceptual "product"; data produced by DoOperation as the result of a computation. If DoOperation, conceptually, doesn't make any such calculation, it would be better conceptually (and thus from an understandability standpoint) to return void and instead throw exceptions on errors. However, you'd have to throw and catch multiple types of exceptions, or keep track of multiple states instead of just "logging".

Also, I don't know if you simply omitted it for the operation, but I would think that knowledge of exactly what went wrong might be good to know, and so any general catch should be catch(Exception ex) or similar to allow use of exception data when handling the error.