bp_setup_current_user() over zealous checking for did_action( "after_setup_theme" )

[bobbingwide]

With WordPress-SEO, BuddyPress and WP_DEBUG true, BuddyPress produces the following _doing_it_wrong message in the Dashboard.

The current user is being initialized without using $wp->init().

In my opinion this message can be avoided by changing the test to something more closely aligned to the problem it is try to warn against... the fact that the current user may not be set.

With the current version of WordPress, 4.2 and above, the test can be changed to

global $current_user; 
if ( is_null( $current_user ) ) {
    _doing_it_wrong( ... );
}

See ​http://wordpress.stackexchange.com/questions/198185/what-is-valid-timing-of-using-current-user-can-and-related-functions.

bbPress has the same problem.

An alternative approach, that I did not investigate, is to not register the action hook for bp_set_current_user until 'after_setup_theme' has been fired.

[DJPaul]

Think this is/was used to detect a number of non-trivial user roles/capabilities issues that was discovered when bbPress 2 was being built. Need to let JJJ look at this as he did the work on that, this might not be such an easy change.

[netweb]

This is not a BuddyPress or bbPress issue, this is a WordPress SEO issue ​https://github.com/Yoast/wordpress-seo

See ​https://codex.bbpress.org/bbp_setup_current_user/was-called-incorrectly/

This was also supposedly fixed in WordPress SEO by Yoast a few months ago and appears to be happening again :/
​https://github.com/Yoast/wordpress-seo/blob/trunk/readme.txt#L253

Release Date: April 21st, 2015

* Bugfixes:
	* Fixes a compatibility issue with BBPress caused by hooking `current_user_can` too early.

[bobbingwide]

Hi @netweb, even though Rarst has addressed this issue for WordPress SEO the problem still exists with other plugins that use current_user_can() before "after_setup_theme".
In my limited experience both BuddyPress and bbPress are working fine.

Another alternative would be not to invoke the 'bp_setup_current_user' action either by testing did_action( 'after_setup_theme') or not calling add_action( 'bp_setup_current_user') until 'after_setup_theme' has been invoked.

[DJPaul]

I don't think we want to change this. All the instances we've had reported have always ended up showing problems in other plugins (minus one for the Customiser in wp-core, which we special-case).

[bobbingwide]

Hi Paul, Rarst's other example was his own plugin - Toolbar Theme Switcher.
He argued in the stack exchange link he had to do it this way.

Do you want to reconsider?

[r-a-y]

I think we should add more info to these bp_setup_current_user() notices so plugin devs can see where their code is throwing these notices.

Attached patch does this by using Exception::getTraceAsString(), which is available in PHP 5.1+.

This is what the notice looks like with the patch:

[08-Apr-2016 00:32:38 UTC] PHP Notice:  bp_setup_current_user was called <strong>incorrectly</strong>. The current user is being initialized without using $wp->init().
===
Trace:
#6 wp-content\plugins\bp-custom.php(53): is_user_logged_in()
#7 [internal function]: BuddyPress->{closure}('')
#8 wp-includes\plugin.php(525): call_user_func_array(Object(Closure), Array)
#9 wp-settings.php(276): do_action('plugins_loaded')
#10 wp-config.php(109): require_once(...)
#11 wp-load.php(37): require_once(...)
#12 wp-blog-header.php(12): require_once(...)
#13 index.php(17): require(...)
#14 {main}
===
 Please see <a href="https://codex.wordpress.org/Debugging_in_WordPress">Debugging in WordPress</a> for more information. (This message was added in version 1.7.) in wp-includes\functions.php on line 3792

---

@bobbingwide - If you really wanted to, you can suppress these notices with a custom PHP error handler:
​https://gist.github.com/r-a-y/bab85fc47fa0357a8100

[boonebgorges]

6589.01.patch​ is pretty clever.

[DJPaul]

new \Exception is invalid syntax in PHP 5.2. :( :)

Do we really need that here?

[r-a-y]

new \Exception is invalid syntax in PHP 5.2

Good catch! The namespacing isn't necessary.

I've refreshed 01.patch to remove the backslash.

[r-a-y]

In 10709:

Core: Add debug trace to bp_setup_current_user() notice.

Previously, the bp_setup_current_user was called incorrectly notice
would not output any useful info for developers.

This commit adds a debug trace so developers are able to determine
whereabouts in the codebase this notice is originating from.

See #6589.

[netweb]

Nice, is there anything else here @r-a-y ? I've added a patch in ​#bbPress2932 and works great :)

[r-a-y]

Nice, is there anything else here

@netweb - Not really. Perhaps we can alter the notice message to say something to the effect of "Please try running your current user check on the 'init' hook or later."

"The current user is being initialized without using $wp->init()" is a little vague if you're not familiar with WP internals.

[netweb]

Replying to r-a-y:

Nice, is there anything else here

@netweb - Not really. Perhaps we can alter the notice message to say something to the effect of "Please try running your current user check on the 'init' hook or later."

"The current user is being initialized without using $wp->init()" is a little vague if you're not familiar with WP internals.

Sounds good :+1

[espellcaste]

This is a really nice and very useful enhancement.

[DJPaul]

This is done. If someone cares enough to change the string, that can be done in a new ticket.