php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #61747 User-defined error handler run despite at sign (@) error control operator
Submitted: 2012-04-16 17:57 UTC Modified: 2012-04-17 02:06 UTC
From: chealer at gmail dot com Assigned:
Status: Not a bug Package: *General Issues
PHP Version: 5.4.0 OS:
Private report: No CVE-ID: None
 [2012-04-16 17:57 UTC] chealer at gmail dot com
Description:
------------
The at sign operator allows to "ignore" error messages, as explained in http://ca3.php.net/manual/en/language.operators.errorcontrol.php

When prepended to an expression in PHP, any error messages that might be generated by that expression will be ignored. 

However, as reported in #61091, user-defined error handlers registered with set_error_handler() are nevertheless run when @ is used, which often causes such messages to show, as in the below example, where a custom error handler is used to customize the display of error messages.

As http://ca3.php.net/manual/en/language.operators.errorcontrol.php#98895 and http://ca3.php.net/manual/en/language.operators.errorcontrol.php#85042 show, this problem is not new. This behavior appears to be by design, and might be wanted in some cases.

Therefore, please either:
Stop calling user-defined error handlers when suppressing errors. This needs serious consideration for backwards-compatibility.
Allow specifying whether user-defined error handlers should be called when suppressing errors.
Make the documentation reflect the current state of things.

Alternatively, if the documentation of the @ operator isn't amended because custom error handlers are considered a corner case, then the set_error_handler() documentation should warn developers tempted to use custom error handlers that they are non-standard, not recommended/supported, and try to explain the pitfalls. In short, tell developers they should only use a custom error handler if they know what they're doing.

However, this alternative should be avoided since set_error_handler() is already used in several applications whose developers didn't receive the warning.

Test script:
---------------
<?php

function myErrorHandler($errno, $errstr) {
    switch ($errno) {
    case E_USER_ERROR:
        echo "<b>My ERROR</b> [$errno] $errstr<br />\n";
        echo "  Fatal error on line $errline in file $errfile";
        echo ", PHP " . PHP_VERSION . " (" . PHP_OS . ")<br />\n";
        echo "Aborting...<br />\n";
        exit(1);
        break;

    case E_USER_WARNING:
        echo "<b>My WARNING</b> [$errno] $errstr<br />\n";
        break;

    case E_USER_NOTICE:
        echo "<b>My NOTICE</b> [$errno] $errstr<br />\n";
        break;

    default:
        echo "Unknown error type: [$errno] $errstr<br />\n";
        break;
    }
}

// function to test the error handling
function scale_by_log($vect, $scale)
{
    if (!is_numeric($scale) || $scale <= 0) {
        trigger_error("log(x) for x <= 0 is undefined, you used: scale = $scale", E_USER_ERROR);
    }

    if (!is_array($vect)) {
        trigger_error("Incorrect input vector, array of values expected", E_USER_WARNING);
        return null;
    }

    $temp = array();
    foreach($vect as $pos => $value) {
        if (!is_numeric($value)) {
            trigger_error("Value at position $pos is not a number, using 0 (zero)", E_USER_NOTICE);
            $value = 0;
        }
        $temp[$pos] = log($scale) * $value;
    }

    return $temp;
}

$a = array(2, 3, "foo", 5.5, 43.3, 21.11);


/* Value at position $pos is not a number, using 0 (zero) */
scale_by_log($a, M_PI);
@scale_by_log($a, M_PI);

set_error_handler("myErrorHandler");
@scale_by_log($a, M_PI);

?>


Expected result:
----------------
Notice: Value at position 2 is not a number, using 0 (zero) in /var/www/atoperator.php on line 42

Call Stack:
    0.0005     339192   1. {main}() /var/www/atoperator.php:0
    0.0005     339836   2. scale_by_log(array (0 => 2, 1 => 3, 2 => 'foo', 3 => 5.5, 4 => 43.3, 5 => 21.11), 3.1415926535898) /var/www/atoperator.php:55
    0.0006     340648   3. trigger_error('Value at position 2 is not a number, using 0 (zero)', 1024) /var/www/atoperator.php:42

Actual result:
--------------
Notice: Value at position 2 is not a number, using 0 (zero) in /var/www/atoperator.php on line 42

Call Stack:
    0.0005     339192   1. {main}() /var/www/atoperator.php:0
    0.0005     339836   2. scale_by_log(array (0 => 2, 1 => 3, 2 => 'foo', 3 => 5.5, 4 => 43.3, 5 => 21.11), 3.1415926535898) /var/www/atoperator.php:55
    0.0006     340648   3. trigger_error('Value at position 2 is not a number, using 0 (zero)', 1024) /var/www/atoperator.php:42

<b>My NOTICE</b> [1024] Value at position 2 is not a number, using 0 (zero)<br />

Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2012-04-16 20:39 UTC] rasmus@php.net
-Status: Open +Status: Not a bug
 [2012-04-16 20:39 UTC] rasmus@php.net
The documentation is quite clear I think. In the first link you provided it 
says:

  If you have set a custom error handler function with set_error_handler() 
  then it will still get called, but this custom error handler can 
  (and should) call error_reporting() which will return 0 when the call 
  that triggered the error was preceded by an @.

