Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Fleshgrinder]

On 6/18/2016 2:29 PM, Niklas Keller wrote:
> Which potential BC? The only thing is a catch all handler that has already
> been adjusted to PHP 7.
> 
> If you catch an exception you somehow promise to handle it. If you can't
> handle it, you should rethrow it.
> 
> I don't think there will be real issues with BC, because it would have
> otherwise resulted in a non-handleable fatal like before.
> 

Definitely no BC since such catch all handlers are usually meant to
(well) catch all and display a nice error message or page to the user
instead of a white page of death.

+1

-- 
Richard "Fleshgrinder" Fussenegger



signature.asc
Description: OpenPGP digital signature

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Niklas Keller]

Christoph Becker  schrieb am Sa., 18. Juni 2016, 12:34:

> On 17.06.2016 at 19:58, Rowan Collins wrote:
>
> > On 17/06/2016 17:47, Christoph Becker wrote:
> >> If something is required, one cannot do without it, so there's only one
> >> course of action, namely to bail out.  In my opinion, this is the least
> >> surprising way to handle missing required files, especially as it always
> >> has been that way (consider the potential BC break).
> >
> > As Alexander has pointed out, there is no BC break, unless you are using
> > an unconditional catch(Error $e) or catch(Throwable $e) - AKA the
> > "Pokemon block" (gotta catch 'em all!). Any uncaught Error is still
> > fatal, so amounts to zero change in behaviour.
> >
> > Note that this same line of argument can be applied to many classes of
> > error which became Throwable Errors in PHP 7.0, such as ParseError.
>
> Indeed, but that was a major version – this suggestion is about PHP 7.x.
>
> >> Frankly, I can't see a single
> >> good reason to replace the fatal error with an exception for require,
> >> but leave include etc. alone.  And if include would throw an exception,
> >> I don't see the use of changing require at all.
> >
> > Because changing include *would* be a breaking change, and in fact make
> > the distinction between require and include rather pointless, because
> > the whole point of include is that it *doesn't* emit anything fatal if
> > the file doesn't exist.
> >
> > The whole point of exceptions / throwables is that they are fatal by
> > default, but handlable in a consistent manner if there is some situation
> > that requires it.
> >
> > As for use cases, consider that you might not be the author of the code
> > containing the "require" statement - you might be including some third
> > party library that doesn't something wonky, and need to be robust. Or,
> > as Alexander points out, you might want to execute "finally" blocks on
> > the way out, even though the application is ultimately going to crash.
>
> Okay, I see now that there are some use cases, but would these justify
> the potential BC?  I'm not sure.
>

Which potential BC? The only thing is a catch all handler that has already
been adjusted to PHP 7.

If you catch an exception you somehow promise to handle it. If you can't
handle it, you should rethrow it.

I don't think there will be real issues with BC, because it would have
otherwise resulted in a non-handleable fatal like before.

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

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Christoph Becker]

On 17.06.2016 at 19:58, Rowan Collins wrote:

> On 17/06/2016 17:47, Christoph Becker wrote:
>> If something is required, one cannot do without it, so there's only one
>> course of action, namely to bail out.  In my opinion, this is the least
>> surprising way to handle missing required files, especially as it always
>> has been that way (consider the potential BC break).
> 
> As Alexander has pointed out, there is no BC break, unless you are using
> an unconditional catch(Error $e) or catch(Throwable $e) - AKA the
> "Pokemon block" (gotta catch 'em all!). Any uncaught Error is still
> fatal, so amounts to zero change in behaviour.
> 
> Note that this same line of argument can be applied to many classes of
> error which became Throwable Errors in PHP 7.0, such as ParseError.

Indeed, but that was a major version – this suggestion is about PHP 7.x.

>> Frankly, I can't see a single
>> good reason to replace the fatal error with an exception for require,
>> but leave include etc. alone.  And if include would throw an exception,
>> I don't see the use of changing require at all.
> 
> Because changing include *would* be a breaking change, and in fact make
> the distinction between require and include rather pointless, because
> the whole point of include is that it *doesn't* emit anything fatal if
> the file doesn't exist.
> 
> The whole point of exceptions / throwables is that they are fatal by
> default, but handlable in a consistent manner if there is some situation
> that requires it.
> 
> As for use cases, consider that you might not be the author of the code
> containing the "require" statement - you might be including some third
> party library that doesn't something wonky, and need to be robust. Or,
> as Alexander points out, you might want to execute "finally" blocks on
> the way out, even though the application is ultimately going to crash.

Okay, I see now that there are some use cases, but would these justify
the potential BC?  I'm not sure.

-- 
Christoph M. Becker


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

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Davey Shafik]

