Fatal errors after [15531] when conditional tags are used too early

[zamoose]

Upon activating BuddyPress, the entire site dies with the following error:

Fatal error: Call to a member function is_page() on a non-object in [file path]/wp-includes/query.php on line 371

Prior to a recent SVN rev (I'm not sure which), BuddyPress worked on 3.1 alpha.

[nacin]

The changes introduced in [15531] will generate fatal errors in plugins that have been incorrectly using the $wp_query object before it is properly initialized.

I like the idea of leaving in fatal errors as then plugin developers know something is wrong -- we did so for register_taxonomy() with rewrite = true before init, although it wasn't a conscious decision to do so -- but because this change affects conditional tags, it'll likely break a good number of plugins and maybe themes.

I have an interesting idea for this -- we leave this as is until late in the dev cycle. Hopefully that fixes up a good number of plugins including BuddyPress. And then we add sanity checks before release. Perhaps crazy, but at least it'll fix up a lot of code along the way.

[zamoose]

I've created a corresponding ticket for BP over here: ​http://trac.buddypress.org/ticket/2609

We'll see if that gets any traction.

As a side note: is there a way to turn on annotation in Trac's source view so that I can see when files were committed/whom they were committed by?

[nacin]

Replying to zamoose:

As a side note: is there a way to turn on annotation in Trac's source view so that I can see when files were committed/whom they were committed by?

Look for the annotate link in the top right corner.

[johnjamesjacoby]

Thanks for making a ticket over on bp.org. Will try to sneak a fix into the next bug-fix release.

[zamoose]

Replying to nacin:

Replying to zamoose:

As a side note: is there a way to turn on annotation in Trac's source view so that I can see when files were committed/whom they were committed by?

Look for the annotate link in the top right corner.

Thanks. Dunno why I missed that the first time.

[demetris]

Replying to nacin:

I have an interesting idea for this -- we leave this as is until late in the dev cycle. Hopefully that fixes up a good number of plugins including BuddyPress. And then we add sanity checks before release. Perhaps crazy, but at least it'll fix up a lot of code along the way.

Another idea would be to make a post on wpdevel, from now, tagged 31compat and going something like:

Please check your plugins/themes with WP_DEBUG on WordPress 3.0. If you get “Trying to get property of non-object” notices in WP 3.0, that means that your plugin/theme will cause FATAL ERRORS in WP 3.1.

Why this happens: Blah blah.

How to fix: Blah blah.

[ryan]

Good idea demetris.

​http://wpdevel.wordpress.com/2010/08/30/dont-use-the-conditional-query-tags-i/

[scribu]

Also opened a topic in the forums:

​http://wordpress.org/support/topic/fatal-error-in-wp-31-alpha

[zamoose]

Anyone mind schooling me, as someone not all that familiar with the intricacies of query.php, why r15531 caused this behavior to go from "warning" to "Fatal error"? I'm parsing through the diff and I think it's due to the change in the types that the various is_*() functions return. Is that correct?

[nacin]

If $wp_query is not set, then accessing a nonexistent property ($wp_query->is_page) triggers a notice (maybe a warning, don't remember). But accessing a nonexistent method ($wp_query->is_page()) is a fatal error.

[zamoose]

Replying to nacin:

If $wp_query is not set, then accessing a nonexistent property ($wp_query->is_page) triggers a notice (maybe a warning, don't remember). But accessing a nonexistent method ($wp_query->is_page()) is a fatal error.

Ahh! Precisely the open lead I needed connected! Many thanks.

[markjaquith]

I upgraded my site to trunk and got WSOD on the front, and an is_feed fatal error on the backend that gave no clue as to which plugin might be causing it. We should check is_object( $wp_query ) before calling a method on it, rather than issue a fatal error.

[nacin]

Replying to markjaquith:

I upgraded my site to trunk and got WSOD on the front, and an is_feed fatal error on the backend that gave no clue as to which plugin might be causing it. We should check is_object( $wp_query ) before calling a method on it, rather than issue a fatal error.

​http://core.trac.wordpress.org/ticket/14729#comment:1

Suggesting we leave this for deep into RC, with the hope that many plugins will be able to correct themselves during the beta/RC phases. Marking for blocker so we don't forget the important part :-)

[SergeyBiryukov]

Similar: #14024

[avbentem]

Replying to nacin:

Suggesting we leave this for deep into RC, with the hope that many plugins will be able to correct themselves during the beta/RC phases.

Now that the beta is out, you might want to reconsider that? I cannot install BuddyPress, which kind of implies I simply cannot test the beta. This is NOT a problem for me, but it might apply to others too and hence you might get less test results than expected?

[nacin]

Tempted to finally introduce a _deprecated_message() for this use case and for some of the more 'innovative' uses of _deprecated_argument().

Or maybe doing_it_wrong( $message ) :-)

[westi]

I think that a generic _doing_it_wrong() type call is best for these cases where we detect a plugin misbehaving.

[filosofo]

Since we get PHP 5 in 3.2, we could throw custom exceptions for these kinds of things.

[nacin]

(In [16947]) You are doing it wrong in query.php. see #15824, see #14729.

[nacin]

Patch here for when we're ready to do this.

[demetris]

Re: 14729.diff

Maybe we could also include a note/reminder to remove the isset() checks at some point in the future; say, when we start developing v3.3.

[scribu]

That would only make sense if by v3.3 it would somehow be less likely that developers would make that mistake, which I doubt.

[nacin]

Don't think we should do that. This is going to break a number of themes and plugins. Unfortunately an E_NOTICE is not enough to believe that we can remove anything, whether it be this or a nooped deprecated function. I agree with scribu.

As a language construct, isset() is crazy fast. That's not something to be concerned with.

[nacin]

(In [17068]) Don't fatal error when conditional tags are called before init. fixes #14729.

[nacin]

(In [17083]) Clarify the _doing_it_wrong message for conditioanl tags. init prevents the fatal error, but the query still isn't run. Just drop the mention of a hook and we end up with better reasoning why this doesn't work to boot. see #14729.