[wp-trac] [WordPress Trac] #17817: do_action/apply_filters/etc. recursion on same filter kills underlying call

[WordPress Trac]

#17817: do_action/apply_filters/etc. recursion on same filter kills underlying call
----------------------------------------+-----------------------
 Reporter:  kernfel                     |       Owner:
     Type:  defect (bug)                |      Status:  reopened
 Priority:  normal                      |   Milestone:  3.6
Component:  Plugins                     |     Version:  3.4.1
 Severity:  normal                      |  Resolution:
 Keywords:  has-patch needs-unit-tests  |
----------------------------------------+-----------------------

Comment (by cheeserolls):

 Jbrinley, I don't agree with your comment.

 You said:
 > In my opinion, it should be up to a plugin author to take care of the
 housekeeping if he ''decides'' to nest action calls.

 My whole point is that it usually won't be a ''conscious decision'' by a
 plugin author to nest action calls.  They won't be directly calling
 do_action('something').  But they might well use another wordpress
 function, which they read about in the wordpress API, which inadvertently
 sets off some other actions.

 For example, I discovered this 'bug' through some strange behaviour in the
 Wordpress Multilingual plugin.  They need to automatically update post B
 when post A is saved.  So of course they hooked into the 'save_post'
 action when post A is saved, and called wp_update_post() on post B.  This
 seems like a completely logical and sensible approach, but it
 'accidentally' caused a nested 'save_post' action, and the inner action
 broke the outer action.

 You're saying, the plugin author should be left to clean up his own mess.
 But how on earth would the plugin author be expected to know he'd made a
 mess in the first place?  You cannot expect a standard wordpress function
 called in an action, to prevent the rest of the actions executing.  The
 only way a plugin author could know, would be to have read the source code
 of do_action, wp_update_post, and everything else that stemmed from it.

 I appreciate your point that you won't be able from inside a hook, to add
 another callback on the same hook.  I must admit that it did not occur to
 me.  But there's an easy workaround for that too:  A plugin author just
 has to put the conditional logic inside the second callback instead of the
 first.  In your example:


 {{{
 function my_first_callback( $post_id, $post ) {
   // do something
 }
 function my_second_callback( $post_id, $post ) {
   if ( $post->post_type == 'pistachio' ) {
     // do something else
   }
 }
 add_action('save_post', 'my_first_callback', 10, 2);
 add_action('save_post', 'my_second_callback', 15, 2);
 }}}


 In contrast the workaround you suggest for plugins in your blog post
 (Remembering the position of the array pointer, then manually finishing
 the do_action loop after you break it.) is torturous and error prone, and
 depends on plugin authors having  huge amount of knowledge about WP's
 internal code.  It's against the whole spirit of plugins.

 It violates the 'principle of least astonishment'.

 A plugin author who sets up new actions after a sequence of actions has
 already started would surely not be too surprised to find it didn't work.
 And would easily be able to fix it.

 A plugin author who calls a wp function inside his action, would be
 astonished to find that he had inadvertently prevented all other plugins
 that used the same action from working.

 Furthermore, if a plugin author has a problem because he can't add new
 callbacks to the same hook, at least he will break his own plugin, and no-
 one else's.  And he'll therefore know that it's not working and have a
 chance to fix it.  The current behaviour is much nastier, because a plugin
 author is likely to instead break other plugins, and won't even know that
 he's done it.

-- 
Ticket URL: <http://core.trac.wordpress.org/ticket/17817#comment:18>
WordPress Trac <http://core.trac.wordpress.org/>
WordPress blogging software