I'm +1 for this on require. I'd love to see exceptions for all file I/O
stuff (no more @fopen()!) but in all likelihood, we'd need a new API for
that to keep BC.

On Fri, Jun 17, 2016 at 1:32 PM, Niklas Keller  wrote:

> >
> > If something is required, one cannot do without it, so there's only one
> > course of action, namely to bail out.
>
>
> What about gracefully bailing out, like showing that composer dependencies
> have to be installed?
>
> It's just like a method call. Usually you expect a return value, unless
> there's an exception.
>
>
> > In my opinion, this is the least
> > surprising way to handle missing required files, especially as it always
> > has been that way (consider the potential BC break).
> >
>
> As already pointed out, there's no BC break, except for catch 'em all. It's
> also not surprising,
> as require + parse error already throws an error instead of stopping with a
> fatal error.
>
> It's surprising that this one still fatals.
>
>
> > Or do you really prefer to be able to do
> >
> > try {
> > require $fileName;
> > } catch (Error $e) {
> > echo "Oops, maybe deleted? " . $e;
> > }
> > functionDefinedInFileName();
> >
>
> Yes, I really prefer that. Not that code exactly, but being able to write:
>
> try {
>   require $config;
> } catch (ParseError $e) {
>
> }
>
>
> > and get a fatal error that function no() is undefined?  I'd rather get a
> > fatal error that the required file couldn't be opened in the first place.
> >
> > If, however, a file is not strictly required, one can (and in my
> > opinion, should) `include` the file.  And yes, there's no absolutely
> > failsafe way to do this without risking a warning or using the @
> > operator, but that affects other filesystem functions (e.g.
> > file_get_contents() and fopen()) as well.  Frankly, I can't see a single
> > good reason to replace the fatal error with an exception for require,
> > but leave include etc. alone.  And if include would throw an exception,
> > I don't see the use of changing require at all.
> >
> > --
> > Christoph M. Becker
> >
>

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Niklas Keller]

>
> If something is required, one cannot do without it, so there's only one
> course of action, namely to bail out.


What about gracefully bailing out, like showing that composer dependencies
have to be installed?

It's just like a method call. Usually you expect a return value, unless
there's an exception.


> In my opinion, this is the least
> surprising way to handle missing required files, especially as it always
> has been that way (consider the potential BC break).
>

As already pointed out, there's no BC break, except for catch 'em all. It's
also not surprising,
as require + parse error already throws an error instead of stopping with a
fatal error.

It's surprising that this one still fatals.


> Or do you really prefer to be able to do
>
> try {
> require $fileName;
> } catch (Error $e) {
> echo "Oops, maybe deleted? " . $e;
> }
> functionDefinedInFileName();
>

Yes, I really prefer that. Not that code exactly, but being able to write:

try {
  require $config;
} catch (ParseError $e) {

}


> and get a fatal error that function no() is undefined?  I'd rather get a
> fatal error that the required file couldn't be opened in the first place.
>
> If, however, a file is not strictly required, one can (and in my
> opinion, should) `include` the file.  And yes, there's no absolutely
> failsafe way to do this without risking a warning or using the @
> operator, but that affects other filesystem functions (e.g.
> file_get_contents() and fopen()) as well.  Frankly, I can't see a single
> good reason to replace the fatal error with an exception for require,
> but leave include etc. alone.  And if include would throw an exception,
> I don't see the use of changing require at all.
>
> --
> Christoph M. Becker
>

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Alexander Deruwe]

On Fri, Jun 17, 2016 at 6:47 PM, Christoph Becker  wrote:
>
> try {
> require $fileName;
> } catch (Error $e) {
> echo "Oops, maybe deleted? " . $e;
> }
> functionDefinedInFileName();

If anyone chooses to write this kind of code, then let them figure out
why their function is not defined. Who are we catering to here?


A.

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

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Rowan Collins]

On 17/06/2016 17:47, Christoph Becker wrote:

If something is required, one cannot do without it, so there's only one
course of action, namely to bail out.  In my opinion, this is the least
surprising way to handle missing required files, especially as it always
has been that way (consider the potential BC break).


