Silent failure when wp_embed_register_handler() has a non-public callback

[paulschreiber]

WordPress fails silently when wp_embed_register_handler() has a non-public callback

If you have a class like this:

class Stuff {
   public static function hooks() {
     wp_embed_register_handler( 'foobar', $regex, array( __CLASS__, 'embed_handler' ) );
   }

  private static function embed_handler( $matches, $attr, $url, $rawattr ) {
    //.... do stuff
  }
}

And you run

Stuff::hooks();

Your handler is never registered and no error is shown or logged.

By contrast, add_action() checks for this condition and logs an error:

PHP Warning:  call_user_func_array() expects parameter 1 to be a valid callback, cannot access private method Stuff::enqueue_scripts() in /srv/www/wp/wp-includes/plugin.php on line 505

[leewillis77]

Actually, add_action() doesn't check for the callability of the function - hence PHP throwing the warning.

The embed code specifically checks that the callback is callable, and doesn't try to call it if not, so no error is thrown. I'm pretty sure we wouldn't want to remove that check, and have the embed code start throwing warnings where it didn't before.

It's possible we could add a check when the handler is registered - however, just because the function isn't callable at that point doesn't mean it won't be at the time it's needed. Perhaps making wp_embed_register_handler() return false if the callback isn't callable but registering it anyway, and relying on the caller to notice the return value that would be acceptable (Patch attached), not 100% sure that "smells" right though?

If someone's happy that's an acceptable approach I'll expand the patch to include tests.
We could also consider the fact that do_action() throws a warning to be the bug and fix that up

[wonderboymusic]

Lemme think about this. The embed handler not working should be enough of a sign that that your method has problems. I would argue against making classes where every method is static and a few are marked private for fun. Action handlers have to be public, always.

[helen]

I agree with the above observations.