Ole Albers
Ole Albers

Reputation: 9285

try-catch for logging purposes?

According to this answer: https://stackoverflow.com/a/1722991/680026 you should only use try-catch if you really do something there besides logging:

Don't catch an exception if you're only going to log the exception and throw it up the stack. It serves no meaning and clutters code.

But what about logging information that are not available at the higher level? eg:

private void AddSomethingToTable(string tablename, string fieldname) {
  try {
    InsertFieldToTable(tablename, fieldname);
  } catch (Exception ex) {
    log.ErrorFormat("Could not insert field '{0}' into table '{1}'", fieldname, tablename);
    throw;
  }
}

private void main() {
  try {
    AddSomethingToTable("users","firstname");
    AddSomethingToTable("users","lastname");
    AddSomethingToTable("users","age");
  } catch (Exception ex) {
    MessageToUser("Sorry. Saving did not work.",ex);
  }
}

As you can see: In my (completely made up) example I log the information about the field that did cause the error. This could be some good information to start finding the error.

So even though I only logged the error, this information could be crucial and be lost outside that method.

Is this a valid use of try-catch here or are there other suggested ways to log that? (I don't think that just always logging this info (regardless if an error occurred or not) would be a valid solution)

Upvotes: 1

Views: 8008

Answers (5)

Webbo
Webbo

Reputation: 82

We use the Try Catch block in a similar way you have suggested and it works well for us. We have implemented ELMAH https://code.google.com/p/elmah/ which is great for logging untrapped errors. With a line of code in the Try Catch block you can also write trapped exceptions into the log.

Catch ex As Exception
            Elmah.ErrorSignal.FromCurrentContext.Raise(ex) 
            Return False
 End Try

The error is handled (the relevant function returned false) and user doesn’t get the Yellow Screen of Death, and we can look up full details of any errors in the log.

Upvotes: 0

jgauffin
jgauffin

Reputation: 101130

From your linked quesion:

The basic rule of thumb for catching exceptions is to catch exceptions if and only if you have a meaningful way of handling them.

I put emphasis on "The basic rule of thumb" as it's not a law. It's a best practice. i.e. follow it until you have a good motivation to not do so.

If you catch exceptions to include information you probably should throw a new meaningful exception with more context information. something like:

try 
{
    //kldfsdölsdöls
}
catch (Exception ex)
{
    throw new MoreDetailedException("Text with context data", ex);
}

That way you'll get all context information for each stack level collected into the same exception (since you included the inner exception in it). Thus the log entry in the top level will contain ALL information in the same line.

Upvotes: 2

Rahul
Rahul

Reputation: 77846

But what about logging information that are not available at a higher instance?

You can pass those information back to caller while re-throwing the exception

private void AddSomethingToTable(string tablename, string fieldname) {
  try {
    InsertFieldToTable(tablename, fieldname);
  } catch (Exception ex) {
    string str = string.Format("Could not insert field '{0}' into table '{1}'", fieldname, tablename);
    throw new Exception(str, ex);
  }

}

Upvotes: 1

user1666620
user1666620

Reputation: 4808

I think you answered your own question with

But what about logging information that are not available at a higher instance

and

this information could be crucial and be lost outside that method

I dislike hard and fast "always do X and never Y", because there are times where it is necessary to go against so-called "best practice" in order to do what is best for your application.

If logging the information is necessary to fix the issue, and you lose this information if you don't log it immediately, then log the information.

Upvotes: 3

Kirill Kobelev
Kirill Kobelev

Reputation: 10557

There is nothing wrong in what you are trying to do. The idea of the other question/answer was that it is better to log the error at the place where you really handle it. In .NET every exception contains a stack trace. This means that upper layer can report location in the code that has generated this error. Doing that in one place instead of many is way more meaningful. This was their idea.

Upvotes: 2

Related Questions