As Alexander has pointed out, there is no BC break, unless you are using 
an unconditional catch(Error $e) or catch(Throwable $e) - AKA the 
"Pokemon block" (gotta catch 'em all!). Any uncaught Error is still 
fatal, so amounts to zero change in behaviour.


Note that this same line of argument can be applied to many classes of 
error which became Throwable Errors in PHP 7.0, such as ParseError.





Frankly, I can't see a single
good reason to replace the fatal error with an exception for require,
but leave include etc. alone.  And if include would throw an exception,
I don't see the use of changing require at all.


Because changing include *would* be a breaking change, and in fact make 
the distinction between require and include rather pointless, because 
the whole point of include is that it *doesn't* emit anything fatal if 
the file doesn't exist.


The whole point of exceptions / throwables is that they are fatal by 
default, but handlable in a consistent manner if there is some situation 
that requires it.


As for use cases, consider that you might not be the author of the code 
containing the "require" statement - you might be including some third 
party library that doesn't something wonky, and need to be robust. Or, 
as Alexander points out, you might want to execute "finally" blocks on 
the way out, even though the application is ultimately going to crash.


Regards,
--
Rowan Collins
[IMSoP]

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

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Christoph Becker]

On 17.06.2016 at 17:44, Niklas Keller wrote:

> 2016-06-17 11:46 GMT+02:00 Christoph Becker :
> 
>> On 17.06.2016 at 10:29, Alexander Lisachenko wrote:
>>
>>> Nikita, Dmitry, ping. What do you think, shouldn't we replace all
>> possible
>>> places with Fatal Errors with correct throwing of Error exceptions?  In
>>> this concrete case it's for "require" operator.
>>>
>>> From what I can see Error will give us more control over the file loading
>>> process and make it more atomic, because additional checks
>>> if(file_exists($file) && is_readable($file)) generate extra stat
>> commands,
>>> they are slow and not reliable, because for highload projects, we can be
>> in
>>> the situation where "if" succeeded, but the "require" will fail because
>> our
>>> file was deleted immediately after check.
>>>
>>> Code like this:
>>> try {
>>> require $fileName;
>>> } catch (Error $e) {
>>> echo "Oops, maybe deleted? " . $e;
>>> }
>>>
>>> is much more reliable than following, by using "include" instead of
>>> "require". This was suggested in https://bugs.php.net/bug.php?id=72089:
>>
>> I have re-opened this ticket.
>>
>>> if (file_exists($fileName) && is_readable($fileName)) {
>>> @include $fileName; // Silencing errors for the case of
>> race-condition,
>>> etc
>>> }
>>
>> I'm not generally against throwing exceptions from include (or several
>> other filesystem functions, for that matter), but the BC break has to be
>> considered.  I am, however, still opposed to offer the ability to catch
>> a failing *require*.
> 
> Why?

If something is required, one cannot do without it, so there's only one
course of action, namely to bail out.  In my opinion, this is the least
surprising way to handle missing required files, especially as it always
has been that way (consider the potential BC break).

Or do you really prefer to be able to do

try {
require $fileName;
} catch (Error $e) {
echo "Oops, maybe deleted? " . $e;
}
functionDefinedInFileName();

and get a fatal error that function no() is undefined?  I'd rather get a
fatal error that the required file couldn't be opened in the first place.

If, however, a file is not strictly required, one can (and in my
opinion, should) `include` the file.  And yes, there's no absolutely
failsafe way to do this without risking a warning or using the @
operator, but that affects other filesystem functions (e.g.
file_get_contents() and fopen()) as well.  Frankly, I can't see a single
good reason to replace the fatal error with an exception for require,
but leave include etc. alone.  And if include would throw an exception,
I don't see the use of changing require at all.

-- 
Christoph M. Becker

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

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Niklas Keller]

2016-06-17 11:46 GMT+02:00 Christoph Becker :

