Changeset 59107

Timestamp:

09/27/2024 07:26:15 PM (4 months ago)

Author:

hellofromTonya

Message:

Code Modernization: Fix trigger_error() with E_USER_ERROR deprecation in wp_trigger_error().

PHP 8.4 deprecates the use of trigger_errror() with E_USER_ERROR as the error level, as there are a number of gotchas to this way of creating a Fatal Error (finally blocks not executing, destructors not executing). The recommended replacements are either to use exceptions or to do a hard exit.

WP has its own wp_trigger_error() function, which under the hood calls trigger_error(). If passed E_USER_ERROR as the $error_level, this will hit the PHP 8.4 deprecation.

Now, there were basically three options:

• Silence the deprecation until PHP 9.0 and delay properly solving this until then. This would lead to an awkward solution, as prior to PHP 8.0, error silencing would apply to all errors, while, as of PHP 8.0, it will no longer apply to fatal errors. It also would only buy us some time and wouldn't actually solve anything.

• Use exit($status) when wp_trigger_error() is called with E_USER_ERROR. This would make the code untestable and would disable handling of these errors via custom error handlers, which makes this an undesirable solution.

• Throw an exception when wp_trigger_error() is called with E_USER_ERROR. This makes for the most elegant solution with the least BC-breaking impact, though it does open it up to the error potential being "caught" via a try-catch. That's not actually a bad thing and is likely to only happen for those errors which can be worked around, in which case, it's a bonus that that's now possible.

The third option is implemented which:

• Introduces a new WP_Exception class.
• Starts using WP_Exception in the wp_trigger_error() function when the $error_level is set to E_USER_ERROR.

This change is covered by pre-existing tests, which have been updated to expect the exception instead of a PHP error.

Why not use WP_Error?

Well, for one, this would lead to completely different behaviour (BC).

As WP_Error doesn't extend Exception, the program would not be stopped, but would continue running, which would be a much bigger breaking change and carries security risks. WP_Error also doesn't natively trigger displaying/logging of the error message, so in that case, it would still need an exit with the error message, bringing us back to point 2 above.

Introducing WP_Exception provides (essentially) the same behaviour in that it retains the fatal error and error message displaying/logging behaviors. It also introduces a base Exception class, from which future exception classes can extend.

References:

• ​https://wiki.php.net/rfc/deprecations_php_8_4#deprecate_passing_e_user_error_to_trigger_error
• ​https://www.php.net/manual/en/migration80.incompatible.php

Follow-up to [56530].

Props jrf, hellofromTonya.
See #62061.

Location:

trunk

Files:

• src/wp-includes/class-wp-exception.php (added)
• src/wp-includes/functions.php (modified) (1 diff)
• src/wp-settings.php (modified) (1 diff)
• tests/phpunit/tests/functions/wpTriggerError.php (modified) (1 diff)

trunk/src/wp-includes/functions.php

@@ -6093 +6093 @@
     );
 
+    if ( E_USER_ERROR === $error_level ) {
+        throw new WP_Exception( $message );
+    }
+
     trigger_error( $message, $error_level );
 }

trunk/src/wp-settings.php

@@ -40 +40 @@
 // Include files required for initialization.
 require ABSPATH . WPINC . '/class-wp-paused-extensions-storage.php';
+require ABSPATH . WPINC . '/class-wp-exception.php';
 require ABSPATH . WPINC . '/class-wp-fatal-error-handler.php';
 require ABSPATH . WPINC . '/class-wp-recovery-mode-cookie-service.php';

trunk/tests/phpunit/tests/functions/wpTriggerError.php

@@ -21 +21 @@
      * @param string $expected_message The expected error message.
      */
-    public function test_should_trigger_error( $function_name, $message, $expected_message ) {
-        $this->expectError();
-        $this->expectErrorMessage( $expected_message );
+    public function test_should_throw_exception( $function_name, $message, $expected_message ) {
+        $this->expectException( WP_Exception::class );
+        $this->expectExceptionMessage( $expected_message );
 
         wp_trigger_error( $function_name, $message, E_USER_ERROR );