Tuesday, April 29, 2014

Exception handling best practices and exception handling application block


this is what it is said in Framework Design Guidelines for reusable .Net library on exception handing:

  1. do not or avoid swallow errors by catching nonspecific exceptions.
  2. do not exclude any specific exception when catch for the purpose of transferring exceptions
  3. consider catching a specific exception when you understand why it was thrown in a given context and can respond to the failure programmatically.
  4. do not over catch. exception should often be allowed to propagated up the call stack.
  5. do prefer using an empty throw when catching and re-throwing an exception ( when Partial handling)
  6. avoid catching and wrapping nonspecific exceptions
  7. do specific inner exception when wrapping exceptions

as for performance consideration it said that when a member throws an exception, its performance can be orders of magnitude slower. “ In Application Architecture Guide v2” published in P&P, the authors said the following in Exception Handling “Raising and handling exceptions is an expensive process, so it is important that the design also takes into account performance issues”.


however, the following are in the sample code for exception handling application block in Developer Guide to Enterprise Library


try {

   SalaryCalculator calc = new SalaryCalculator();  

   Console.WriteLine("Result is: {0}",      calc.RaiseArgumentOutOfRangeException("jsmith", 0));

       }

 catch (Exception e)


  if (exceptionManager.HandleException(e, "Exception Policy Name")) throw;

}


try {  

SalaryCalculator calc = new SalaryCalculator();  

Console.WriteLine("Result is: {0}",      calc.RaiseArgumentOutOfRangeException("jsmith", 0));

}

catch (Exception e)

 {  

Exception exceptionToThrow; 

 if (exceptionManager.HandleException(e, "Exception Policy Name",     out exceptionToThrow))  

 {  

   if(exceptionToThrow == null)       throw;    

 else      

 throw exceptionToThrow;   }

}

public Decimal GetWeeklySalary(string employeeId, int weeks)

 {   String connString = string.Empty;   String employeeName = String.Empty;  

 Decimal salary = 0;  

try  

{     connString = ConfigurationManager.ConnectionStrings                                       ["EmployeeDatabase"].ConnectionString;   

  // Access database to get salary for employee here...    

 // In this example, just assume it's some large number.     employeeName = "John Smith";     salary = 1000000;    

 return salary / weeks;   }  

catch (Exception ex) 

  {     // provide error information for debugging    

string template = "Error calculating salary for {0}."                      + " Salary: {1}. Weeks: {2}\n"                      + "Data connection: {3}\n{4}";    

 Exception informationException = new Exception( string.Format(template, employeeName, salary, weeks,                        connString, ex.Message));     throw informationException;   } }

 
In my architecture review for .net development projects, I came across projects where the architects claim the project has implemented exception handling using Exception Handling Application Block. However, when I dove into the code I found the code sample listed above are all over the places. I am listing a few more to give you some kind of sense how they look like.

    try
            {
              generalEligibility = _DealerEligibilityBusinessService.GeneralEligibilityGet();
                ViewBag.Dealer = _DealerEligibilityBusinessService.DistrictsGet();
            }
            catch (Exception ex)
            {
                Exception n;
                if (_exceptionHelper.HandleException(ex, ExceptionHandlingPolicy.UIPolicy, out n))

                {

                    throw n;

                }

            }

            try
            {
                response.Data = _service.GeneralEligibilityGet();
               response.IsSuccessful = true;
            }
            catch (Exception ex)
            {
                response = _webAPIHelper.HandleException<List<GenEligibilityDTO>>(ex);
            }

The commonality I have seen is they all like “Exception” very much. They catch “Exception”, they throw “Exception” as if it is the only exception in .NET framework. The following are the issues I have in the exception handling approach by “implementing Enterprise Library Exception Handling Application Block:
 
1.  as the “Framework Design Guidelines” called out, “consider catching a specific exception when you understand why it was thrown in a given context and can respond to the failure programmatically”. The context where exception is throw in important. For the same exception type in 2 different contexts, the way they should be handled could be totally different. To centralize the exception handling logic based on exception type or even worse for all “Exception” type could be seen as ineffective or un-relevant.

2.  As the “Framework Design Guidelines” and Application Architecture Guide v called out, Raising and handling exceptions is an expensive process, it can be orders of magnitude slower. But in the sample code of Exception Handling Application Block,  catch and re-throw becomes a pattern and been used all over the places.