> On 17.06.2016 at 10:29, Alexander Lisachenko wrote:
>
> > Nikita, Dmitry, ping. What do you think, shouldn't we replace all
> possible
> > places with Fatal Errors with correct throwing of Error exceptions?  In
> > this concrete case it's for "require" operator.
> >
> > From what I can see Error will give us more control over the file loading
> > process and make it more atomic, because additional checks
> > if(file_exists($file) && is_readable($file)) generate extra stat
> commands,
> > they are slow and not reliable, because for highload projects, we can be
> in
> > the situation where "if" succeeded, but the "require" will fail because
> our
> > file was deleted immediately after check.
> >
> > Code like this:
> > try {
> > require $fileName;
> > } catch (Error $e) {
> > echo "Oops, maybe deleted? " . $e;
> > }
> >
> > is much more reliable than following, by using "include" instead of
> > "require". This was suggested in https://bugs.php.net/bug.php?id=72089:
>
> I have re-opened this ticket.
>
> > if (file_exists($fileName) && is_readable($fileName)) {
> > @include $fileName; // Silencing errors for the case of
> race-condition,
> > etc
> > }
>
> I'm not generally against throwing exceptions from include (or several
> other filesystem functions, for that matter), but the BC break has to be
> considered.  I am, however, still opposed to offer the ability to catch
> a failing *require*.


Why?

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Christoph Becker]

On 17.06.2016 at 10:29, Alexander Lisachenko wrote:

> Nikita, Dmitry, ping. What do you think, shouldn't we replace all possible
> places with Fatal Errors with correct throwing of Error exceptions?  In
> this concrete case it's for "require" operator.
> 
> From what I can see Error will give us more control over the file loading
> process and make it more atomic, because additional checks
> if(file_exists($file) && is_readable($file)) generate extra stat commands,
> they are slow and not reliable, because for highload projects, we can be in
> the situation where "if" succeeded, but the "require" will fail because our
> file was deleted immediately after check.
> 
> Code like this:
> try {
> require $fileName;
> } catch (Error $e) {
> echo "Oops, maybe deleted? " . $e;
> }
> 
> is much more reliable than following, by using "include" instead of
> "require". This was suggested in https://bugs.php.net/bug.php?id=72089:

I have re-opened this ticket.

> if (file_exists($fileName) && is_readable($fileName)) {
> @include $fileName; // Silencing errors for the case of race-condition,
> etc
> }

I'm not generally against throwing exceptions from include (or several
other filesystem functions, for that matter), but the BC break has to be
considered.  I am, however, still opposed to offer the ability to catch
a failing *require*.

> Pay an attention, that we always need to put the silencing operator here to
> prevent warnings, etc. This also hides all internal errors for parsing,
> thanks to the PHP. And we can't easily detect the reason why include was
> failed. Only possible way is to register error_handler and throw an
> instance of ErrorException.
> 
> But if require will throw an Error, nothing will changed, because unhandled
> Error will produce a Fatal Error, but all modern code will be able to
> handle this situation.

-- 
Christoph M. Becker


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

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Alexander Lisachenko]

Nikita, Dmitry, ping. What do you think, shouldn't we replace all possible
places with Fatal Errors with correct throwing of Error exceptions?  In
this concrete case it's for "require" operator.

>From what I can see Error will give us more control over the file loading
process and make it more atomic, because additional checks
if(file_exists($file) && is_readable($file)) generate extra stat commands,
they are slow and not reliable, because for highload projects, we can be in
the situation where "if" succeeded, but the "require" will fail because our
file was deleted immediately after check.

Code like this:
try {
require $fileName;
} catch (Error $e) {
echo "Oops, maybe deleted? " . $e;
}

is much more reliable than following, by using "include" instead of
"require". This was suggested in https://bugs.php.net/bug.php?id=72089:

if (file_exists($fileName) && is_readable($fileName)) {
@include $fileName; // Silencing errors for the case of race-condition,
etc
}

Pay an attention, that we always need to put the silencing operator here to
prevent warnings, etc. This also hides all internal errors for parsing,
thanks to the PHP. And we can't easily detect the reason why include was
failed. Only possible way is to register error_handler and throw an
instance of ErrorException.

But if require will throw an Error, nothing will changed, because unhandled
Error will produce a Fatal Error, but all modern code will be able to
handle this situation.

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Alexander Lisachenko]

2016-06-15 22:28 GMT+03:00 Niklas Keller :

> didn't read it was planned for all file functions.



Let's restrict my first suggestion only for "require" expression for now.
So proposal is following:

Rewrite "require" to throw an Error instead of current Fatal Error if some
file can not be loaded. This is fully BC and can be merged into 7.0 and
7.1, because it's impossible to catch a fatal error now. This will be
consistent with handling of eval errors - now we use the ParseError
exception instead of fatal errors.

