Re: [PHP-DEV] [RFC][Vote] Throw Error in Extensions

[Pascal MARTIN, AFUP]

Le 27/06/2016 17:17, Aaron Piotrowski a écrit :

Voting has opened on the RFC to change most conditions in extensions that raise 
E_ERROR or E_RECOVERABLE_ERROR to throw an instance of Error instead.


Hi,

At AFUP, we would be +1 on this RFC, as it fits well into the path 
started with PHP 7.0


Thanks for your work on this!

--
Pascal MARTIN, AFUP - French UG
http://php-intern...@afup.org

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

Re: [PHP-DEV] [RFC][Vote] Throw Error in Extensions

[Aaron Piotrowski]

Hi Chris,

> On Jun 30, 2016, at 6:15 AM, Christopher Jones  
> wrote:
> 
> Hi Aaron,
> 
> I was someone who spent time on the RFC template to try and get better
> quality and to capture more information about each RFC.  I think your
> RFC needs a lot more content before going to the vote.
> 
> Chris
> 

It was unclear if these changes even needed a formal RFC, but the RMs felt it 
would be better to have an RFC to ensure extension maintainers and others were 
aware of and agreed with the changes to be made. The RFC was short because I 
felt there wasn't much to say. However, I have added to the RFC a list of 
extensions and under what conditions an instance of Error will be thrown 
instead of a fatal or recoverable fatal error. Hopefully this is along the 
lines of what you were looking for.

If possible, I would still like each extension maintainer to take a look at the 
changes made to ensure I have not missed cleaning up cases where bail-out 
behavior was being relied upon.

Cheers!

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

Re: [PHP-DEV] [RFC][Vote] Throw Error in Extensions

[Dan Ackroyd]

On 30 June 2016 at 01:32, Pierre Joye  wrote:
>> It's not a language change, so doesn't seem to meet the criteria for
>> needing a 2/3 pass rate.
>
> It is changing the core like ext/standard and other core functions.

That's true, but doesn't appear to be the required test. From the Voting RFC:

> a feature affecting the language itself (new syntax for example) will be 
> considered as 'accepted'
> if it wins a 2/3 of the votes. Other RFCs require 50% + 1 votes to get 
> 'accepted'.

Although the core functions ship as PHP they aren't part of the
language, so 50% + 1 seems to appropriate.

cheers
Dan

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

Re: [PHP-DEV] [RFC][Vote] Throw Error in Extensions

[Christopher Jones]

On 28/06/2016 1:17 AM, Aaron Piotrowski wrote:

Hello,

Voting has opened on the RFC to change most conditions in extensions that raise 
E_ERROR or E_RECOVERABLE_ERROR to throw an instance of Error instead.

RFC: https://wiki.php.net/rfc/throw_error_in_extensions 

PR: https://github.com/php/php-src/pull/1942 


Aaron Piotrowski



Hi Aaron,

I was someone who spent time on the RFC template to try and get better
quality and to capture more information about each RFC.  I think your
RFC needs a lot more content before going to the vote.

Chris

--
http://twitter.com/ghrd

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

Re: [PHP-DEV] [RFC][Vote] Throw Error in Extensions

[Aaron Piotrowski]

Hi Dan,

> On Jun 29, 2016, at 3:41 PM, Dan Ackroyd  wrote:
> 
> For the record, I'm beginning to think the RFC process should probably
> be slightly more orchestrated, and RFCs should have a "pre-vote"
> announcement at least one week before the vote actually opens, when
> the RFC author thinks the discussion of the RFC is complete.
> 
> This point would be the time for the implementations full impact on
> the PHP engine to be analyzed, and also when the final voting choice
> can be discussed/challenged before the voting is actually open.
> 

Sorry if this RFC was a little rushed. I originally hoped to make these changes 
for 7, then the original PR got pushed off to 7.1 and forgotten about until it 
was almost too late even for 7.1.

I agree that a pre-vote phase could be useful, as it may help bring more 
attention to an RFC before voting actually opens. There seems to be a pattern 
of issues with RFCs not being discussed until voting has begun. The pre-vote 
phase could replace one week of the discussion period, so an RFC would still 
only need a minimum of two weeks between announcement and voting.

Cheers!

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

Re: [PHP-DEV] [RFC][Vote] Throw Error in Extensions

[Pierre Joye]

On Jun 30, 2016 3:41 AM, "Dan Ackroyd"  wrote:
>
> On 28 June 2016 at 03:36, Pierre Joye  wrote:
> >
> > I like the idea.
> >
> > It should be 2/3 tho'.
>
> Why?
>
> It's not a language change, so doesn't seem to meet the criteria for
> needing a 2/3 pass rate.

It is changing the core like ext/standard and other core functions.

Re: [PHP-DEV] [RFC][Vote] Throw Error in Extensions

[Dan Ackroyd]

On 28 June 2016 at 03:36, Pierre Joye  wrote:
>
> I like the idea.
>
> It should be 2/3 tho'.

Why?

