[RFC] Enable error_handler callback parameters to be passed by reference

Hi all,

if you want custom logs, maybe you should use custom logger.

It requires to catch all errors, I suppose. If PHP allows it, either would
work.

I considered a little more. With error handler that catches E_ERROR cannot
be
complex handler as complex handler may use various resources that may
cause
all kinds of errors. Therefore, complex custom error handler logger may
miss to
log important errors.

Best practice would be separating app log and system(PHP) error log.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

Hi,

the idea behind the rfc is not to change error messages, it's only to append useful information to them.
Changing lines/filenames is surely something controversial and might be only useful when compiling configuration files or templates to php code. Therefore we have 3 options in the voting section.

Maybe better access to logging system for custom logging would be more
efficient instead?

The logging system can be accessed with 1 line of code (trigger_error, throw new exception, etc), so from userland perspective I don't see any deficits here.
In case someone needs multiple error handlers, it might be necessary to implement sth. like spl_set_error_handler (similar to spl_autoload_register), but that's not in the scope of this rfc.

Regards
Thomas

Stanislav Malyshev wrote on 27.01.2015 20:43:

Hi!

I feel it's kindof wrong direction.

if you want custom logs, maybe you should use custom logger.

Same here, API basing on modifying something that is supposed to be
internal engine things feels wrong, even if it does work. Changing error
message is borderline, changing things like lines/filenames just feels
wrong.

Maybe better access to logging system for custom logging would be more
efficient instead?

--
Stas Malyshev
smalyshev@gmail.com

There are some workarounds with register_shutdown_function to extend E_ERROR messages, but I think that's quite dirty in a system with many parallel requests.
Here is an 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');
ini_set('date.timezone', 'Europe/Berlin');
ini_set('memory_limit', '16M');

$_SERVER['HTTP_HOST'] = 'mywebsite.com';
$_SERVER['REQUEST_URI'] = '/custom/foobar';
$_REQUEST = array('action' => 'edit');

function check_for_fatal() {
$error = error_get_last();
if ($error['type'] == E_ERROR) {
$errstr = '';
if (!empty($_SERVER['REQUEST_URI'])) {
$errstr .= ' Request-Uri: '.$_SERVER['REQUEST_URI'].PHP_EOL;
}
$errstr .= ' Request-Params: '.json_encode($_REQUEST);
file_put_contents('php_error.log', $errstr. PHP_EOL, FILE_APPEND);
}
}

register_shutdown_function('check_for_fatal');

$str = str_repeat('##########', 210241024);

gives:

[28-Jan-2015 07:00:53 Europe/Berlin] PHP Fatal error: Allowed memory size of 16777216 bytes exhausted (tried to allocate 20971521 bytes) in test.php on line 27
Request-Uri: /custom/foobar
Request-Params: {"action":"edit"}
actually, this should already be in access.log(the last 500 error
one)...... I don't see why you need it in error log....

thanks

Regards
Thomas

Xinchen Hui wrote on 27.01.2015 13:26:

Hey:

Hi Thomas,

Here is the rfc:

https://wiki.php.net/rfc/error_handler_callback_parameters_passed_by_reference

Thanks to reeze for coding and putting it on the wiki.

It looks good to me.

Future Scope
set_error_handler() callback might be able to handle E_ERROR to be able to
append additional information to memory_limit exhaustions (or others).

I really want to catch E_RROR by user defined error handler. The only
reason why this was disabled is abuse of error handler. We may document
abuse of error handler with E_ERROR may result in undefined behavior
including memory leak/crash and allow it.
I feel it's kindof wrong direction.

if you want custom logs, maybe you should use custom logger.

thanks

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net

--
Xinchen Hui
@Laruence
http://www.laruence.com/

--
Xinchen Hui
@Laruence
http://www.laruence.com/

Error handlers are global anyway, so this is useless (or dangerous) for
libraries.

Yes that's true, maybe set_error_handler can be changed so it behaves like register_shutdown_function or spl_autoload_register when multiple callbacks are registered?

Regards
Thomas

Andrea Faulds wrote on 28.01.2015 18:38:

Hi Thomas,

the idea behind the rfc is not to change error messages, it's only to append
useful information to them.

That’s still changing the message.

Error handlers are global anyway, so this is useless (or dangerous) for
libraries.

I don’t get it.

--
Andrea Faulds
http://ajf.me/

Hi Andrea,

Error handlers are global anyway, so this is useless (or dangerous) for
libraries.

I suggest library/framework developers not to use user defined error
handler.
It's better to leave error handler for application developers.

I strongly suggest application developers to use user defined error handler
to catch
unexpected problems.

If app codes are written not to raise errors under normal operations (which
I strongly
recommend to do so), errors are sign of system problems. e.g. Subsystem is
down,
misconfiguration, crackers are attacking system, etc.

In case of attack, admins would like to know which account is causing the
problem. Error message like "Warning: failed to write /etc/passwd" is less
useful than "Warning: failed to write /etc/passwd. User: yohgaki".

User defined error handler is for error handling customization, letting
user
customize message makes sense.

Thomas,
If there are many people concerns to change error message, how about
append user error message to original error message when it's changed?
It's a hacky behavior to me, though.

Regards,

--
Yasuo Ohgaki
yohgaki@ohgaki.net