Re: [PHP-DEV] [RFC DISCUSSION] Error Storage Behavior

[Fleshgrinder]

On 7/13/2016 10:29 PM, Nikita Popov wrote:
> I think there is some confusion about this change because the description
> was unclear.
> 
> The change is **only** about EH_THROW handling. EH_THROW is used by
> extensions to replace warnings etc with exceptions. Currently the exception
> is thrown, but error_get_last() still behaves as if the warning was thrown
> instead. Fixing this does seem like a normal bug fix to me. Note that this
> does not touch exceptions thrown from error handlers or similar in any way.
> 
> Basically the problem is that an extension using zend_throw_exception
> directly and an exception throwing through EH_THROW will behave
> differently, while there should be no difference.
> 
> The part about behavior being dependent on whether you catch the exception
> or not: This is true only insofar as an uncaught exception generates a
> fatal error and that fatal error is retrievable through error_get_last().
> There is no special casing occurring here.
> 
> Nikita
> 

The thing that bugs me is the fact that I could not find a single
situation in which error_get_last() contains a warning although an
exception was thrown other than in the DateTime example from the
original bug report.

I do not want to simply report this as a bug fix without fully
understanding its implications. In other words: why does
error_get_last() sometimes contain a warning and sometimes doesn't?!?
This is why I opened this thread, to get some insights from you guys
since you know internals much better.

-- 
Richard "Fleshgrinder" Fussenegger



signature.asc
Description: OpenPGP digital signature

Re: [PHP-DEV] [RFC DISCUSSION] Error Storage Behavior

[Nikita Popov]

On Mon, Jun 27, 2016 at 9:53 PM, Stanislav Malyshev 
wrote:

> Hi!
>
> > Currently error_get_last() always contains the last error that occurred,
> > however, this is actually not desired if the last error was an exception
> > that was caught.
> >
> > https://github.com/php/php-src/pull/1936
>
> I think conditioning warnings on whether exception is caught or not is a
> very bad idea. However, having _either_ exception _or_ PHP error and not
> producing both (except in the case where uncaught exception becomes
> fatal error I guess) may be a good idea, but it needs careful checking
> of the implications. In general, old error mechanism and exceptions are
> not very good combination, and using them together may be troublesome.
>
> It's definitely not "just a bug fix" - it needs careful check of what
> exactly happens when.
>

I think there is some confusion about this change because the description
was unclear.

The change is **only** about EH_THROW handling. EH_THROW is used by
extensions to replace warnings etc with exceptions. Currently the exception
is thrown, but error_get_last() still behaves as if the warning was thrown
instead. Fixing this does seem like a normal bug fix to me. Note that this
does not touch exceptions thrown from error handlers or similar in any way.

Basically the problem is that an extension using zend_throw_exception
directly and an exception throwing through EH_THROW will behave
differently, while there should be no difference.

The part about behavior being dependent on whether you catch the exception
or not: This is true only insofar as an uncaught exception generates a
fatal error and that fatal error is retrievable through error_get_last().
There is no special casing occurring here.

Nikita

Re: [PHP-DEV] [RFC DISCUSSION] Error Storage Behavior

[Fleshgrinder]

On 6/27/2016 9:53 PM, Stanislav Malyshev wrote:
> Hi!
> 
>> Currently error_get_last() always contains the last error that occurred,
>> however, this is actually not desired if the last error was an exception
>> that was caught.
>>
>> https://github.com/php/php-src/pull/1936
> 
> I think conditioning warnings on whether exception is caught or not is a
> very bad idea. However, having _either_ exception _or_ PHP error and not
> producing both (except in the case where uncaught exception becomes
> fatal error I guess) may be a good idea, but it needs careful checking
> of the implications. In general, old error mechanism and exceptions are
> not very good combination, and using them together may be troublesome.
> 
> It's definitely not "just a bug fix" - it needs careful check of what
> exactly happens when.
> 

This is exactly how I see this situation too and why I mailed the list
while asking for feedback.

I think the actual problem from the bug report is not related to the
provided patch that I updated a bit, the problem is as far as I can tell
within the DateTime::__construct method were the error mode is changed
to E_THROW but upon throwing a warning is generated as well. I had a
look at other implementations that change the mode but they did not
behave the same.

