[Feature Request]: Support for "wp_php_error_message"-hook

[Chrico]

Is your feature request related to a problem?

Right nowWonolog is listening to several PHP build in functions to catch "fatal errors". But WordPress has since version 5.2.0 the WP_Fatal_Error_Handler [1] which listens to register_shutdown_function() [2] as well.

The standard callback will just print via wp_die() [3] a generic template like There has been a critical error on this website. and no error message will be logged to our logger anymore.

[1] https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wp-fatal-error-handler.php#L29 [2] https://github.com/WordPress/WordPress/blob/master/wp-includes/error-protection.php#L95 [3] https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wp-fatal-error-handler.php#L239

Describe the desired solution

The WP_Fatal_Error_Handler provides a new hook called wp_php_error_message which has a $message and the $error which we can use to log PHP errors.

See: https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wp-fatal-error-handler.php#LL218C13-L218C13

Describe the alternatives that you have considered

-

Additional context

No response

Code of Conduct

• [X] I agree to follow this project's Code of Conduct

[gmazzap]

The fatal error handler can be disabled, and it is common to disable it in a non-production environment. And if we would listen to that hook, and fatal error handler is disabled, we won't see any log for PHP errors.

[Chrico]

@gmazzap Can't we listen to that WP_DISABLE_FATAL_ERROR_HANDLER constant and either use the hook or the register_shutdown_function() to ensure that no log entires are lost? ;)

[gmazzap]

What is the benefit of that instead of just using register_shutdown_function()?

Right now Wonolog is uses:

1. set_exception_handler
2. set_error_handler
3. register_shutdown_function

We would need the first two anyway. The reason is that in case there's some other code using them (e.g. the popular "Query Monitor" plugin) we would loose those info if we would just listen to the fatal hander hook.

We could replace the third, but only if WP_DISABLE_FATAL_ERROR_HANDLER is not defined or false.

So this code: https://github.com/inpsyde/Wonolog/blob/2.x/src/Configurator.php#L1024-L1026 would change from:

        if (PhpErrorController::typesMaskContainsFatals($errorTypes)) {
            register_shutdown_function([$controller, 'onShutdown']);
        }

to:

        if (!PhpErrorController::typesMaskContainsFatals($errorTypes)) {
           return;
        }

        if (!defined('WP_DISABLE_FATAL_ERROR_HANDLER') || !WP_DISABLE_FATAL_ERROR_HANDLER) {
           add_filter(
               'wp_php_error_message',
               function ($message, $error) use ($controller) {
                     $controller->onShutdown($message, $error);
                     return $message;
               },
               10,
               2
           );
           return;
        }

        register_shutdown_function([$controller, 'onShutdown']);

Plus, we would need to modify PhpErrorController::onShutdown() to optionally receive a message and the error data. And considering that is a plugin method, we would need to add some logic to verify the passed data.

Which means the current:

    public function onShutdown(): void
    {
        $lastError = error_get_last();
        if (!$lastError) {
            return;
        }

        $error = array_replace(
            ['type' => -1, 'message' => '', 'file' => '', 'line' => null],
            $lastError
        );

        if (in_array($error['type'], self::FATALS, true)) {
            $this->onError($error['type'], $error['message'], $error['file'], $error['line']);
        }
    }

becomes:

    public function onShutdown($message = null, $customError = null): void
    {
        $rawError = error_get_last();
        $lastError = is_array($customError) ? $customError : $rawError;
        if (!$lastError) {
            return;
        }

        $error = array_replace(
            ['type' => -1, 'message' => '', 'file' => '', 'line' => null],
            $lastError
        );

        ($message && is_string($message)) and $error['message'] = $message;

        if (in_array($error['type'], self::FATALS, true)) {
            $this->onError($error['type'], $error['message'], $error['file'], $error['line']);
        }
    }

This would allow logging a possibly-filtered message instead of the "raw" message, but I'm not sure that is a benefit at all, or a benefit that justifies augmenting the complexity of the two methods.

[Chrico]

We're discussing this currently internally and I'll write down a conclusion.

The big problem is, that the WP_Fatal_Error_Handler is created _before [1] mu-Pluigins/Plugins are booted. So eventho Wonolog uses register_shutdown_function(), the WP class will call wp_die() and not allow to execute the callback from Wonolog.

Additionally when is_admin() and headers_sent() is true the hook will also not be triggered and there is no fallback/hook called at all. So it will fail silently [2] without being able to log anything.

---

Edit: As discussed:

wp_die callbacks only die if $args['exit'] is true. So we could use wp_php_error_args-filter [3] to set $args['exit'] to false. That way, WP will not call die() and our functions registered on shutdown would execute.

So on activation Wonolog will always add:

add_filter( 'wp_php_error_args', function ($args) {
   is_array($args) or $args = [];
   $args['exit'] = false;
    return $args;
}, PHP_INT_MAX);

---

[1] https://github.com/WordPress/WordPress/blob/4e4016f61fa40abda4c0a0711496f2ba50a10563/wp-settings.php#L64

[2] https://github.com/WordPress/WordPress/blob/master/wp-includes/class-wp-fatal-error-handler.php#L57

[3] https://github.com/WordPress/WordPress/blob/4e4016f61fa40abda4c0a0711496f2ba50a10563/wp-includes/class-wp-fatal-error-handler.php#L229

[gmazzap]

I closed #70 because the final conclusion was to use $args['exit'] = false; on wp_php_error_args, but WP does it already , so there's nothing else to do.

@Chrico can we close thisas well?

[Chrico]

Jep, feel free to close 👍