Inconsistent Error and Exception Handling: Difference between revisions

From OpenPetra Wiki
Jump to navigation Jump to search
(Moved input from WolfgangU to new page 'A Use Case for a Custom Exception'.)
 
(5 intermediate revisions by 2 users not shown)
Line 1: Line 1:
==Overview==
==Overview==
'''We do not yet have a consistent way of handling errors in all situations. This is strongly desired.'''
This page served as a 'blackboard' for the Architecture Team to discuss Error and Exception handling in OpenPetra. As such, personal opinions are recorded.
 
'''TODO''': Extend the [[Error and Exception Handling Policy]] - especially in the areas of Client and Server error handling.


''Information found on this page is mostly 'historic' - much of the suggested has been implemented, and where this is so it is noted.''


== Input from ThiasG ==
== Input from ThiasG ==
Expected System-Exceptions shall be caught and be put into an OpenPetra-specific Exception. This help to differentiate between programmed errors and expected Exception conditions.
Expected System-Exceptions shall be caught and be put into an OpenPetra-specific Exception. This help to differentiate between programmed errors and expected Exception conditions.
 
''--> Use <code>EOPAppException</code> or one of the OpenPetra Exceptions that derive from it (or create another OpenPetra Exception, deriving from <code>EOPAppException</code>, it if that is warranted). --[[User:Christiankatict|Christiankatict]] 07:35, 10 April 2014 (UTC)''


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.
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.
Line 19: Line 18:


--[[User:Thiasg|Thiasg]] 09:27, 31 January 2011 (UTC)
--[[User:Thiasg|Thiasg]] 09:27, 31 January 2011 (UTC)
== 2.Input from ThiasG ==
I would throw an Exception for verification errors. Otherwise this would be a extra condition to think of. There should be a special Exception Class for verification errors, which include the TVerificationResult and TVerificationResultCollection objects for information about the verification error.
''--> This has been addressed: The <code>EVerificationResultsException</code> serves that purpose.'' --[[User:Christiankatict|Christiankatict]] 07:31, 10 April 2014 (UTC)
I would not allow to "translate" an exception with one exception: Putting a database error into a OpenPetra specific exception. But as general rule, translation should not be done and not be allowed.
All Exception we are throwing should be custom exceptions deriving from OPDBExcpetion and OPAppException (perhaps some more, eg on for the verification errors). By this it is possible to catch exceptions by catching only some types of exception we are interested in.
''--> This has been addressed: <code>EOPException</code> and <code>EOPAppException</code> Exception Classes have been introduced. Only <code>EOPAppException</code> and <code>EOPDBException</code> derive from <code>EOPException</code>, all other OpenPetra-specific Exceptions derive from one of those two (e.g. <code>EVerificationResultsException</code> derives from <code>EOPAppException</code>). --[[User:Christiankatict|Christiankatict]] 07:31, 10 April 2014 (UTC)
Example of use case does not make sense. The exception should not catched, because it is not handled at this point of code! The information, that it is the ledger 44, is an internal information, a end user is not interested in. This information is available in the debugger through the stacktrace anyway. Writing a log and rethrowing the original exception would be sufficient. The problem with catching and rethrowing is, that in most cases it is harder to debug, because the exception is handled and the original cause is hard to find.
<code>CancelSaveException</code> sounds weird: The exception is mis-used as flow control. In which cases the save operation is canceled, after a verification was already done? Why is it not possible to throw a custom exception with the specific information, which is shown later on after rollback to the user?
''--> This has been addressed in [[https://tracker.openpetra.org/view.php?id=1503 Bug 1503]] - CancelSaveException has been removed; the Data Validation Framework is used instead.'' --[[User:Christiankatict|Christiankatict]] 07:31, 10 April 2014 (UTC)
--[[User:Thiasg|Thiasg]] 07:51, 9 May 2011 (UTC)

Latest revision as of 14:24, 24 April 2014

Overview

This page served as a 'blackboard' for the Architecture Team to discuss Error and Exception handling in OpenPetra. As such, personal opinions are recorded.

Information found on this page is mostly 'historic' - much of the suggested has been implemented, and where this is so it is noted.

Input from ThiasG

Expected System-Exceptions shall be caught and be put into an OpenPetra-specific Exception. This help to differentiate between programmed errors and expected Exception conditions. --> Use EOPAppException or one of the OpenPetra Exceptions that derive from it (or create another OpenPetra Exception, deriving from EOPAppException, it if that is warranted). --Christiankatict 07:35, 10 April 2014 (UTC)

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 extreme cases, such as

  • Losing connection to DB;
  • Client-Server/Server-Client connection;
  • other cases?

What to do in such situations: Do we stop the client session? In which cases the server needs to stop? - Not all error conditions are initiated by a function call from the client.

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

2.Input from ThiasG

I would throw an Exception for verification errors. Otherwise this would be a extra condition to think of. There should be a special Exception Class for verification errors, which include the TVerificationResult and TVerificationResultCollection objects for information about the verification error. --> This has been addressed: The EVerificationResultsException serves that purpose. --Christiankatict 07:31, 10 April 2014 (UTC)


I would not allow to "translate" an exception with one exception: Putting a database error into a OpenPetra specific exception. But as general rule, translation should not be done and not be allowed.

All Exception we are throwing should be custom exceptions deriving from OPDBExcpetion and OPAppException (perhaps some more, eg on for the verification errors). By this it is possible to catch exceptions by catching only some types of exception we are interested in. --> This has been addressed: EOPException and EOPAppException Exception Classes have been introduced. Only EOPAppException and EOPDBException derive from EOPException, all other OpenPetra-specific Exceptions derive from one of those two (e.g. EVerificationResultsException derives from EOPAppException). --Christiankatict 07:31, 10 April 2014 (UTC)

Example of use case does not make sense. The exception should not catched, because it is not handled at this point of code! The information, that it is the ledger 44, is an internal information, a end user is not interested in. This information is available in the debugger through the stacktrace anyway. Writing a log and rethrowing the original exception would be sufficient. The problem with catching and rethrowing is, that in most cases it is harder to debug, because the exception is handled and the original cause is hard to find.

CancelSaveException sounds weird: The exception is mis-used as flow control. In which cases the save operation is canceled, after a verification was already done? Why is it not possible to throw a custom exception with the specific information, which is shown later on after rollback to the user? --> This has been addressed in [Bug 1503] - CancelSaveException has been removed; the Data Validation Framework is used instead. --Christiankatict 07:31, 10 April 2014 (UTC)


--Thiasg 07:51, 9 May 2011 (UTC)