I am not familiar enough with all the code yet to simply point out what
is actually wrong but will definitely invest more time unless somebody
else who knows more does and simply fixes it.

-- 
Richard "Fleshgrinder" Fussenegger



signature.asc
Description: OpenPGP digital signature

Re: [PHP-DEV] [RFC DISCUSSION] Error Storage Behavior

[Stanislav Malyshev]

Hi!

> Currently error_get_last() always contains the last error that occurred,
> however, this is actually not desired if the last error was an exception
> that was caught.
> 
> https://github.com/php/php-src/pull/1936

I think conditioning warnings on whether exception is caught or not is a
very bad idea. However, having _either_ exception _or_ PHP error and not
producing both (except in the case where uncaught exception becomes
fatal error I guess) may be a good idea, but it needs careful checking
of the implications. In general, old error mechanism and exceptions are
not very good combination, and using them together may be troublesome.

It's definitely not "just a bug fix" - it needs careful check of what
exactly happens when.

-- 
Stas Malyshev
smalys...@gmail.com

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC DISCUSSION] Error Storage Behavior

[Dan Ackroyd]

On 6/11/2016 5:48 PM, Fleshgrinder wrote:
> Currently error_get_last() always contains the last error that occurred,
> however, this is actually not desired if the last error was an exception
> that was caught.

Changing the behaviour of how errors are handled, to depend on what
happens after the error occurs, seems a really bad choice to me.

>From https://bugs.php.net/bug.php?id=54043
> If you create a custom error handler which converts all PHP errors to 
> exceptions,
> catching internal exceptions inline therefore throws another Exception in the 
> custom error handler,

Can you provide a reproduce case for this? The code below has a custom
error handler; it is never called.

It's actually not that clear what problem you are trying to solve here.

> Should we treat it as a simple bug fix?

No, I don't think we can. First, wrong behaviour is a design flaw not
a bug, and second it's really not obvious that having weird complex
behaviour would be preferrable.

On 26 June 2016 at 08:08, Fleshgrinder  wrote:
> Any feedback here?

This month you've sent almost 4 emails a day to this list. That is an
incredibly high number for a list that is distributed to thousands of
people. This list would be much better served if there were fewer
emails, that contained more fully thought through proposals.

cheers
Dan





getMessage());
}

echo 'END' . PHP_EOL;


function shutdown()
{
$error = error_get_last();
var_dump('Error ', @$error);
}

function tierErrorHandler($errorNumber, $errorMessage, $errorFile, $errorLine) {
echo "Is this called?" . PHP_EOL;
$message = "Error: [$errorNumber] $errorMessage in file $errorFile
on line $errorLine\n";

throw new \Exception($message);
}

-- 
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php

Re: [PHP-DEV] [RFC DISCUSSION] Error Storage Behavior

[Fleshgrinder]

On 6/11/2016 5:48 PM, Fleshgrinder wrote:
> Currently error_get_last() always contains the last error that occurred,
> however, this is actually not desired if the last error was an exception
> that was caught.
> 
> https://github.com/php/php-src/pull/1936
> 
> With the above change only uncaught exceptions are retrievable via
> error_get_last(). This is consistent with the behavior of custom error
> handlers that successfully handle the error; they are not retrievable
> via error_get_last(0.
> 
> Note that this change is a breaking change since users might rely on
> exactly this behavior. However, it is undocumented and there is a bug
> report for it too:
> 
> https://bugs.php.net/bug.php?id=54043
> 

Any feedback here? Should we treat it as a simple bug fix?

-- 
Richard "Fleshgrinder" Fussenegger



signature.asc
Description: OpenPGP digital signature

[PHP-DEV] [RFC DISCUSSION] Error Storage Behavior

[Fleshgrinder]

Currently error_get_last() always contains the last error that occurred,
however, this is actually not desired if the last error was an exception
that was caught.

https://github.com/php/php-src/pull/1936

With the above change only uncaught exceptions are retrievable via
error_get_last(). This is consistent with the behavior of custom error
handlers that successfully handle the error; they are not retrievable
via error_get_last(0.

Note that this change is a breaking change since users might rely on
exactly this behavior. However, it is undocumented and there is a bug
report for it too:

https://bugs.php.net/bug.php?id=54043

-- 
Richard "Fleshgrinder" Fussenegger



signature.asc
Description: OpenPGP digital signature