# Don't silence fatal errors - Thread: https://externals.io/message/103513 - Original archive: https://news-web.php.net/php.internals/103513 - Started by Nikita Popov on 2018-11-26T21:42:55+00:00 - Messages: 34 - Page 1 of 2: messages 1 to 25 (25 per page, add `?page=N` to the URL) - Next page: https://externals.io/message/103513.md?page=2 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · RFCs at https://externals.io/rfc.md · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #103513 — Nikita Popov — 2018-11-26T21:42:55+00:00 URL: https://externals.io/message/103513#103513 Hi internals, When the silencing operator @ is used, the intention is generally to silence expected warnings or notices. However, it currently also silences fatal errors. As fatal errors also abort request execution, the result will often be a hard to debug white screen of death. The most recent occurrence which motivated me to write this mail is https://bugs.php.net/bug.php?id=77205, but I've seen this play out multiple times already. I would like to propose to change the behavior of @ to only silence warnings, notices and other low-level diagnostics, but leave fatal errors intake. In particular, the following should not be silenced: * `E_ERROR` * `E_CORE_ERROR` * `E_COMPILE_ERROR` * `E_USER_ERROR` * `E_RECOVERABLE_ERROR` * `E_PARSE` This change would have two main implications for backwards compatibility: 1. Code that legitimately wants to silence fatal errors for whatever reason should now use `error_reporting()` (or `ini_set()`) to do so, instead of @. 2. Error handlers that want to only handle non-silenced errors may have to be adjusted. A common pattern I found in our own tests if checks for `error_reporting()` != 0 to detect silencing. This should be changed to `error_reporting()` & $err_no to detect whether the specific error type is silenced. A preliminary patch for this change is available at https://github.com/php/php-src/pull/3685. What do you think about this? Nikita --- ## #103516 — Thomas Hruska — 2018-11-27T13:20:07+00:00 URL: https://externals.io/message/103513#103516 · In reply to: #103513 > Hi internals, > > When the silencing operator @ is used, the intention is generally to > silence expected warnings or notices. However, it currently also silences > fatal errors. As fatal errors also abort request execution, the result will > often be a hard to debug white screen of death. > > The most recent occurrence which motivated me to write this mail is > https://bugs.php.net/bug.php?id=77205, but I've seen this play out multiple > times already. > > I would like to propose to change the behavior of @ to only silence > warnings, notices and other low-level diagnostics, but leave fatal errors > intake. In particular, the following should not be silenced: > > * `E_ERROR` > * `E_CORE_ERROR` > * `E_COMPILE_ERROR` > * `E_USER_ERROR` > * `E_RECOVERABLE_ERROR` > * `E_PARSE` > > This change would have two main implications for backwards compatibility: > > 1. Code that legitimately wants to silence fatal errors for whatever reason > should now use `error_reporting()` (or `ini_set()`) to do so, instead of @. > > 2. Error handlers that want to only handle non-silenced errors may have to > be adjusted. A common pattern I found in our own tests if checks for > `error_reporting()` != 0 to detect silencing. This should be changed to > `error_reporting()` & $err_no to detect whether the specific error type is > silenced. > > A preliminary patch for this change is available at > https://github.com/php/php-src/pull/3685. > > What do you think about this? > > Nikita Instead of a blank screen (or early termination if some output has been sent), maybe emit, "[GMT date/time] A fatal error occurred. Check the error logs." The only bug I see here is that fatal errors are being suppressed from reaching the log files. But they should still be suppressed from the browser/client if the INI settings are configured to send messages to the logs. WSODs should have been fixed a long time ago to emit a simple, generic message to check the logs (i.e. no more WSODs). The average WSOD is usually accompanied with a HTTP 500 response but having to look at network tools tab to see the 500 is an extra step. A surprising number of developers I encounter don't know what a HTTP 500 means nor what to do when they encounter one. Therefore, helpful but very generic directions would be useful and save a few moments of head-scratching. Also, forcing users to override the default error handler to restore previous (and almost correct) behavior is a bit obnoxious. -- Thomas Hruska CubicleSoft President I've got great, time saving software that you will find useful. http://cubiclesoft.com/ And once you find my software useful: http://cubiclesoft.com/donate/ --- ## #103517 — Nikita Popov — 2018-11-27T15:26:16+00:00 URL: https://externals.io/message/103513#103517 · In reply to: #103516 On Tue, Nov 27, 2018 at 2:20 PM Thomas Hruska wrote: > > > […] > will > > […] > multiple > > […] > reason > > […] > to > > […] > > Instead of a blank screen (or early termination if some output has been > sent), maybe emit, "[GMT date/time] A fatal error occurred. Check the > error logs." The only bug I see here is that fatal errors are being > suppressed from reaching the log files. But they should still be > suppressed from the browser/client if the INI settings are configured to > send messages to the logs. > > WSODs should have been fixed a long time ago to emit a simple, generic > message to check the logs (i.e. no more WSODs). The average WSOD is > usually accompanied with a HTTP 500 response but having to look at > network tools tab to see the 500 is an extra step. A surprising number > of developers I encounter don't know what a HTTP 500 means nor what to > do when they encounter one. Therefore, helpful but very generic > directions would be useful and save a few moments of head-scratching. I think you are mixing two orthogonal degrees of error configurability here, which are a) The error_reporting level, which determines which errors are reported in the first place, and which is what @ influences, and b) The display_error, error_log etc. directives, which control what happens when an error is reported. The proposed change does not impact b) in any way. If you have display_errors=Off and use error_log (as you should in production), you use @ and a fatal error occurs, then (with the proposed change) no error will be displayed, but it *will* be logged. If you have display_errors=On and don't use error_log (as is common in development), you use @ and a fatal error occurs, then (with the proposed change) the error will be directly displayed. Without the proposed change, in both cases, you would not get an error, either logged or displayed. Nikita --- ## #103526 — Thomas Hruska — 2018-11-28T02:58:40+00:00 URL: https://externals.io/message/103513#103526 · In reply to: #103517 > On Tue, Nov 27, 2018 at 2:20 PM Thomas Hruska > wrote: > > > […] > > > I think you are mixing two orthogonal degrees of error configurability > here, which are > > a) The error_reporting level, which determines which errors are reported in > the first place, and which is what @ influences, and > > b) The display_error, error_log etc. directives, which control what happens > when an error is reported. > > The proposed change does not impact b) in any way. If you have > display_errors=Off and use error_log (as you should in production), you use > @ and a fatal error occurs, then (with the proposed change) no error will > be displayed, but it *will* be logged. If you have display_errors=On and > don't use error_log (as is common in development), you use @ and a fatal > error occurs, then (with the proposed change) the error will be directly > displayed. Without the proposed change, in both cases, you would not get an > error, either logged or displayed. > > Nikita The way it was worded sounded like the changes *might* override the directives in b). Thanks for the clarification. Carry on. -- Thomas Hruska CubicleSoft President I've got great, time saving software that you will find useful. http://cubiclesoft.com/ And once you find my software useful: http://cubiclesoft.com/donate/ --- ## #103527 — Agustin Casiva — 2018-11-28T20:25:14+00:00 URL: https://externals.io/message/103513#103527 · In reply to: #103513 > Hi internals, > > When the silencing operator @ is used, the intention is generally to > silence expected warnings or notices. However, it currently also silences > fatal errors. As fatal errors also abort request execution, the result will > often be a hard to debug white screen of death. > > The most recent occurrence which motivated me to write this mail is > https://bugs.php.net/bug.php?id=77205, but I've seen this play out > multiple > times already. > > I would like to propose to change the behavior of @ to only silence > warnings, notices and other low-level diagnostics, but leave fatal errors > intake. In particular, the following should not be silenced: > > * `E_ERROR` > * `E_CORE_ERROR` > * `E_COMPILE_ERROR` > * `E_USER_ERROR` > * `E_RECOVERABLE_ERROR` > * `E_PARSE` > > This change would have two main implications for backwards compatibility: > > 1. Code that legitimately wants to silence fatal errors for whatever reason > should now use `error_reporting()` (or `ini_set()`) to do so, instead of @. > > 2. Error handlers that want to only handle non-silenced errors may have to > be adjusted. A common pattern I found in our own tests if checks for > `error_reporting()` != 0 to detect silencing. This should be changed to > `error_reporting()` & $err_no to detect whether the specific error type is > silenced. > > A preliminary patch for this change is available at > https://github.com/php/php-src/pull/3685. > > What do you think about this? > > Nikita > I think the need of the @ is to silence everything, and I think is used only in extreme cases where the developer can't handle properly the errors (or the dev is kinda lazy to do it, I have used it a couple of times of course :) ). The developer that uses @ knows his risks and knows that can hides important information for debugging, I don't think that showing fatal error will help the developers in the long term, yes it might help it when is debugging, but later when the code is in production and the @ is still in place I think the developers will not expect to see any error. I don't see the real value of this change knowing the price of lost of backward compatibility. My two cents. Best -- Casiva Agustin Mail/Msn/GTalk/Jabber: casivaagustin@gmail.com Skype: casivaagustin CEL : 054-0362-155280015 Site: http://www.casivaagustin.com.ar --- ## #103528 — Claude Pache — 2018-11-28T22:20:37+00:00 URL: https://externals.io/message/103513#103528 · In reply to: #103513 > Le 26 nov. 2018 à 22:42, Nikita Popov a écrit : > > Hi internals, > > When the silencing operator @ is used, the intention is generally to > silence expected warnings or notices. However, it currently also silences > fatal errors. As fatal errors also abort request execution, the result will > often be a hard to debug white screen of death. > > The most recent occurrence which motivated me to write this mail is > https://bugs.php.net/bug.php?id=77205, but I've seen this play out multiple > times already. > > I would like to propose to change the behavior of @ to only silence > warnings, notices and other low-level diagnostics, but leave fatal errors > intake. In particular, the following should not be silenced: > > * `E_ERROR` > * `E_CORE_ERROR` > * `E_COMPILE_ERROR` > * `E_USER_ERROR` > * `E_RECOVERABLE_ERROR` > * `E_PARSE` > > This change would have two main implications for backwards compatibility: > > 1. Code that legitimately wants to silence fatal errors for whatever reason > should now use `error_reporting()` (or `ini_set()`) to do so, instead of @. > > 2. Error handlers that want to only handle non-silenced errors may have to > be adjusted. A common pattern I found in our own tests if checks for > `error_reporting()` != 0 to detect silencing. This should be changed to > `error_reporting()` & $err_no to detect whether the specific error type is > silenced. > > A preliminary patch for this change is available at > https://github.com/php/php-src/pull/3685. > > What do you think about this? > > Nikita Although this can be viewed as an issue of the silencing operator, this can also be viewed as an issue of the default error handler, which should not blindly obey the `error_reporting()` directive (or the @ operator) in case of fatal error. Hopefully, custom error handlers are able to scream even when they are asked to shut up. My suggestion is rather to change the implementation of the default error handler, so that it refuses to ever sweep fatal error messages under the rug. Code that has legitimate reasons to silence fatal errors can always use a custom error handler that will obey it. (BTW, it could be handy to have the following built-in constant: const E_ANY_ERROR = `E_ERROR` | `E_CORE_ERROR` | `E_COMPILE_ERROR` | `E_USER_ERROR` | `E_RECOVERABLE_ERROR` | E_PARSE; and ditto for warnings and notices.) —Claude --- ## #103529 — Fwentish Aelondes — 2018-11-29T04:44:32+00:00 URL: https://externals.io/message/103513#103529 · In reply to: #103513 Breaking BC might be unnecessary if instead of changing the default behavior of @, you add an additional flag to error_reporting that enables the new behavior, something like E_UNSILENCE_FATAL. Then developers would only need to switch a flag in php.ini to get the old behavior back, instead of re-working existing code around the new behavior. > Hi internals, > > When the silencing operator @ is used, the intention is generally to > silence expected warnings or notices. However, it currently also silences > fatal errors. As fatal errors also abort request execution, the result will > often be a hard to debug white screen of death. > > The most recent occurrence which motivated me to write this mail is > https://bugs.php.net/bug.php?id=77205, but I've seen this play out multiple > times already. > > I would like to propose to change the behavior of @ to only silence > warnings, notices and other low-level diagnostics, but leave fatal errors > intake. In particular, the following should not be silenced: > > * `E_ERROR` > * `E_CORE_ERROR` > * `E_COMPILE_ERROR` > * `E_USER_ERROR` > * `E_RECOVERABLE_ERROR` > * `E_PARSE` > > This change would have two main implications for backwards compatibility: > > 1. Code that legitimately wants to silence fatal errors for whatever reason > should now use `error_reporting()` (or `ini_set()`) to do so, instead of @. > > 2. Error handlers that want to only handle non-silenced errors may have to > be adjusted. A common pattern I found in our own tests if checks for > `error_reporting()` != 0 to detect silencing. This should be changed to > `error_reporting()` & $err_no to detect whether the specific error type is > silenced. > > A preliminary patch for this change is available at > https://github.com/php/php-src/pull/3685. > > What do you think about this? > > Nikita --- ## #103531 — michal@brzuchalski.com — 2018-11-29T09:59:22+00:00 URL: https://externals.io/message/103513#103531 · In reply to: #103529 I don't really know if it fits here but some weeks ago I was thinking about annotations with "@" prefix and was considering to propose to handle @ (silence operator) similar way as annotations, like: $value = @ fopen('test.txt','rb+'); might be equivalent to: $value = @SupressError(E_ALL) fopen('test.txt','rb+'); BTW This way I believe it would be easier to parse annotations with '@' prefix in all desired places with one branch inside parser. As well as there will be a place to put supression error level per function/method call with more specific requirements, like: $value = @SupressError(E_ALL ^ E_ERROR) fopen('test.txt','rb+'); Which might work as supress all errors except fatal errors. Does that sound like a solution at all? The developer then has full controll on what errors are supressed or not. Sorry to bother you if it's insane and crazy idea. czw., 29 lis 2018 o 05:44 Fwentish Aelondes napisał(a): > Breaking BC might be unnecessary if instead of changing the default > behavior of @, you add an additional flag to error_reporting that > enables the new behavior, something like E_UNSILENCE_FATAL. Then > developers would only need to switch a flag in php.ini to get the old > behavior back, instead of re-working existing code around the new > behavior. > > > > […] > will > > […] > multiple > > […] > reason > > […] > to > > […] > > -- > > > > -- regards / pozdrawiam, -- Michał Brzuchalski about.me/brzuchal brzuchalski.com --- ## #103532 — Dan Ackroyd — 2018-11-29T11:24:29+00:00 URL: https://externals.io/message/103513#103532 · In reply to: #103531 > > I don't really know if it fits here but some weeks ago I was thinking about > annotations with "@" prefix >.... > > Which might work as supress all errors except fatal errors. > > Does that sound like a solution at all? > The developer then has full controll on what errors are suppressed or not. I was thinking along similar lines a while ago: https://gist.github.com/Danack/5ae0b1b1ce30a0d785dd The reason I never formally suggested it as an RFC is that I think it's doubling down on the wrong solution. The vast majority of places where errors/warning are used currently, could either be just removed or have the result of the function be changed to be a tuple of the current result, and an error flag/message. [$result, $error] = foo($bar); if ($error !== null) { // something went wrong. } // $result is usable cheers Dan --- ## #103533 — Christoph M. Becker — 2018-11-29T11:49:26+00:00 URL: https://externals.io/message/103513#103533 · In reply to: #103532 > > > […] > > I was thinking along similar lines a while ago: > https://gist.github.com/Danack/5ae0b1b1ce30a0d785dd > > The reason I never formally suggested it as an RFC is that I think > it's doubling down on the wrong solution. > > The vast majority of places where errors/warning are used currently, > could either be just removed or have the result of the function be > changed to be a tuple of the current result, and an error > flag/message. > > [$result, $error] = foo($bar); > if ($error !== null) { > // something went wrong. > } > // $result is usable If a function issues a warning and returns some value indicating failure, we should consider to let the function throw an exception instead. Typical cases would be `getimagesize()`, `fopen()` and `password_hash()`, for instance. -- Christoph M. Becker --- ## #103541 — Andrea Faulds — 2018-12-02T22:51:00+00:00 URL: https://externals.io/message/103513#103541 · In reply to: #103513 Hi Nikita, Nikita Popov wrote: > When the silencing operator @ is used, the intention is generally to > silence expected warnings or notices. However, it currently also silences > fatal errors. As fatal errors also abort request execution, the result will > often be a hard to debug white screen of death. > > The most recent occurrence which motivated me to write this mail is > https://bugs.php.net/bug.php?id=77205, but I've seen this play out multiple > times already. > > I would like to propose to change the behavior of @ to only silence > warnings, notices and other low-level diagnostics, but leave fatal errors > intake. It's always been bizarre to me that @ can silence fatal errors, which has no practical application and makes using @ to silence a lower-level error potentially hszardous if its targeted function can also produce a fatal error. Obviously, I'd be in favour of fixing this. :) -- Andrea Faulds https://ajf.me/ --- ## #103542 — Stanislav Malyshev — 2018-12-02T23:19:39+00:00 URL: https://externals.io/message/103513#103542 Hi! > It's always been bizarre to me that @ can silence fatal errors, which > has no practical application and makes using @ to silence a lower-level > error potentially hszardous if its targeted function can also produce a > fatal error. Yes, silencing something that kills the script is probably not a very great idea. It can have some BC effects if this is a part of larger environments where outgoing messages are tracked, but I think it's a worthy price to pay for making it more sensible. This won't fix WSOD problem by itself, but probably will make WSODs a little less annoying to debug. Which is a win in my book. -- Stas Malyshev smalyshev@gmail.com --- ## #103546 — Zeev Suraski — 2018-12-03T12:08:20+00:00 URL: https://externals.io/message/103513#103546 > Hi Nikita, > > Nikita Popov wrote: > > […] > will > > […] > multiple > > […] > It's always been bizarre to me that @ can silence fatal errors, which > has no practical application and makes using @ to silence a lower-level > error potentially hszardous if its targeted function can also produce a > fatal error. > Well, it does have one practical application which I think we should pay attention to if we are to change the behavior - in case of display_errors being on (yes, nobody should have that in production, but that doesn't change the fact that many do) - suddenly we would potentially be exposing sensitive data to remote users - data that was previously explicitly silenced and with this change we'd be breaking that contract. This is not an unlikely scenario. OTOH I do see the value of being able to silence only non-critical messages. What if we do something along the lines of what somebody proposed, and introduce another error level E_ERROR_WHILE_SILENCED or something like that? Folks who would want to will be able to turn it on, and we can leave it out of `E_ALL` so that it would require explicit enabling. Zeev --- ## #104188 — Nikita Popov — 2019-02-05T15:16:44+00:00 URL: https://externals.io/message/103513#104188 · In reply to: #103513 > Hi internals, > > When the silencing operator @ is used, the intention is generally to > silence expected warnings or notices. However, it currently also silences > fatal errors. As fatal errors also abort request execution, the result will > often be a hard to debug white screen of death. > > The most recent occurrence which motivated me to write this mail is > https://bugs.php.net/bug.php?id=77205, but I've seen this play out > multiple times already. > > I would like to propose to change the behavior of @ to only silence > warnings, notices and other low-level diagnostics, but leave fatal errors > intake. In particular, the following should not be silenced: > > * `E_ERROR` > * `E_CORE_ERROR` > * `E_COMPILE_ERROR` > * `E_USER_ERROR` > * `E_RECOVERABLE_ERROR` > * `E_PARSE` > > This change would have two main implications for backwards compatibility: > > 1. Code that legitimately wants to silence fatal errors for whatever > reason should now use `error_reporting()` (or `ini_set()`) to do so, instead of > @. > > 2. Error handlers that want to only handle non-silenced errors may have to > be adjusted. A common pattern I found in our own tests if checks for > `error_reporting()` != 0 to detect silencing. This should be changed to > `error_reporting()` & $err_no to detect whether the specific error type is > silenced. > > A preliminary patch for this change is available at > https://github.com/php/php-src/pull/3685. > > What do you think about this? > > Nikita > I'd like to move forward with this change. I think the overall reception here has been positive, although in the discussion some other possibilities that avoid/reduce the BC aspect have been discussed. I think now that we have a PHP 8 branch, it would make sense to apply this as-is. The BC break is quite minor (compared to the other changes in PHP 8) and I think this is the cleanest way to solve the problem, as it only changes the list of silences errors, without introducing any new error handling concepts or mechanisms. Nikita --- ## #104197 — Zeev Suraski — 2019-02-05T16:55:38+00:00 URL: https://externals.io/message/103513#104197 · In reply to: #104188 > On Mon, Nov 26, 2018 at 10:42 PM Nikita Popov > wrote: > > > I'd like to move forward with this change. I think the overall reception > here has been positive, although in the discussion some other possibilities > that avoid/reduce the BC aspect have been discussed. I think now that we > have a PHP 8 branch, it would make sense to apply this as-is. The BC break > is quite minor (compared to the other changes in PHP 8) and I think this is > the cleanest way to solve the problem, as it only changes the list of > silences errors, without introducing any new error handling concepts or > mechanisms. I don't think that other changes that may or may not make it into PHP 8 should influence our decision - BC breaks accumulate and the more you have of them, the more difficult it is to migrate. I'm also present unaware of anything we already decided to 'break' in PHP 8 as of now (with the exception of the removal of deprecated 7.x features). That said - I think we all agree the BC breakage scope is small - but at the same time, it may be quite fatal for those that are affected. Those who have display_errors on (which is both horrible for production and at the same time fairly popular) are risking exposing sensitive data that beforehand, they were safely and explicitly hiding using @. Is there any reason *not* to do it in such a way that provides a gentler migration path? Introduce a new error level that would be a part of `E_ALL` in 7.4, but outside of `E_ALL` in 8.0 - that would govern whether fatal errors are silenced or not. Zeev --- ## #104199 — Nikita Popov — 2019-02-05T17:18:39+00:00 URL: https://externals.io/message/103513#104199 · In reply to: #104197 > > > > […] > > > I don't think that other changes that may or may not make it into PHP 8 > should influence our decision - BC breaks accumulate and the more you have > of them, the more difficult it is to migrate. I'm also present unaware of > anything we already decided to 'break' in PHP 8 as of now (with the > exception of the removal of deprecated 7.x features). > > That said - I think we all agree the BC breakage scope is small - but at > the same time, it may be quite fatal for those that are affected. Those > who have display_errors on (which is both horrible for production and at > the same time fairly popular) are risking exposing sensitive data that > beforehand, they were safely and explicitly hiding using @. > I can see the general concern here, but I'm having a hard time imagining that this will be an issue in practice. If you have display_errors=on you are at risk of leaking information in error messages with or without this change. The prime example of leaking information, which is passwords contained in the exception stack trace of a failed PDO connection, isn't even affected by this, because the silencing will be removed as part of exception unwinding anyway. > Is there any reason *not* to do it in such a way that provides a gentler > migration path? Introduce a new error level that would be a part of `E_ALL` > in 7.4, but outside of `E_ALL` in 8.0 - that would govern whether fatal > errors are silenced or not. > The reason not to do it is pretty much the same as always: Adding that new error level is basically the same as adding a new ini setting (and if we do want that, then I think it should be it's own ini setting and not hacked in as part of error_reporting), and you know our usual opinion on that topic. Furthermore, in this particular case it would also defeat the purpose of the change. If we set the option such that fatals are silenced by default, then we may as well not make the change, because the people who benefit most from it (non-expert users) are not going to change that default. Conversely, if we do not silence fatals by default, then we don't solve the issue with display_errors=on you mentioned above (as it requires explicitly setting an option, in which case they could just set display_errors=off instead.) Nikita --- ## #104219 — Zeev Suraski — 2019-02-06T04:42:41+00:00 URL: https://externals.io/message/103513#104219 · In reply to: #104199 > > > > […] > > I can see the general concern here, but I'm having a hard time imagining > that this will be an issue in practice. If you have display_errors=on you > are at risk of leaking information in error messages with or without this > change. The prime example of leaking information, which is passwords > contained in the exception stack trace of a failed PDO connection, isn't > even affected by this, because the silencing will be removed as part of > exception unwinding anyway. > > Well, there are all sorts of information leaks that can happen as a result of this, including filesystem layout and even the fact that the server is running PHP in the first place. Is there any reason *not* to do it in such a way that provides a gentler > > […] > > The reason not to do it is pretty much the same as always: Adding that new > error level is basically the same as adding a new ini setting (and if we do > want that, then I think it should be it's own ini setting and not hacked in > as part of error_reporting), and you know our usual opinion on that topic. > Furthermore, in this particular case it would also defeat the purpose of > the change. If we set the option such that fatals are silenced by default, > then we may as well not make the change, because the people who benefit > most from it (non-expert users) are not going to change that default. > Conversely, if we do not silence fatals by default, then we don't solve the > issue with display_errors=on you mentioned above (as it requires explicitly > setting an option, in which case they could just set display_errors=off > instead.) > While I don't really agree that adding a new error level is equivalent to adding a new INI entry, and I would go for having it as a part of the error levels and not as a separate INI entry if we were to add this functionality - I think you're fundamentally right that this approach doesn't truly bring value in making the migration smoother. I can't really think of an elegant way to handle this given the unique situation where this whole change is in the context of error suppression - which means deprecation notices aren't helpful. As long as we have a prominent warning about this in our migration guide alerting people to the associated risk in setups where display errors is on - I can live with the change as-is. How do we ensure that it doesn't get lost in the clutter given that it has no RFC? Zeev --- ## #104261 — Christoph M. Becker — 2019-02-06T21:45:22+00:00 URL: https://externals.io/message/103513#104261 > As long as we have a prominent warning about this in our migration guide > alerting people to the associated risk in setups where display errors is on > - I can live with the change as-is. How do we ensure that it doesn't get > lost in the clutter given that it has no RFC? Generally, *everything* that needs to be mentionend in the migration guide, should have a respective entry in UPGRADING. That makes creating the migration guide straight forward for the doc team, and worked quite well for PHP 7.3 at least. -- Christoph M. Becker --- ## #104265 — Pierre Joye — 2019-02-07T01:03:39+00:00 URL: https://externals.io/message/103513#104265 Good morning Nikita, > Hi internals, > > When the silencing operator @ is used, the intention is generally to > silence expected warnings or notices. However, it currently also silences > fatal errors. As fatal errors also abort request execution, the result will > often be a hard to debug white screen of death. > > The most recent occurrence which motivated me to write this mail is > https://bugs.php.net/bug.php?id=77205, but I've seen this play out > multiple > times already. > > I would like to propose to change the behavior of @ to only silence > warnings, notices and other low-level diagnostics, but leave fatal errors > intake. In particular, the following should not be silenced: > I am surely missing use cases because I wonder why we need @, at all? Yes there are functions generating extra messages and should not, be fro. PHP implementation or from external libraries (wrapping stderr to php errors). All of them could be fixed. Best, Pierre --- ## #104266 — Girgias — 2019-02-07T01:07:37+00:00 URL: https://externals.io/message/103513#104266 > Good morning Nikita, > > > > > […] > will > > […] > > > I am surely missing use cases because I wonder why we need @, at all? > > Yes there are functions generating extra messages and should not, be fro. > PHP implementation or from external libraries (wrapping stderr to php > errors). All of them could be fixed. > > Best, > Pierre > The most common case which comes to mind is to suppress erros while file reading. Because even if you check a file exists it could be deleted inbetween the check and the read command. As you can see this is even written in the documentation [1] Best regards George Peter Banyard [1] https://secure.php.net/manual/en/function.fopen.php --- ## #104267 — Yasuo Ohgaki — 2019-02-07T01:25:36+00:00 URL: https://externals.io/message/103513#104267 > > > > […] > silences > > […] > errors > > […] > > The most common case which comes to mind is to suppress erros while file > reading. > Because even if you check a file exists it could be deleted inbetween the > check and the > read command. As you can see this is even written in the documentation [1] > > Best regards > > George Peter Banyard > > [1] https://secure.php.net/manual/en/function.fopen.php If a developer decides to write sloppy code, then @ may be acceptable. I wouldn't use @, though. IMO, there should be INI switch that kills @ operator. There is extension module for this purpose, but PHP itself should have it. @ operator is convenient, but @ is evil and make debugging a lot harder. Regards, -- Yasuo Ohgaki yohgaki@ohgaki.net --- ## #104268 — Pierre Joye — 2019-02-07T01:32:00+00:00 URL: https://externals.io/message/103513#104268 > > The most common case which comes to mind is to suppress erros while file > reading. > Because even if you check a file exists it could be deleted inbetween the > check and the > read command. As you can see this is even written in the documentation [1] > > Best regards > > George Peter Banyard > > [1] https://secure.php.net/manual/en/function.fopen.php > That is one the cases I meant. Also this is a bug in the user code. I can imagine doing it on purpose for performance reasons (if the file is created/deleted by other apps) tho'. flock is what should be used here. best, Pierre --- ## #104271 — Pierre Joye — 2019-02-07T01:37:43+00:00 URL: https://externals.io/message/103513#104271 On Thu, Feb 7, 2019 at 3:14 PM Christian Schneider wrote: > > Am 07.02.2019 um 02:32 schrieb Pierre Joye : > > […] > > Sorry if I'm missing something but would `flock()` help with `file_get_contents()` and an external program deleting a file? > > We have the following fields where we sometimes use @ to suppress error_log entries so they do not hide real problems not handled by our code already: > - filesystem operations like `file_get_contents()` or `mkdir()` which can have (benevolent) races > - `json_decode()` of external data (we check the validity of the data afterwards anyway) > - DB connections which we log more detailed separately where the generic error is not interesting > > In general I agree with the notion of using @ as little as possible but I think it will has use cases where using `error_reporting()` instead would decrease code readability. > > Please do not remove @ ;-) My thought that @ mainly relates to another RFC where errors/warning are very inconsistently reported or designed (like forcing one to use @). 8 would be a good candidate to clean that up, like the TypeError RFC. best, Pierre --- ## #104270 — Christian Schneider — 2019-02-07T08:14:58+00:00 URL: https://externals.io/message/103513#104270 Am 07.02.2019 um 02:32 schrieb Pierre Joye : > > > […] > > That is one the cases I meant. Also this is a bug in the user code. I can > imagine doing it on purpose for performance reasons (if the file is created/deleted by other apps) tho'. > flock is what should be used here. Sorry if I'm missing something but would `flock()` help with `file_get_contents()` and an external program deleting a file? We have the following fields where we sometimes use @ to suppress error_log entries so they do not hide real problems not handled by our code already: - filesystem operations like `file_get_contents()` or `mkdir()` which can have (benevolent) races - `json_decode()` of external data (we check the validity of the data afterwards anyway) - DB connections which we log more detailed separately where the generic error is not interesting In general I agree with the notion of using @ as little as possible but I think it will has use cases where using `error_reporting()` instead would decrease code readability. Please do not remove @ ;-) - Chris --- ## #104274 — Benjamin Morel — 2019-02-07T09:01:01+00:00 URL: https://externals.io/message/103513#104274 > > That is one the cases I meant. Also this is a bug in the user code. I can > imagine doing it on purpose for performance reasons (if the file is > created/deleted by other apps) tho'. > flock is what should be used here. flock doesn't protect from other failures, such as a `fread()`ing a file located on NFS. As it stands now, @ may still useful to mute warnings and throw exceptions instead. The other solution today is to set up a temporary error handler before calling the native function, and restore the previous error handler right after, such as here . This allows to get the error message, but might incur a small performance penalty. IMO, @ can be safely removed the day PHP converts current warnings to exceptions. > > > > […] > [1] > > […] > > That is one the cases I meant. Also this is a bug in the user code. I can > imagine doing it on purpose for performance reasons (if the file is > created/deleted by other apps) tho'. > flock is what should be used here. > > best, > Pierre --- Next page: https://externals.io/message/103513.md?page=2