Inconsistent Error and Exception Handling

Overview

We do not yet have a consistent way of handling errors in all situations. This is strongly desired.

TODO: Extend the Error and Exception Handling Policy - especially in the areas of Client and Server error handling.

Input from Wolfgang U

Section Practices That Need to be Adhered to:

• • • Another part of re throwing an exception is a proper reaction of an error message (ref. Comments to Revision 1089#A new common Exception: CancelSaveException). In this case the error handling has been done inside of the exception handler joined with a appropriate error message and then a large part of manual code and automatic generated code shall not to be done. One example of this is the CancelSaveException which is defined in PetraEditForms.cs. This exception is thrown in GLAccountHierarchy.ManualCode.cs. In the try block in specific situations a System.Data.ConstraintException is thrown.

Input from ThiasG

• An exception shall never be caught and re-thrown with one exception: Resources need to be freed in error case, e.g. SQL transaction rollbacked, handles freed, wait cursor reseted, what-ever.
• Expected System-Exceptions shall be caught and be put into a OpenPetra specific exception. This help to differentiate between programmed errors and expected error conditions.
• Exceptions, which are a result of a programming error shall never be caught, e.g. NullPointerException. This would mask the original error making it hard to find it.
• If there is additional information for an exception when it is thrown, then it should be added in extra attributes to the exception. This help in handling the exception, because it is possible to process the additional data in the code. Also it is possible to show a i18n error including the additional data. (Is this possible in C# like in C++/Python?)
• All OpenPetra specific Exception shall derive from the base class XXX. The base class adds support for i18n of the Exception for showing an error message to the user. Exceptions should always logged in English to the log file (without i18n).
• Specialized base class for exception for database, IO, server-communication, ... are provided. This helps to catch specific areas of expected exception, which could be handled in this place.
• Exceptions should not be used to change the flow control. Exception shall only be used for error conditions.

I would remove the part of specialization. Often you throw a specialized exception like System.IO.Exception because e.g. a file is not found, but from the point of view of caller of the function/method, e.g. cached data is not accessible and is a more appropriate error then the file is not found message. Internally it is important to have the original exception, which needs to be logged, too.

Until now, we have not added anything about handling the exception, e.g. handling the exception in the client, aborting the current function, logging the exception and showing the error to the user.

We need to add something about handling the extreme cases of losing connection to DB or errors in the Client-Server/Server-Client connection. Do we stop the client session? In which cases the server needs to stop? Not all error condition are initiated by a function call from the client.

All our exceptions should be RemotedExceptions. If possible, our base class should handle most stuff of the GetObjectData method. Why is it needed, that the Exception are in the namespace 'Ict.Petra.Shared.RemotedExceptions'?

I do not understand the use case, where an exception is thrown in a loop many times, because the first exception would abort the loop and the exception should not be caught without been re-thrown...

I would avoid to make the remote boundary a special case in the policy, even if it cost some time. We are talking about error condition, not the good case...

--Thiasg 09:27, 31 January 2011 (UTC)

Nested Exceptions (Wolfgang U)

Let me point out another important part of the error handling policy and this are nested and parametrizde exceptions.

Let us see the following code:

   public class InvalidLedgerInfoException: SystemException
   {
   	public InvalidLedgerInfoException(string message, Exception innerException)
   		: base(message, innerException) 	{}
   }
   
   public class GetLedgerInfo
   {
   	int ledgerNumber;
   	private ALedgerTable ledger=null;
   	
   	public GetLedgerInfo(int ALedgerNumber)
   	{
   		ledgerNumber = ALedgerNumber;
   		ledger = ALedgerAccess.LoadByPrimaryKey(ALedgerNumber, null);
   	}
   	
   	public string RevaluationAccount
   	{
   		
   		get {
   			try {
   				ALedgerRow row = (ALedgerRow)ledger[0];
   				return row.ForexGainsLossesAccount;
   			} catch (Exception catchedException)
   			{
   				string message = 
   					"The RevaluationAccount of leger {0} has been unsuccessfully required!";
   				throw new InvalidLedgerInfoException(
   						String.Format(message, ledgerNumber) ,catchedException);
   			}
   		}
   	}
   	
   }

A data table is read and only the content of a field is used. If we use the test code line

 Assert.AreEqual("5003", new GetLedgerInfo(44).RevaluationAccount, "...");

we get the following result.

This means that we have the info:

Tests.MFinance.GL.TRevaluationTests.T03_GetLedgerInfo:

Ict.Petra.Server.MFinance.GL.InvalidLedgerInfoException : The RevaluationAccount of leger 44 has been unsuccessfully required!

----> System.IndexOutOfRangeException : There is no row at position 0.

In normal cases we only have the information of the System.IndexOutOfRangeException in specific file and line but now we have the additional information that the ledger number 44 has been requested. Even if programm is delivered without any debug info, we now new a user level reason for the error (wrong ledger number) and the programm level consequences for the software (There is no row at position 0).