In my opinion, all fatal errors should be replaced with exception throwing
(if possible), because it's more developer-friendly and allow us to handle
failures gracefully, invoking finally, destructors, etc.

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Niklas Keller]

Oh, just read the require example, didn't read it was planned for all file
functions.

Fleshgrinder  schrieb am Mi., 15. Juni 2016, 19:57:

> On 6/15/2016 7:52 PM, Niklas Keller wrote:
> > I think it can go into 7.1. It's not a BC break, as it will still
> produce a
> > fatal error if the error isn't caught.
> >
>
> file_get_contents() and friends emit an E_WARNING and not an E_ERROR or
> worse. ;)
>
> --
> Richard "Fleshgrinder" Fussenegger
>
>

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Fleshgrinder]

On 6/15/2016 7:52 PM, Niklas Keller wrote:
> I think it can go into 7.1. It's not a BC break, as it will still produce a
> fatal error if the error isn't caught.
> 

file_get_contents() and friends emit an E_WARNING and not an E_ERROR or
worse. ;)

-- 
Richard "Fleshgrinder" Fussenegger



signature.asc
Description: OpenPGP digital signature

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Niklas Keller]

2016-06-15 19:38 GMT+02:00 Fleshgrinder :

> On 6/15/2016 12:27 PM, Alexander Lisachenko wrote:
> > For PHP7 we have pretty errors for almost everything, even eval can
> throw a
> > ParseError exception, but not for the require expression, because it's
> > still producing an uncatchable fatal errors:
> >
> > try {
> > require('no.php');
> > } catch (Throwable $e) {
> > echo 'Catch!';
> > } finally {
> > echo 'Finally?';
> > }
> > // Warning: require(no.php): failed to open stream: No such file or
> > directory
> > // Fatal error: require(): Failed opening required 'no.php'
> >
>
> This is exactly how require is meant to work, just use include as
> Christoph said already. These are two distinct features for a reason.
>
> On 6/15/2016 12:27 PM, Alexander Lisachenko wrote:
> > Can we also add FileNotFoundException and use it for all file functions
> to
> > avoid silencing with "@" operator? Then require expression can throw an
> > Error with nested FileNotFoundException.
> >
>
> We definitely need this but it cannot go into PHP 7.0.


I think it can go into 7.1. It's not a BC break, as it will still produce a
fatal error if the error isn't caught.


>
> --
> Richard "Fleshgrinder" Fussenegger
>
>

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Fleshgrinder]

On 6/15/2016 12:27 PM, Alexander Lisachenko wrote:
> For PHP7 we have pretty errors for almost everything, even eval can throw a
> ParseError exception, but not for the require expression, because it's
> still producing an uncatchable fatal errors:
> 
> try {
> require('no.php');
> } catch (Throwable $e) {
> echo 'Catch!';
> } finally {
> echo 'Finally?';
> }
> // Warning: require(no.php): failed to open stream: No such file or
> directory
> // Fatal error: require(): Failed opening required 'no.php'
> 

This is exactly how require is meant to work, just use include as
Christoph said already. These are two distinct features for a reason.

On 6/15/2016 12:27 PM, Alexander Lisachenko wrote:
> Can we also add FileNotFoundException and use it for all file functions to
> avoid silencing with "@" operator? Then require expression can throw an
> Error with nested FileNotFoundException.
> 

We definitely need this but it cannot go into PHP 7.0.

-- 
Richard "Fleshgrinder" Fussenegger



signature.asc
Description: OpenPGP digital signature

Re: [PHP-DEV] Throwing an Error for require expressions in PHP7.x

[Rowan Collins]

On 15/06/2016 11:27, Alexander Lisachenko wrote:

For PHP7 we have pretty errors for almost everything, even eval can throw a
ParseError exception, but not for the require expression


Agree that this would be good. I don't know if there's a technical 
reason against it, or just that nobody's got round to it yet.




Can we also add FileNotFoundException and use it for all file functions to
avoid silencing with "@" operator?


This, however, is a completely different topic. It's one that's come up 
before, but not gained much traction, because it breaks an extremely 
large amount of existing code. I would suggest having a look in the list 
archives, and raising this as another topic if you can come up with a 
good plan for how it should work.


Regards,
--
Rowan Collins
[IMSoP]

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