# Throwing an Error for require expressions in PHP7.x - Thread: https://externals.io/message/93995 - Original archive: https://news-web.php.net/php.internals/93995 - Started by Alexander Lisachenko on 2016-06-15T10:27:56+00:00 - Messages: 19 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #93995 — Alexander Lisachenko — 2016-06-15T10:27:56+00:00 URL: https://externals.io/message/93995#93995 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? --- ## #93997 — Rowan Collins — 2016-06-15T10:39:02+00:00 URL: https://externals.io/message/93995#93997 · In reply to: #93995 > 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] --- ## #93999 — Christoph Becker — 2016-06-15T10:52:05+00:00 URL: https://externals.io/message/93995#93999 · In reply to: #93995 > 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] -- Christoph M. Becker --- ## #94017 — Fleshgrinder — 2016-06-15T17:38:57+00:00 URL: https://externals.io/message/93995#94017 · In reply to: #93995 > 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 --- ## #94020 — Niklas Keller — 2016-06-15T17:52:57+00:00 URL: https://externals.io/message/93995#94020 · In reply to: #94017 2016-06-15 19:38 GMT+02:00 Fleshgrinder : > > > […] > throw a > > […] > > This is exactly how require is meant to work, just use include as > Christoph said already. These are two distinct features for a reason. > > > > […] > to > > […] > > 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 --- ## #94022 — Fleshgrinder — 2016-06-15T17:57:47+00:00 URL: https://externals.io/message/93995#94022 · In reply to: #94020 > 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 --- ## #94023 — Niklas Keller — 2016-06-15T19:28:31+00:00 URL: https://externals.io/message/93995#94023 · In reply to: #94022 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: > > > […] > produce a > > […] > > `file_get_contents()` and friends emit an `E_WARNING` and not an `E_ERROR` or > worse. ;) > > -- > Richard "Fleshgrinder" Fussenegger --- ## #94044 — Alexander Lisachenko — 2016-06-16T12:48:55+00:00 URL: https://externals.io/message/93995#94044 · In reply to: #94023 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. --- ## #94067 — Alexander Lisachenko — 2016-06-17T08:29:06+00:00 URL: https://externals.io/message/93995#94067 · In reply to: #94044 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. --- ## #94071 — Christoph Becker — 2016-06-17T09:46:31+00:00 URL: https://externals.io/message/93995#94071 · In reply to: #94067 > 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 --- ## #94085 — Niklas Keller — 2016-06-17T15:44:22+00:00 URL: https://externals.io/message/93995#94085 · In reply to: #94071 2016-06-17 11:46 GMT+02:00 Christoph Becker : > > > > […] > possible > > […] > commands, > > […] > in > > […] > our > > […] > > I have re-opened this ticket. > > > […] > race-condition, > > […] > > 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? --- ## #94086 — Christoph Becker — 2016-06-17T16:47:22+00:00 URL: https://externals.io/message/93995#94086 · In reply to: #94085 > 2016-06-17 11:46 GMT+02:00 Christoph Becker : > > > […] > > 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 --- ## #94088 — Rowan Collins — 2016-06-17T17:58:54+00:00 URL: https://externals.io/message/93995#94088 · In reply to: #94086 > 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] --- ## #94089 — Alexander Deruwe — 2016-06-17T18:29:08+00:00 URL: https://externals.io/message/93995#94089 · In reply to: #94086 > > 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. --- ## #94095 — Niklas Keller — 2016-06-17T20:32:10+00:00 URL: https://externals.io/message/93995#94095 · In reply to: #94086 > > 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 --- ## #94101 — Davey Shafik — 2016-06-18T01:08:36+00:00 URL: https://externals.io/message/93995#94101 · In reply to: #94095 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. > > […] > > > 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. > > > > […] > > 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. > > > > […] > > Yes, I really prefer that. Not that code exactly, but being able to write: > > try { > require $config; > } catch (ParseError $e) { > > } > > > > […] --- ## #94105 — Christoph Becker — 2016-06-18T10:34:23+00:00 URL: https://externals.io/message/93995#94105 · In reply to: #94088 > > > […] > > 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. > > […] > > 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 --- ## #94106 — Niklas Keller — 2016-06-18T12:29:49+00:00 URL: https://externals.io/message/93995#94106 · In reply to: #94105 Christoph Becker schrieb am Sa., 18. Juni 2016, 12:34: > > > > […] > > Indeed, but that was a major version – this suggestion is about PHP 7.x. > > > […] > > 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 --- ## #94123 — Fleshgrinder — 2016-06-19T15:53:02+00:00 URL: https://externals.io/message/93995#94123 · In reply to: #94106 > 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