Hi Hui, > -----Original Message----- > From: Xinchen Hui [mailto:larue...@php.net] > Sent: Sunday, October 9, 2016 8:06 AM > To: Anatol Belski <anatol....@belski.net> > Cc: Davey Shafik <da...@php.net>; Nikita Popov <nikita....@gmail.com>; > Derick Rethans <der...@php.net>; PHP Developers Mailing List > <internals@lists.php.net> > Subject: Re: [PHP-DEV] Regression between RC1 and RC2? > > Hey: > > On Sun, Oct 9, 2016 at 2:06 AM, Anatol Belski <anatol....@belski.net> wrote: > > > Hi, > > > > > -----Original Message----- > > > From: m...@daveyshafik.com [mailto:m...@daveyshafik.com] On Behalf Of > > > Davey Shafik > > > Sent: Friday, October 7, 2016 10:05 PM > > > To: Nikita Popov <nikita....@gmail.com> > > > Cc: Derick Rethans <der...@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....@gmail.com> > > wrote: > > > > > > > On Fri, Oct 7, 2016 at 9:31 PM, Derick Rethans <der...@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: > > we are expecting __debugInfo always returns array, but it doesn't if > exception is threw. > > 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 > 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: > > " Fatal error, __debugInfo must return an array, but an exception with > " msg" > is threw" > 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 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
