RE: [PHP-DEV] Regression between RC1 and RC2?

Hi, > -----Original Message----- > From: Derick Rethans [mailto:derick@php.net] > Sent: Monday, October 10, 2016 5:26 PM > To: Xinchen Hui <laruence@php.net> > Cc: Anatol Belski <anatol.php@belski.net>; Davey Shafik <davey@php.net>; > Nikita Popov <nikita.ppv@gmail.com>; PHP Developers Mailing List > <internals@lists.php.net> > Subject: Re: [PHP-DEV] Regression between RC1 and RC2? > > On Sun, 9 Oct 2016, Xinchen Hui wrote: > > > On Sun, Oct 9, 2016 at 2:06 AM, Anatol Belski <anatol.php@belski.net> wrote: > > > > > From: me@daveyshafik.com [mailto:me@daveyshafik.com] On Behalf Of > > > Davey Shafik > > > > > > > > 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) > > > > > > > > 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: > > > > > > > > > > > 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 > > > > however, I think latter is better, but maybe we could improve > > the fatal error message: > > Sorry, but that's making it a user problem, and not a technical problem for us to > solve correctly. I very much expected an Uncaught Exception here. > Ok, for 7.0.12, I've reverted 2d8ab51576695630a7471ff829cc5ea10becdc0f. Thus, there are yet two weeks till 7.0.13RC1 there to improve the patch. Thanks Anatol