Newsgroups: php.internals Path: news.php.net Xref: news.php.net php.internals:103526 Return-Path: Delivered-To: mailing list internals@lists.php.net Received: (qmail 75817 invoked from network); 28 Nov 2018 06:34:55 -0000 Received: from unknown (HELO 28.ip-149-56-142.net) (149.56.142.28) by pb1.pair.com with SMTP; 28 Nov 2018 06:34:55 -0000 Received: from [127.0.0.1] (localhost [127.0.0.1]) (Authenticated sender: thruska@cubiclesoft.com) with ESMTPSA id 04DC73E877 To: Nikita Popov Cc: PHP internals References: <097b5f2f-4222-192d-01cc-19b4073f03ea@cubiclesoft.com> Message-ID: Date: Tue, 27 Nov 2018 19:58:40 -0700 User-Agent: Mozilla/5.0 (X11; Linux i686; rv:11.0) Gecko/20120327 Thunderbird/11.0.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [PHP-DEV] Don't silence fatal errors From: thruska@cubiclesoft.com (Thomas Hruska) On 11/27/2018 8:26 AM, Nikita Popov wrote: > On Tue, Nov 27, 2018 at 2:20 PM Thomas Hruska > wrote: > >> On 11/26/2018 2:42 PM, Nikita Popov wrote: >>> 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. > > > 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/