Hi, internals!
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'
See https://3v4l.org/sQeAQ for live demo. What are the reasons not to throw
Error here? Looks like an oversight for me.
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.
Thoughts?
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]
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'See https://3v4l.org/sQeAQ for live demo. What are the reasons not to throw
Error here? Looks like an oversight for me.
This had recently been reported as feature request, and my reply was:
| With regard to your example: if a file is required and it can't
| be included, it doesn't make much sense to go on. If on the other
| hand the file is not really required, just include it.
[1] https://bugs.php.net/72089
--
Christoph M. Becker
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.
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
2016-06-15 19:38 GMT+02:00 Fleshgrinder php@fleshgrinder.com:
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.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
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
Oh, just read the require example, didn't read it was planned for all file
functions.
Fleshgrinder php@fleshgrinder.com schrieb am Mi., 15. Juni 2016, 19:57:
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 anE_WARNINGand not anE_ERRORor
worse. ;)--
Richard "Fleshgrinder" Fussenegger
2016-06-15 22:28 GMT+03:00 Niklas Keller me@kelunik.com:
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.
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.
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
2016-06-17 11:46 GMT+02:00 Christoph Becker cmbecker69@gmx.de:
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?
2016-06-17 11:46 GMT+02:00 Christoph Becker cmbecker69@gmx.de:
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
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]
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
Christoph Becker cmbecker69@gmx.de schrieb am Sa., 18. Juni 2016, 12:34:
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
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
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.
> 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
stuff (no more @fopen()!) but in all likelihood, we'd need a new API for
that to keep BC.
> >
> > 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