I see no reason to change anything here. The current approach gives you all the 
control you need. If you have a custom error handler you can decide whether you 
want to ignore silenced calls or not. Any change to this would also be a major 
BC break.
 [2012-04-16 20:59 UTC] chealer at gmail dot com
I'm sorry. This was fixed in http://svn.php.net/viewvc/phpdoc/en/trunk/language/operators.xml?r1=322134&r2=323370
We now have:

When prepended to an expression in PHP, any error messages that might be generated by that expression will be ignored.

If you have set a custom error handler function with set_error_handler() then it will still get called, but this custom error handler can (and should) call error_reporting() which will return 0 when the call that triggered the error was preceded by an @.


Note that this second sentence contradicts the first in some [edge] cases. At least, the second sentence should immediately follow the first, rather than having its own paragraph.
 [2012-04-16 21:23 UTC] chealer at gmail dot com
This is a duplicate of #52338.

Note that I partly disagree with the fix. Custom error handlers *can* check error_reporting(), as illustrated in the example from http://ca3.php.net/manual/en/function.set-error-handler.php

function myErrorHandler($errno, $errstr, $errfile, $errline)
{
    if (!(error_reporting() & $errno)) {
        // This error code is not included in error_reporting
        return;
    }
    [...]
}

However, it would be rather inefficient if custom error handlers *should* (had to) do that, in general. If that was the case, PHP should simply not call user-defined error handlers when @ was used.
I think user-defined error handlers *should* do something like that, but only in some cases.
 [2012-04-16 21:31 UTC] rasmus@php.net
There is nothing inefficient about calling error_reporting(). It is a trivially 
small and fast internal function. And like I said, changing anything here would 
be a major BC break. There is nothing wrong with the current implementation.
 [2012-04-16 21:50 UTC] chealer at gmail dot com
I meant it would be inefficient to ask PHP programmers to include

if (!(error_reporting() & $errno)) {
   // This error code is not included in error_reporting
   return;
}

in all custom error handlers they write, if the problem could be addressed once and for all by the PHP interpreter.

If that is your stance though, this requirement should be documented in http://ca3.php.net/manual/en/function.set-error-handler.php
And if you really think that custom error handlers should ignore suppressed errors, then not calling custom error handlers would be more of a bugfix (for those handlers that fail to do it) than a BC break. Certainly, that wouldn't be a "major BC break", although it may be disruptive enough to warrant waiting for a major release to do such a behavior change.

I'm not saying there's something *wrong* (in the sense of buggy) with the current implementation, as long as how it works is documented (which wasn't the case until recently), and the requirement to check for suppressed errors is documented in set_error_handler()'s documentation (which is still not the case).
 [2012-04-16 22:34 UTC] rasmus@php.net
I didn't say that I think custom error handlers should ignore suppressed errors. 
I said that the authors of the custom error handlers should decide what to do 
with them so they should called error_reporting() and take an appropriate 
action. I have seen systems that use the @ to classify something that generates 
an E_WARNING as non-critical for example, where an E_WARNING without the @ 
causes someone to get paged.

And it is documented on the set_error_handler() page. It says:

  error_reporting() settings will have no effect and your error handler 
  will be called regardless - however you are still able to read the 
  current value of error_reporting and act appropriately. Of particular
  note is that this value will be 0 if the statement that caused the
  error was prepended by the @ error-control operator.

The point of a custom error handler is to override the default PHP error 
handling behaviour. By default PHP ignores errors from calls preceded by @, but 
since you are writing your own you should have the power to override any and all 
such defaults. If you choose to treat @-preceded errors like any other error, 
that's fine.

This really isn't going to change.
 [2012-04-17 00:36 UTC] chealer at gmail dot com
You write:
If you choose to treat @-preceded errors like any other error, that's fine.

Yet http://ca3.php.net/manual/en/language.operators.errorcontrol.php says a custom error handler should call error_reporting().
So why should a custom error handler which chooses to treat @-preceded errors like any other error call error_reporting()?

You write:
By default PHP ignores errors from calls preceded by @, but since you are writing your own you should have the power to override any and all such defaults.

I'm not sure I would say that custom error handlers *should* have the power to override error suppression, but I certainly understand that it can be useful. In any case, offering that flexibility doesn't have to make it more complicated to write a simple custom handler. set_error_handler() could simply have an argument to control whether the callback is called even on normally suppressed errors.
 [2012-04-17 00:59 UTC] rasmus@php.net
Right, it says "should" not "must". If you choose to not treat @ differently, 
then you obviously don't need to call error_reporting() from your handler.
 [2012-04-17 01:40 UTC] chealer at gmail dot com
Right. So, the documentation is saying custom error handlers should treat suppressed errors differently. You are saying it is fine not to do that.
I think your version is right.
 [2012-04-17 02:00 UTC] rasmus@php.net
No, I think a custom error handler should make good use of the silence operator. 
There are all kinds of interesting things you can do with it. But yes, if you 
want to ignore it entirely, that is fine too.
 [2012-04-17 02:06 UTC] chealer at gmail dot com
No what? It's not fine not to treat suppressed errors differently?
 [2012-04-18 03:35 UTC] anon at anon dot anon
What a lot of fuss about nothing.
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Fri Feb 23 04:01:34 2024 UTC