3.  As recommended in the Exception Handling Application Block, most of system I have reviewed integrate Exception Handling block with Logging Application block. as Logging Application Block defines and implements ILogger interface and the Log method is defined in ILogger interface.  the following is the signature of log method:

void Write(
IEnumerable<string> sources,
TraceEventType eventType,
int eventId,
string message,
IDictionary<string, Object> data
)

it is hardly match to Exception structure. when we log unhandled exceptions we want to log exception type, call stack, and inner exceptions. Especially because the type of inner exception is also exception, so the exception logging need to be run recursively till there is no more inner exceptions. Due to the situation described above, using Logging Application Block to log unhandled exceptions will not be seen as a best practices.
In all these code blocks I have seen, over millions line code I have seen, I seldom an catch of more specific exception like “ArgumentNullException” or  HttpRequestValidationException” how graceful the handling could be.
.


I personally believe Exception Handling Application Block has its fair share of the contribution to what we see here. Many .Net architects were misled by the application block or the quick starter shipped with the block. I have personally did a research on exceptions logged in a production system. To my surprise, 90% of the exceptions were caused by programming bug and should be prevented or handled gracefully before the system goes to production. But as we did not spend time to figure out what are the possible exceptions be thrown, and how to handle them in the context of the code, we just log them and re-throw them. When it is at the boundary of the app domain, we just log them. With this we missed many opportunities to fix those bugs and just logged them somewhere.
 

This is how I believe it should be with regarding to Exception Handling:
 

1.  When the code is not at the boundary of the application domain:

a.  Avoid catching general exception  “Exception”

b.  Do catch more specific exceptions and handle them according.( Most likely start with no exception handling, start to handle some more specific exception when you see them.)

c.  do make try block as small as possible when design try-catch block, this is so that the exception handling can be more precise.

d.  Consider “rethrow” exception only when the exception is partially handled

e.  Consider wrap the caught exception with application specific exception with more information and throw it back to the call stack. When the code determined not capable of handle the exception and want to provide more information to the upstream caller,

f.  Avoid “Catch and rethrow”

2.  When the code is on the boundary of the application domain:

a.  Do implement “catch all” policy ( Catch Exception ex) ,log the exception and display a general message to gracefully notify the user (user facing app) or provide feedback to the caller via return value or status code ( service based app)

b.  Do break the execution in debugging mode when an unhandled exception is caught, this is to offer the programmer the first hand information on the exception so that he or she could work on the code to either prevent the exception form been thrown or handle the exception in appropriate context. ( most likely not in the boundary of the application domain). The following conditional compiling could be used to achieve the goal.    

      #If DEBUG Then
   System.Diagnostics.Debugger.Break()
#End If

3.  Exception logging

a.  Do log information like message, call stack, inner exceptions when logging exceptions

b.  Consider log exceptions to structural data store backed up with local event logger implementation when the structural data store is not available due to network issues. this is to enable correlation among exceptions logged in multiple participating computers and facilitating further analysis.

c. Consider define and implement iExceptionLogger outside of normal event logging framework. in iExceptionLogger, overload Log method with various Exception type the system wants to log.

c.  Do review logged exceptions with development team.

d.  Consider log inner exceptions as their own entries, correlated with incident Id. the following is a recommended table structure for unhandled exception logging. it was implemented in a large enterprise system and produced positive impact in overall system quality and stability.

CREATE TABLE [UNHANDLED_EXCEPTION](

[ID] int IDENTITY(1,1) NOT NULL,

[INCIDENT_ID] [varchar](40) NOT NULL,

[INNER_LEVEL_NO] [tinyint] default (0) NOT NULL,

[HOST_NAME] [varchar](30) NOT NULL,

[TYPE] [varchar](100) NOT NULL,

[MESSAGE] [varchar](512) NOT NULL,

[STACK_TRACE] [varchar](1024) NOT NULL,

[CLIENT_IDENTITY] [varchar](50) NULL,

[SQL_SERVER_ERROR_CODE] [int] NULL,

[SQL_SERVER_ERROR_CLASS] [tinyint] NULL,

[SQL_SERVER_SERVER_NAME] [varchar](64) NULL,

[SQL_SERVER_PROCEDURE_NAME] [varchar](128) NULL,

[RECORD_DATE_TIME] [datetime] default (getdate())NOT NULL,

CONSTRAINT [XPKUNHANDLED_EXCEPTION] PRIMARY KEY CLUSTERED

([ID] ASC)

)




 

No comments:

Post a Comment