Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » Throwing an Error for require expressions in PHP7.x »

Throwing an Error for require expressions in PHP7.x

php.internals

Alexander Lissachenko

10 years ago
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?

Rowan Collins

10 years ago
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]

Christoph Becker

10 years ago
On 15.06.2016 at 12: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, 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

Fleshgrinder

10 years ago
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

Niklas Keller

10 years ago
2016-06-15 19:38 GMT+02:00 Fleshgrinder <php@fleshgrinder.com>:
> 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.

Fleshgrinder

10 years ago
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

Niklas Keller

10 years ago
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:

Alexander Lissachenko

10 years ago
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.

Alexander Lissachenko

10 years ago
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.

Christoph Becker

10 years ago
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

Niklas Keller

10 years ago
2016-06-17 11:46 GMT+02:00 Christoph Becker <cmbecker69@gmx.de>:
> 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?

Christoph Becker

10 years ago
On 17.06.2016 at 17:44, Niklas Keller wrote:
> 2016-06-17 11:46 GMT+02:00 Christoph Becker <cmbecker69@gmx.de>: > >> 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