Wrong documented or coded 'schedule_event' filter

[esemlabel]

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/cron.php#L41
says that $event parameter should always be an object.

But the following code allows to terminate script only when passing "false" values to filter ("", array(), null, 0 or false), whereas checking false as object will produce error.

The documentation should be changed to force check isset( $event->hook ) when using this filter, otherwise the filter should be changed to something like this

<?php
$event = apply_filters( 'schedule_event', $event->hook, $event );
if ( ! $event->hook )
    return false;

[esemlabel]

To not break existing plugins, we can change condition, which allows to "false" only the hook name value

<?php
if ( ! $event->hook )
    return false;

[abhishekfdd]

patch to fix the issue

[swissspidy]

I cannot seem to understand the issue you're facing with this.

Developers would do something like this:

function my_filter_event( $event ) {
    if ( $event  && 'foo' === $event->hook ) {
        // Prevent this specific event from running.
        return false;
    }

    return $event;
}

add_filter( 'schedule_event', 'my_filter_event' );

Of course $event should be an object, but you can return false to short-circuit this. If that's confusing, we can look at updating the inline documentation.

[esemlabel]

Yes, someone can return false to short-circuit this. But many of other developers will produce error when hook in to their even with filter

if ( $event->hook === 'my_plugin_event' ) then perform something inside my plugin

, because they can't predict if anyone else will use such way to prevent event from running.
The one way without modifying core is always use low priority for every filter hook, that return false to 'schedule_event' filter. Otherwise, there is huge probability to catch debug error from plugins and themes.

[swissspidy]

But many of other developers will produce error when hook in to their even with filter […] , because they can't predict if anyone else will use such way to prevent event from running.

But that's the same with every filter out there, isn't it? That's why I added a more strict check in my previous example.

If you understand you correctly, with 40161.patch​ you want to say "don't just return false, but set the hook property to a falsey value, so others don't run into problems". If so, then I think 40161.patch​ is not enough and this would need to be adjusted in the documentation as well, to educate developers.

However, returning false in a filter to short-circuit something is a common pattern in WordPress and in this particular scenario I don't see much value in changing this. I mean, all the plugins out there using this filter won't suddenly change their code. You'd still run into the same problems.

[peterwilsoncc]

I'm closing this as wontfix, as mentioned above it's the nature of WordPress filters that plugins need to be aware a modified value may be passed to their function.

[johnbillion]

The documentation for the $event parameter can still be improved to indicate that allowed values are either an object or boolean false.

[johnbillion]

To clarify: The return value from filters can be anything, but the documentation should show the allowed or expected types.