Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96300
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain belski.net from 85.214.73.107 cause and error)
To: "'Xinchen Hui'" <laruence@php.net>
Cc: "'Davey Shafik'" <davey@php.net>,
	"'Nikita Popov'" <nikita.ppv@gmail.com>,
	"'Derick Rethans'" <derick@php.net>,
	"'PHP Developers Mailing List'" <internals@lists.php.net>
References: <alpine.DEB.2.20.1610071530340.3648@whisky.home.derickrethans.nl> <CAF+90c8F+jitXS4OGxy+b-BdhzdpGu3PvBhwB70eYLCFMqxohQ@mail.gmail.com> <CANaX5ndw-aTtAwepCNX90TzFdmX8dv3yMpjRpD83Be746Umiyg@mail.gmail.com> <010701d2218e$b2e12d60$18a38820$@belski.net> <CABBJUpcMVArH67tUOrNtVrD5U_cTdkDiXhEuX5oqVvp0cwhBNw@mail.gmail.com>
In-Reply-To: <CABBJUpcMVArH67tUOrNtVrD5U_cTdkDiXhEuX5oqVvp0cwhBNw@mail.gmail.com>
Date: Sun, 9 Oct 2016 13:49:02 +0200
Message-ID: <002001d22223$23363300$69a29900$@belski.net>
MIME-Version: 1.0
Content-Type: text/plain;
	charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Thread-Index: AQItOLydTqT+dIGA3YBv5mmFOlapewMWylwtAqhC8kICT73digIfoWNen5ha23A=
Content-Language: en-us
Subject: RE: [PHP-DEV] Regression between RC1 and RC2?
From: anatol.php@belski.net ("Anatol Belski")

Hi Hui,

> -----Original Message-----
> From: Xinchen Hui [mailto:laruence@php.net]
> Sent: Sunday, October 9, 2016 8:06 AM
> To: Anatol Belski <anatol.php@belski.net>
> Cc: Davey Shafik <davey@php.net>; Nikita Popov <nikita.ppv@gmail.com>;
> Derick Rethans <derick@php.net>; PHP Developers Mailing List
> <internals@lists.php.net>
> Subject: Re: [PHP-DEV] Regression between RC1 and RC2?
>=20
> Hey:
>=20
> On Sun, Oct 9, 2016 at 2:06 AM, Anatol Belski <anatol.php@belski.net> =
wrote:
>=20
> > Hi,
> >
> > > -----Original Message-----
> > > From: me@daveyshafik.com [mailto:me@daveyshafik.com] On Behalf Of
> > > Davey Shafik
> > > Sent: Friday, October 7, 2016 10:05 PM
> > > To: Nikita Popov <nikita.ppv@gmail.com>
> > > Cc: Derick Rethans <derick@php.net>; PHP Developers Mailing List
> > > <internals@lists.php.net>
> > > Subject: Re: [PHP-DEV] Regression between RC1 and RC2?
> > >
> > > Yes, we should not mask the exception. The behavior in =
7.0/7.1.0RC1
> > > is
> > much
> > > better IMO.
> > >
> > > (As seen here: https://3v4l.org/EJpD4#v700)
> > >
> > > - Davey
> > >
> > > On Fri, Oct 7, 2016 at 12:52 PM, Nikita Popov =
<nikita.ppv@gmail.com>
> > wrote:
> > >
> > > > On Fri, Oct 7, 2016 at 9:31 PM, Derick Rethans <derick@php.net> =
wrote:
> > > >
> > > > > Hi,
> > > > >
> > > > > I was looking at Xdebug for PHP 7.1, and I ran into the
> > > > > following
> > > > > inconsistency:
> > > > >
> > > > > https://3v4l.org/tHteN
> > > > >
> > > > > I first thought that Xdebug was messing up, but it seems like
> > > > > it's different behaviour in PHP itself. As I clearly return an
> > > > > array from __debugInfo, I don't think the new result is the =
correct one.
> > > > >
> > > > > cheers,
> > > > > Derick
> > > > >
> > > >
> > > > This is due to https://github.com/php/php-src/commit/
> > > > 2d8ab51576695630a7471ff829cc5ea10becdc0f, which landed in =
PHP-7.0
> > > > as
> > > well.
> > > > The problem is that __debugInfo currently is not able to handle
> > > > exceptions gracefully. I think we should revert this change for
> > > > now as it hides the fact that the underlying cause of the error =
is
> > > > an
> > exception.
> > > >
> > As far as I understand the bug #73067, it was about avoiding the =
fatal
> > error, not about avoiding the exception. Please correct if I'm =
wrong.
> > But given this, the fatal error still persists while the exception =
is removed.
> > It looks like it's doing the exact reversed to what one would expect =
-
> > no fatal error and the exception can be catched.
> >
> > I see that it's not yet released in 7.0, so I would prefer to revert
> > this in the release, at least. Hui, Nikita, do you think it's =
possible
> > to improve this for 7.0 in a follow up? I would revert in 7.0.12 and
> > there'll be room to fix it in the dev branch till 7.0.13. Otherwise
> > I'd suggest to revert to the previous behavior in 7.0+ and do a fix =
in
> > an appropriate higher branch.
> >
> The real problem is:
>=20
>     we are expecting __debugInfo always returns array,  but it doesn't =
if
> exception is threw.
>=20
>     so, if we keep the exception,  then we need inserts checks for =
exception (no
> array) after every debugInfo is called, or, make a FATAL error like =
the way I did
>=20
In the link sent from Derick https://3v4l.org/tHteN it's to see, that =
the other behavior is affected. In that case, the exception is thrown =
from the core, not from the user space, while __debuginfo() properly =
returns an array. This is the behavior, that patch to #73067 indirectly =
affects.

>     however,  I think latter is better,  but maybe we could improve =
the fatal error
> message:
>=20
>      " Fatal error,  __debugInfo must return an array, but an =
exception with " msg"
> is threw"
>=20
Yeah, that would be good, so the exception information is not lost. =
IMHO, if we could get rid of the fatal error completely, it'd be more =
user friendly. In that case the execution could continue, when exception =
is catched. From the performance POV, it's anyway some additional =
effort, either zend_clear_exception() or if(EG(exception)). But, if user =
chooses to call __debuginfo(), it's clearly not performance oriented =
anyway.

If having exception instead of fatal is not viable for safety reasons =
(or whatever), I'd take your suggestion to extend the fatal message with =
the exception text. Since the exception is not catchable in that case =
anyway, we'll at least keep the useful information.

Thanks

anatol