It's not a language change, so doesn't seem to meet the criteria for
needing a 2/3 pass rate.

For the record, I'm beginning to think the RFC process should probably
be slightly more orchestrated, and RFCs should have a "pre-vote"
announcement at least one week before the vote actually opens, when
the RFC author thinks the discussion of the RFC is complete.

This point would be the time for the implementations full impact on
the PHP engine to be analyzed, and also when the final voting choice
can be discussed/challenged before the voting is actually open.

cheers
Dan

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

Re: [PHP-DEV] [RFC][Vote] Throw Error in Extensions

[Stanislav Malyshev]

Hi!

>> Voting has opened on the RFC to change most conditions in extensions that 
>> raise E_ERROR or E_RECOVERABLE_ERROR to throw an instance of Error instead.
>>
>> RFC: https://wiki.php.net/rfc/throw_error_in_extensions 
>> 
>> PR: https://github.com/php/php-src/pull/1942 
>> 

Isn't there a case that php_error(E_ERROR) does not return? At least it
was in 5.x, I'm not sure if that didn't change. If so, we need to be
very careful here - some code may make assumptions about the things
because of previous E_ERROR conditions, and if zend_throw_error returns
where php_error didn't there might be subtle and dangerous bugs.

-- 
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][Vote] Throw Error in Extensions

[Aaron Piotrowski]

Hi Jakub,

> On Jun 28, 2016, at 12:28 PM, Jakub Zelenka  wrote:
> 
> Hi,
>> 
>> 
> Just noticed the openssl case in X509_digest and it's obviously oversight
> by whoever added that bit because it should be warning as it's for all
> other similar fails. I'm going to change it to warning to make it
> consistent.

I noticed most others were warnings in openssl, but I did not want to make 
assumptions about what level an error should be. If an error was E_ERROR, I 
assumed there was a reason it was fatal. If you change it to a warning I'll be 
sure not to overwrite this when merging the patch.

> In general I agree with the idea but the patch should be a bit more
> sensible and considers consistency with other errors in the extension. It
> should be also reviewed by all active maintainers or regular contributors
> to the changed extensions before it gets merged so it might be a bit late
> for 7.1

I agree, either the maintainers or someone very familiar with each extension 
should examine the changes before merging. Note that this patch is only meant 
to allow catching and handling of otherwise fatal errors, not to modify overall 
error handling in each extension. I would rather individual extension 
maintainers make decisions on error levels.

Thanks!

Aaron Piotrowski



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

Re: [PHP-DEV] [RFC][Vote] Throw Error in Extensions

[Jakub Zelenka]

Hi,

On Mon, Jun 27, 2016 at 4:17 PM, Aaron Piotrowski  wrote:

> Hello,
>
> Voting has opened on the RFC to change most conditions in extensions that
> raise E_ERROR or E_RECOVERABLE_ERROR to throw an instance of Error instead.
>
> RFC: https://wiki.php.net/rfc/throw_error_in_extensions <
> https://wiki.php.net/rfc/throw_error_in_extensions>
> PR: https://github.com/php/php-src/pull/1942 <
> https://github.com/php/php-src/pull/1942>
>
>
Just noticed the openssl case in X509_digest and it's obviously oversight
by whoever added that bit because it should be warning as it's for all
other similar fails. I'm going to change it to warning to make it
consistent.

In general I agree with the idea but the patch should be a bit more
sensible and considers consistency with other errors in the extension. It
should be also reviewed by all active maintainers or regular contributors
to the changed extensions before it gets merged so it might be a bit late
for 7.1

Cheers

Jakub

Re: [PHP-DEV] [RFC][Vote] Throw Error in Extensions

[Pierre Joye]

hi,

On Mon, Jun 27, 2016 at 10:17 PM, Aaron Piotrowski  wrote:
> Hello,
>
> Voting has opened on the RFC to change most conditions in extensions that 
> raise E_ERROR or E_RECOVERABLE_ERROR to throw an instance of Error instead.
>
> RFC: https://wiki.php.net/rfc/throw_error_in_extensions 
> 
> PR: https://github.com/php/php-src/pull/1942 
> 

I like the idea.

It should be 2/3 tho'.

Also "Generally none, though it is possible some exceptions thrown
could be unintentionally caught by code written for PHP 7. However, it
is rare for Errorexceptions to be caught outside of cleanup or
logging, so catching these exceptions is likely desirable over a fatal
error." is a BC break, not sure there are codes out there that expects
or may behave (more) badly with this than with a fatal error.

Cheers,
-- 
Pierre

@pierrejoye | http://www.libgd.org

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

[PHP-DEV] [RFC][Vote] Throw Error in Extensions

[Aaron Piotrowski]

Hello,

Voting has opened on the RFC to change most conditions in extensions that raise 
E_ERROR or E_RECOVERABLE_ERROR to throw an instance of Error instead.

RFC: https://wiki.php.net/rfc/throw_error_in_extensions 

PR: https://github.com/php/php-src/pull/1942 


Aaron Piotrowski