Enable error_handler callback parameters to be passed by reference View as Markdown

I like the idea, we have the similar scenario, we need to print request log
id to the log,

so I just made a PR to implement this:
https://github.com/php/php-src/pull/1018 the PR is against master,
but I think this could also been in 5.5.

Hi Thomas,

In userland it is sometimes necessary to extend PHP's notices/warnings
with additional information (e.g. username from session, stack trace,
etc.)
I'm proposing to enable error_handler callback parameters to be passed by
reference to be able to append additional data to error messages.

Example:

<?php
ini_set('error_reporting', E_ALL);
ini_set('display_errors', 0);
ini_set('log_errors', 1);
ini_set('error_log', 'php_error.log');

function myErrorHandler($errno, &$errstr, $errfile, $errline) {
if (!empty($_SESSION['username'])) {
$errstr .= ', username: '.$_SESSION['username'];
}
return false; // continue normal error handler
}

set_error_handler("myErrorHandler");

function test() {
echo tests; // Use of undefined constant tests
}

test();

Gives in php_error.log:
[21-Jan-2015 04:32:26 UTC] PHP Notice: Use of undefined constant tests -
assumed 'tests', username: ... in error.php on line 17

Instead of:
[21-Jan-2015 04:32:26 UTC] PHP Notice: Use of undefined constant tests -
assumed 'tests' in error.php on line 17

This would be very useful for accounting!
If you cannot send pull requests, please file this to bugs.php.net as
request.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Yeah, seem other want it it too, so I just updated the PR to allow the
first four parameters been passed by reference.

On 23 January 2015 at 16:23, Nicolas Grekas nicolas.grekas+php@gmail.com
wrote:

function myErrorHandler($errno, &$errstr, $errfile, $errline) {...}

It would be awesome to also pass $errfile and $errline by reference. This
would allow mapping "compiled" source to real source code and have
meaningful error file+line information.
By "compiled", I mean e.g. inlined classes (like the bootstrap.cache.php
file in Symfony), or preprocessed sources, etc.

Nicolas

Hi Dan,

In userland it is sometimes necessary to extend PHP's notices/warnings
with additional information (e.g. username from session, stack trace, etc.)

Why can't you just use in the error handler function to write the
exact message you want?

http://php.net/manual/en/function.error-log.php

All logs may be written into different log, of course.
However, not all apps write "proper"/"required" logs for admins. Here
comes
customized save handler.

Besides poorly written apps, apps log should use completely separated log
system IMO.
i.e. Apps should not raise any errors under normal operations and leave
user error/exception
handler to users(admins).

The reason is to detect fatal code/security/subsystem failure as soon as
possible
and to take action for it. i.e. Since errors should not happen with normal
circumstances,
all uncatched error/exception is fatal that requires immediate action.

It's not mandatory, but it's best practice IMHO.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Hi Dan,

Why can't you just use in the error handler function to write the
exact message you want?

http://php.net/manual/en/function.error-log.php

Forgot to mention important part.
It's perfectly valid to use error_log. I just think leaving PHP error log
for fatal
errors is a best practice, in general.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

error_log() is a good point. In the future, set_error_handler() might be changed to be called multiple times with different custom error handlers, similar to how register_shutdown_function() and spl_autoload_register() act on multiple calls.
Having a chain of error handlers appending data to $errstr makes it difficult to use error_log(), because this is a one-time operation.
Also, error_log() has the ability to override the "error_log" property from php.ini, which might not be the desired behaviour.
So I would prefer $errstr to be passed by reference.

For completeness, error_log() currently has no parameters for $errno, $line and $file, so an example would look like this:

function myErrorHandler($errno, $errstr, $errfile, $errline) {
switch($errno){
case E_WARNING: $errnoStr='Warning'; break;
case E_NOTICE: $errnoStr='Notice'; break;
case E_STRICT: $errnoStr='Strict'; break;
case E_RECOVERABLE_ERROR: $errnoStr='Recoverable Error'; break;
case E_DEPRECATED: $errnoStr='Deprecated'; break;
case E_USER_ERROR: $errnoStr='User Error'; break;
case E_USER_WARNING: $errnoStr='User Warning'; break;
case E_USER_NOTICE: $errnoStr='User Notice'; break;
case E_USER_DEPRECATED: $errnoStr='User Deprecated'; break;
}
if (!empty($_SESSION['username'])) {
$errstr .= ', username: '.$_SESSION['username'];
}
error_log('PHP '.$errnoStr.': '.$errstr.' in '.$errfile.' on line '.$errline);
return true;
}

Regards
Thomas

Dan Ackroyd wrote on 30.01.2015 10:38:

In userland it is sometimes necessary to extend PHP's notices/warnings with
additional information (e.g. username from session, stack trace, etc.)

Why can't you just use in the error handler function to write the
exact message you want?

http://php.net/manual/en/function.error-log.php

cheers
Dan