0

In asp.net application, all the exception that occurs and are not inside try catch can be handled by application_error.

If we just need to log the exception along with its stack trace, and we need not make any other decision/logic inside catch, why should we put try catch at application/bl or dal layer functions? Is there any reason to put try/catch with every database call function?

For example we have hundreds of function in DAL layer that executes following code:

try
{
  //open db connection, execute stored procedure
}
catch
{
  //log error
}

In case we get any exception from stored procedure OR in opening database connection, we get an exception but we are not doing anything except for logging these errors. We don't have very critical data-storage/retrieval requirement. We are logging error just to be alerted and fix it later. Is this correct to put catch in every such function?

Uwe Keim
  • 39,551
  • 56
  • 175
  • 291
Sahil Sharma
  • 3,847
  • 6
  • 48
  • 98
  • 2
    There's a reason to use `try` and `catch` regardless. It's just a basic part of programming. So, the answer is yes. – Koby Douek Aug 27 '17 at 18:08
  • what is that reason? specifically what we can't achieve by putting logging exception at application_error function? – Sahil Sharma Aug 27 '17 at 18:09
  • Don't cause your code to fallback to application_error. Each error should be handled within the scope it was generated from. application_error is your last fallback, and theoretically should never be reached. – Koby Douek Aug 27 '17 at 18:11
  • I get the theoretical part. But what is the reason behind that? I see benefit of not putting try catch with multiple functions if i put things at application_error. I want to understand benefit of putting it as function level? – Sahil Sharma Aug 27 '17 at 18:12
  • 1
    An exception means that something wasn't completed. If something wasn't completed, your business process failed. If your business process failed, you need to know about it and handle it within the scope of that code, not in application_error. – Koby Douek Aug 27 '17 at 18:14
  • I agree. But we are not doing anything scope/business related in the catch block and just putting logging at every catch block. Why should we do this if we just require to log the exceptions? – Sahil Sharma Aug 27 '17 at 18:20
  • Let us [continue this discussion in chat](http://chat.stackoverflow.com/rooms/152958/discussion-between-koby-douek-and-maverick). – Koby Douek Aug 27 '17 at 18:29

2 Answers2

1

Using try and catch is not for logging purposes only, especially when dealing with database connections.

An exception means that something wasn't completed. If something wasn't completed, your business process failed. If your business process failed, you need to know about it and handle it within the scope of that code, not application_error. Each error should be handled within the scope it was generated from. application_error should be your last fallback, and theoretically should never be reached.

Sure, you can use it for logging, but also for closing your DB connection (which was probably opened before the exception occurred and be left forever open), informing your users that an exception occured, and for data recovery, alternating your process to deal with the exception or preparing it for a retry.

So, taking your posted template, good code handling should look like this:

try
{
  //open db connection, execute stored procedure
}
catch
{
    // Inform the user
    // Alternate your process or preparing for retry
    // log error
}
finally
{
    // Close the DB connection
}
Koby Douek
  • 16,156
  • 19
  • 74
  • 103
  • Close DB connection can be handled automatically if we use dapper. (https://stackoverflow.com/questions/40824948/closing-connection-when-using-dapper). If we don't have any requirement to retry for the same request, shouldn't logging part be handled at some common level? – Sahil Sharma Aug 27 '17 at 18:30
  • @maverick It's opinion based, but for my opinion, using "general" error handling like, as you call "common level", is bad practice. Again - each error has a meaning, there's no such thing as a general error. Each failed DB execution has its consequences and should be taken care of in their scope. – Koby Douek Aug 27 '17 at 18:33
1

One should use try/catch blocks only in places where you can meaningfully handle an exception. However, "meaningful handling " includes providing good error messages.

If your catch block simply logs the exception with no additional context, then such block could be replaced with a top-level handler (like application_error) that does the same thing.

If, however, you log additional information available only at the point of invocation, then having a catch block is entirely justified: it enhances the experience by providing better diagnostics, which is a perfectly legitimate goal.

Sahil Sharma
  • 3,847
  • 6
  • 48
  • 98
Sergey Kalinichenko
  • 714,442
  • 84
  • 1,110
  • 1,523