1

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)

Ole Albers
  • 8,715
  • 10
  • 73
  • 166
  • 4
    i think you answered your own question with "But what about logging information that are not available at a higher instance". I dislike hard and fast "always do X and never Y", because there are times where it is necessary. It's also why "best practice" questions are discouraged here. 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. – user1666620 Jan 20 '16 at 12:05
  • This [related question](http://stackoverflow.com/questions/14973642/how-using-try-catch-for-exception-handling-is-best-practice) will probably help you. – Larry Jan 20 '16 at 12:24

5 Answers5

3

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.

user1666620
  • 4,800
  • 18
  • 27
2

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.

Kirill Kobelev
  • 10,252
  • 6
  • 30
  • 51
2

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.

jgauffin
  • 99,844
  • 45
  • 235
  • 372
1

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);
  }

}

Rahul
  • 76,197
  • 13
  • 71
  • 125
  • the question was about the practice of only using try/catch where you can meaningfully do something about the error. In other words, is the OP wrong to place his try catch within the low-level method merely to add logging, or just leave it at the highest level possible. – user1666620 Jan 20 '16 at 12:19
  • 1
    @user1666620, may a wrong iterpretation my end but part of the question is if it's not logged in lower level catch then the data is missed in caller (OP pointed that). I am trying to point the same here. – Rahul Jan 20 '16 at 12:23
  • @Rahul look at the OP's question again, then read the linked answer. he is already logging the data in the low-level method. He is asking if "this is the wrong thing to do", – user1666620 Jan 20 '16 at 12:25
  • You are both right somehow :) I asked for logging the info but Rahul offered a possible different approach I did not ask for explicitly, but might be a solution in some scenarios – Ole Albers Jan 20 '16 at 12:28
0

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.

Webbo
  • 92
  • 7
  • Logging an error isn't what I would call "is handled". – Ole Albers Jan 20 '16 at 14:54
  • I wasn't saying that logging the error itself is handling it. In the example above the function returns false when there's an exception so that the application can handle it. Might not be appropriate in every scenario but works for us – Webbo Jan 20 '16 at 16:25