Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:83207
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 209.85.220.178 as permitted sender)
MIME-Version: 1.0
In-Reply-To: <CAHXmDMCc3JBxj4=7tLR4XZ9_r30kueSXdakrtonN0nnRkAK5Ww@mail.gmail.com>
References: <CAF+90c-7w1fLqmffYvPRhmtt8rzESEpSmW8vfMD-YfLt1mG_8w@mail.gmail.com>
	<CAF+90c8NAROuCDgRajF4AN2oTc4fzmxTjtVC_OAQH=ZFm5N9Uw@mail.gmail.com>
	<CA+kxMuREL5RfEyB7cJhQ6PK+zHsb2wLVcaWxB=0KveYRrRYSsg@mail.gmail.com>
	<CA+9eiLscm+oWZMZzPw75-93cw5HC6siOGL+S3oey=TThGP19Rw@mail.gmail.com>
	<CAHXmDMCc3JBxj4=7tLR4XZ9_r30kueSXdakrtonN0nnRkAK5Ww@mail.gmail.com>
Date: Thu, 19 Feb 2015 19:27:56 +0400
Message-ID: <CA+9eiLu8o8SAXE=frORQvYGTA4w3ZrEEerzWiQXftJFVTsptrA@mail.gmail.com>
To: Trevor Suarez <rican7@gmail.com>
Cc: Dan Ackroyd <danack@basereality.com>, Nikita Popov <nikita.ppv@gmail.com>, 
	PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary=089e0115f048882bfa050f7294d2
Subject: Re: [PHP-DEV] Re: [RFC] Exceptions in the engine
From: dmitry@zend.com (Dmitry Stogov)

--089e0115f048882bfa050f7294d2
Content-Type: text/plain; charset=UTF-8

On Thu, Feb 19, 2015 at 6:14 PM, Trevor Suarez <rican7@gmail.com> wrote:

> I think that structure makes sense Dmitry.
>
> Though, just a bit on naming here (I know, its not a big deal, and naming
> is hard, haha):
>
> I think that naming the new parent exception something like "Throwable" or
> "Catchable" (as Nikita previously suggested) would be a bit more concise in
> meaning than "BaseException". You may not have even meant that name as a
> formal proposal, but I figured I'd weigh in regardless. :P
>

We thought about "Throwable" or "Catchable" interface, but this change
would require more changes and will make more BC breaks.

Thanks. Dmitry.


>
> - Trevor
>
>
> On Thu Feb 19 2015 at 4:55:38 AM Dmitry Stogov <dmitry@zend.com> wrote:
>
>> On Wed, Feb 18, 2015 at 10:30 PM, Dan Ackroyd <danack@basereality.com>
>> wrote:
>>
>> > On 13 February 2015 at 23:25, Nikita Popov <nikita.ppv@gmail.com>
>> wrote:
>> > > Subclassing: Should there be more specific subclasses of
>> EngineException
>> > > for particular errors?
>> >
>> > It's not obvious that any subclasses would be useful. However using
>> > the code to specify the exact type of error, rather than having to
>> > inspect the message would be good.
>> >
>> > > Should EngineException inherit from Exception (and as such be
>> > > subject to catch(Exception)) or should we introduce some kind
>> > > of special super-class that is not caught by default
>> >
>> > Even ignoring the BC problem with having EngineExceptions extending
>> > Exception, I think the EngineException needs to be in a different
>> > hierarchy to Exception to be able to write reasonable code in the
>> > future
>> >
>> > Without having EngineException in a separate hierarchy of exceptions,
>> > the code below will catch exceptions where the data is 'ok' but there
>> > was a problem with the code, and continue to process items. This is
>> > almost certainly not the correct behaviour when an EngineException has
>> > been encountered.
>> >
>> > interface Service {
>> >     function foo($item);
>> > }
>> >
>> >
>> > function processData(array $itemsToProcess, service $service) {
>> >     foreach ($itemsToProcess as $item) {
>> >         try {
>> >             $service->foo($item);
>> >         }
>> > // Because $service can throw an Exception that is specific to the
>> > // implementation we have to catch \Exception, unless we are going
>> > // to list all possible implementation specific exception types here.
>> > // That would be a subtle case of strong coupling, and when a new
>> > // implementation is made the new exception type would need to
>> > // be added here.
>> >         catch(\Exception $e) {
>> >             // item was not processable but PHP engine is OK.
>> >             $item->markAsErrored();
>> >             //Go on to process the next item
>> >         }
>> >     }
>> > }
>> >
>> >
>> > To avoid having EngineExceptions in a separate hierarchy, this
>> > function could be converted to:
>> >
>> > function processData(array $itemsToProcess, service $service) {
>> >     foreach ($itemsToProcess as $item) {
>> >         try {
>> >             $service->foo($item);
>> >         }
>> >         catch(\EngineException $ee) {
>> >             //PHP engine is not stable - lets get out of here.
>> >             throw $ee; //or throw new ProcessException($ee)
>> >         }
>> >         catch(\Exception $e) {
>> >             $item->markAsErrored();
>> >         }
>> >     }
>> > }
>> >
>> > However that is bad as i)it's boiler plate to do the correct behaviour
>> > ii) you have to remember to do that everywhere. Having to remember to
>> > do the correct thing, is going to lead to people forgetting.
>> >
>> > It will still be necessary to catch all types of Exception in a single
>> > catch block i.e. at the top level of a script to prevent exceptions
>> > being shown to the end user. This could be made easier by having a
>> > common super class for Exception and EngineException. However having
>> > one try block that is required to have multiple catch statements to
>> > catch all different types of exception is not that much of a burden:
>> >
>> > try {
>> >     runApp();
>> > }
>> > catch(EngineException $e) {
>> >     handleException($ee);
>> > }
>> > catch(Exception $e) {
>> >     handleException($e);
>> > }
>> >
>> > As that would be the only place it would be required to catch both
>> types.
>> >
>> > TL:DR EngineException needs to not extend Exception, whether we need a
>> > super class is not as clear.
>> >
>> > cheers
>> > Dan
>> >
>> > --
>> > PHP Internals - PHP Runtime Development Mailing List
>> > To unsubscribe, visit: http://www.php.net/unsub.php
>> >
>> >
>> I think we may introduce the following hierarchy
>>
>> abstarct class BaseException {
>> }
>> class Exception extends BaseException {
>> }
>> class EngineException extends BaseException {
>> }
>>
>> the existing code that caught Exception is going to be unaffected.
>> New code may decide to catch engine exception in separate catch block
>> (using EngineException) or in single block (using BaseException)
>>
>> Thanks. Dmitry.
>>
>

--089e0115f048882bfa050f7294d2--