Don't install a custom error handler for hooks
authorOri Livneh <ori@wikimedia.org>
Fri, 4 Dec 2015 19:59:52 +0000 (11:59 -0800)
committerOri Livneh <ori@wikimedia.org>
Fri, 4 Dec 2015 19:59:52 +0000 (11:59 -0800)
Installing a custom error handler on every hook invocation has a high overhead,
and does not even correctly achieves what it sets out to achieve, which is to
flag hook function signature errors (and only hook function signature errors).
The "PHP way" is to simply increase the error reporting level for development
environments, which we do already.

Bug: T117553
Change-Id: Iba0138a6d0a0ddf839bc5a36e03cadb012e06f3c

includes/Hooks.php

index 980d350..9018581 100644 (file)
@@ -193,34 +193,17 @@ class Hooks {
                        $badhookmsg = null;
                        $hook_args = array_merge( $hook, $args );
 
-                       set_error_handler( 'Hooks::hookErrorHandler' );
-
                        // mark hook as deprecated, if deprecation version is specified
                        if ( $deprecatedVersion !== null ) {
                                wfDeprecated( "$event hook (used in $func)", $deprecatedVersion );
                        }
 
-                       try {
-                               $retval = call_user_func_array( $callback, $hook_args );
-                       } catch ( MWHookException $e ) {
-                               $badhookmsg = $e->getMessage();
-                       } catch ( Exception $e ) {
-                               restore_error_handler();
-                               throw $e;
-                       }
-
-                       restore_error_handler();
+                       $retval = call_user_func_array( $callback, $hook_args );
 
                        // Process the return value.
                        if ( is_string( $retval ) ) {
                                // String returned means error.
                                throw new FatalError( $retval );
-                       } elseif ( $badhookmsg !== null ) {
-                               // Exception was thrown from Hooks::hookErrorHandler.
-                               throw new MWException(
-                                       'Detected bug in an extension! ' .
-                                       "Hook $func has invalid call signature; " . $badhookmsg
-                               );
                        } elseif ( $retval === false ) {
                                // False was returned. Stop processing, but no error.
                                return false;
@@ -229,31 +212,4 @@ class Hooks {
 
                return true;
        }
-
-       /**
-        * Handle PHP errors issued inside a hook. Catch errors that have to do
-        * with a function expecting a reference, missing arguments, or wrong argument
-        * types. Pass all others through to to the default error handler.
-        *
-        * This is useful for throwing errors for major callback invocation errors
-        * (with regard to parameter signature) which PHP just gives warnings for.
-        *
-        * @since 1.18
-        *
-        * @param int $errno Error number (unused)
-        * @param string $errstr Error message
-        * @throws MWHookException If the error has to do with the function signature
-        * @return bool
-        */
-       public static function hookErrorHandler( $errno, $errstr ) {
-               if ( strpos( $errstr, 'expected to be a reference, value given' ) !== false
-                       || strpos( $errstr, 'Missing argument ' ) !== false
-                       || strpos( $errstr, ' expects parameter ' ) !== false
-               ) {
-                       throw new MWHookException( $errstr, $errno );
-               }
-
-               // Delegate unhandled errors to the default handlers
-               return false;
-       }
 }