[dev] Dealing with Exceptions

Tue Jan 11 17:13:52 UTC 2011

For the record I'd prefer NOT to have the dependency. But I'm not dead-set against it.

Gunnar Wrobel <wrobel at horde.org> wrote:

>
>Zitat von Jan Schneider <jan at horde.org>:
>
>> Zitat von Gunnar Wrobel <wrobel at horde.org>:
>>
>>>
>>> Zitat von Jan Schneider <jan at horde.org>:
>>>
>>>> Zitat von Gunnar Wrobel <wrobel at horde.org>:
>>>>
>>>>> Hi Jan,
>>>>>
>>>>> we already briefly discussed Exceptions on IRC and decided to  
>>>>> continue on the mailing (has been a while back, please forgive me  
>>>>> for the rather long delay on the topic).
>>>>>
>>>>>
>>>>> Zitat von Jan Schneider <jan at horde.org>:
>>>>>
>>>>>> The branch "master" has been updated.
>>>>>> The following is a summary of the commits.
>>>>>>
>>>>>> from: 1e943c0937d592233379d8cac82b89f80861b11c
>>>>>>
>>>>>> 43b8c54 Fix package name.
>>>>>> 7390733 Here too.
>>>>>> e627e65 Fix directory name, remove outdated tests.
>>>>>> f2b35e7 Consistently extend exception classes from  
>>>>>> Horde_Exception_Prior. CS, cleanup.
>>>>>
>>>>> Why should there be any need for a low level framework package to  
>>>>> extend from Horde_Exception_Prior?
>>>>>
>>>>> On IRC you mentioned that
>>>>>
>>>>> 1) this should be done so that re-throwing Exceptions can be done  
>>>>> on both 5.2 and 5.3 in a decent way (using the third argument of  
>>>>> the Horde_Exception constructor, named "previous")
>>>>>
>>>>> 2) it is convenient to allow throwing anything as the first  
>>>>> argument at the constructor (which is what Horde_Exception_Prior  
>>>>> does on top of Horde_Exception)
>>>>>
>>>>> 3) "this has to be consistent"
>>>>>
>>>>> I have been looking at the Kolab_Format package the last week and  
>>>>> I must say that I see no reason for it to extend from  
>>>>> Horde_Exception_Prior.
>>>>>
>>>>> It does not re-throw any exceptions as it is at the base of the  
>>>>> hierarchy. So Kolab_Format has no specific need to extend from  
>>>>> Horde_Exception. It would not use any of its functionality.
>>>>>
>>>>> The package also does not require any convenience methods when  
>>>>> constructing new exceptions. The only exception parameter is  
>>>>> usually just the message string. So Kolab_Format has no specific  
>>>>> need to extend from Horde_Exception_Prior either. It would not  
>>>>> use any of its functionality.
>>>>>
>>>>> This leaves "consistency" which is what I'd like to understand  
>>>>> first. My assumption is that this means that it should be  
>>>>> consistent on the application level. So that we can rely on  
>>>>> catching "Horde_Exception_Prior" and be certain that we can catch  
>>>>> anything coming from the underlying framework that way.
>>>>>
>>>>> If that is what you refer to with "this has to be consistent"  
>>>>> then I agree to the point that we need to know what to catch on  
>>>>> the application level. A generic "catch (Exception $e)" might not  
>>>>> be a good thing. But I would say that this still does not require  
>>>>> all framework libraries to extend from the same exception.
>>>>>
>>>>> Kolab_Format is at the base of the hierarchy. So it should not  
>>>>> care or know about the application level. Throwing a  
>>>>> Horde_Exception_Prior because some upper level might need it  
>>>>> seems like a problematic design choice to me. Kolab_Format should  
>>>>> care about its own business and ensure that it handles its  
>>>>> exceptions in a decent way.
>>>>>
>>>>> In case there is another layer on top using Kolab_Format it  
>>>>> should definitely ensure that it is able to deal with all  
>>>>> Exceptions that might be thrown from Kolab_Format. This would  
>>>>> refer to Kolab_Storage for example which is the main consumer for  
>>>>> Kolab_Format. It should always only throw its own type of  
>>>>> exceptions. If it would run into situations where a  
>>>>> Horde_Kolab_Format_Exception bubbles to the surface I would  
>>>>> consider this to be a bug of Kolab_Storage.
>>>>>
>>>>> Now Kolab_Storage could decide to extend from  
>>>>> Horde_Exception_Prior or it could happen in an exception class of  
>>>>> a package even further up in the chain. Nearer to the application  
>>>>> level.
>>>>>
>>>>> Kolab_Format is a good example to me because it is definitely  
>>>>> used stand-alone outside of any Horde context. And I don't see  
>>>>> any reason to force other consumers to pull in the  
>>>>> Horde_Exception package if that is not required.
>>>>>
>>>>> An additional remark: We should of course rename  
>>>>> Horde_Exception_Prior because I chose a real bad name for that  
>>>>> class. May "Horde_Exception_Base" or something like that would be  
>>>>> more appropriate.
>>>>
>>>> First of all, I still think it makes sense to have a common base  
>>>> exception class for all horde code, so that consumers (whether  
>>>> those are horde applications or external code using some horde  
>>>> library) can implement some catch-all for any exceptions thrown by  
>>>> horde code.
>>>
>>> As I argued above such a catch-all wouldn't be necessary if the  
>>> libraries only throw exceptions specific to this library.
>>>
>>>> I see that it makes sense to throw special purpose PHP/SPL  
>>>> extensions though, and we actually do this, which is inconsistent  
>>>> again. But that's not the inconsistency I care about right now.
>>>>
>>>> What I mean with consistency is that all horde exceptions should  
>>>> behave the same way. You may know that  
>>>> Horde_Kolab_Format_Exception is never going to wrap another  
>>>> extension/php error/pear error. But I don't. We have so many  
>>>> libraries, each using its own exception class, and I don't want do  
>>>> look up each exception's class definition to see how I have to use  
>>>> it, when I'm going to fix/code something in some arbitrary  
>>>> library. I want to be sure that any Horde exception class is  
>>>> working the same, so I can wrap it around any of the  
>>>> errors/exceptions that might occur during PHP development.
>>>>
>>>> This helps a lot during development and it doesn't add much  
>>>> overhead either. All library exceptions should extend a common  
>>>> horde exception anyway, and whether this is Horde_Exception or  
>>>> Horde_Exception_Prior doesn't really matter.
>>>
>>> If that is the consistency we seek (and Micheal S. confirmed that  
>>> he has similar needs) then I would say the most consistent  
>>> interface we can get is the one the base PHP Exception class  
>>> provides. This is even known to all PHP developers that don't know  
>>> Horde. And in case a package needs to use the feature of previous  
>>> exceptions it must extend Horde_Exception as we want to support PHP  
>>> 5.2.
>>>
>>> The drawbacks I see with this approach:
>>>
>>> - You can't catch Exceptions generically. But for all I know this  
>>> is only necessary if the called library has issues and passes  
>>> exceptions from lower levels without catching them. This basically  
>>> repeats my initial argument on how layered exceptions should work.
>>>
>>> - When using previous exceptions you always have to set a message  
>>> and a code. You can't omit that and if you want to reuse the  
>>> original message and code you have to use "throw new  
>>> Horde_Xyz_Exception($e->getMessage(), $e->getCode(), $e);". I  
>>> believe in most cases it actually makes sense to use a new message  
>>> and a new code as the context is different from the original source  
>>> of the error.
>>>
>>> The advantages I see:
>>>
>>> - no unnecessary dependencies
>>>
>>> - forces us to use a decent layered exception approach
>>>
>>> - very consistent exception interface known to all devs familiar with PHP5
>>
>> Maybe there's some misunderstanding. We don't need the consistency  
>> when catching and using Exceptions, but when *throwing* exceptions.  
>> And the base Exception is not offering the convenience wrappers that  
>> Horde_Exception_Prior provides.
>>
>
>I was referring to the throwing of exceptions, yes. The lack of the  
>convenience wrappers was what I considered a potential drawback. But  
>to me this is less important than the dependency that I feel is not  
>required. And I would like to see us developing libraries that only  
>throw exceptions specific to that library.
>
>> I don't have any problems with all Horde libraries requiring a  
>> dependency on the Horde_Exception package either.
>
>So, as everyone seems to be okay with adding this dependency I'll  
>probably better shut up now - ... and try to reopen it once PHP 5.2 is  
>a thing of the past ;)
>
>Thanks for the feedback.
>
>Gunnar
>
>-- 
>Core Developer
>The Horde Project
>
>e: wrobel at horde.org
>t: +49 700 6245 0000
>w: http://www.horde.org
>
>pgp: 9703 43BE
>tweets: http://twitter.com/pardus_de
>blog: http://log.pardus.de
>
>
>-- 
>Horde developers mailing list
>Frequently Asked Questions: http://horde.org/faq/
>To unsubscribe, mail: dev-unsubscribe at lists.horde.org