[dev] Dealing with Exceptions

Tue Jan 11 14:11:41 UTC 2011

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 don't have any problems with all Horde libraries requiring a  
dependency on the Horde_Exception package either.

Jan.

-- 
Do you need professional PHP or Horde consulting?
http://horde.org/consulting/