Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:85619
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain basereality.com from 209.85.215.45 cause and error)
MIME-Version: 1.0
In-Reply-To: <CAO3MkaDv+AmLGNWH4CThDJ1qvMVwxOJefC3VMj5ij3-+gLs-5Q@mail.gmail.com>
References: <CAO3MkaDv+AmLGNWH4CThDJ1qvMVwxOJefC3VMj5ij3-+gLs-5Q@mail.gmail.com>
Date: Wed, 1 Apr 2015 01:59:40 +0000
Message-ID: <CA+kxMuQnxkuK8WQpU7S8fOngAHW3UsGUCHROtScH+qOK=N6s1w@mail.gmail.com>
To: James Gilliland <neclimdul@gmail.com>
Cc: "internals@lists.php.net" <internals@lists.php.net>
Content-Type: text/plain; charset=UTF-8
Subject: Re: [PHP-DEV] Backwards compatibility with set_exception_handler
 callback and type hints
From: danack@basereality.com (Dan Ackroyd)

Hi James,

On 31 March 2015 at 21:51, James Gilliland <neclimdul@gmail.com> wrote:
> By design, \EngineException does not extend \Exception so code doesn't
> accidentally catch this special type of exception. ...
>
> I don't know if this is all acceptable and/or by design but it is awkward
> so I wanted to bring it to the list to discuss.


Let me try to explain why the BC break at the top level is the right choice.

The change in the Exception hierarchy has to cause a BC break to exist
'somewhere', as errors in code that were previously not throwing an
exception, but in effect just doing exit() are now throwing an
exception. This is a change in behaviour that cannot be handled
without some sort of BC break.

There are two types of places where \Exception are currently being caught:

i) in the middle of applications where some library is being called
where all exceptions need to be caught and changed to a more specific
exception.

function foo() {
    try {
        someOtherLibrary();
    }
    catch (\Exception $e) {
        throw new OtherLibraryException(
            $e->getMessage(),
            $e->getCode,
           $e
        );
    }
}


ii) At the top level of the application where all exceptions are being
caught, so that they can be logged and a more graceful error message
than a stack trace can be shown to the user.

If the BC break was for (i) by making all of the new exceptions be
under the hierarchy of \Exception, it would result in a lot of BC
break spread out across applications.

Having the BC break in (ii) is pretty strongly preferable; it means
that there are just a few (or one), easy to find places in an
application where the code now needs to be changed from catching
\Exception to catching \BaseException (or whatever it will be after
the exception tidy up).

Drupal (or any other application) can handle this BC break reasonably
easily by either dropping the type from the parameter of
_default_exception_handler or by adding a version check around it's
declaration like:

if (PHP_VERSION_ID >= 700000) {
    function _default_exception_handler(\BaseException $e) {
    }
}
else {
    function _default_exception_handler(\Exception $e) {
    }
}

Yeah, this isn't the most awesome thing ever to have to do, but imo it
sure beats having to change code in the middle of applications.

cheers
Dan