White screen of death if theme can't be found

[ericlewis]

If the current theme can't be found or is renamed, the front-end of the site will white screen. This is developer error, as when WP_USE_THEMES is true, the template loader should always have a template to load (as far as I understand).

It's rare, but I come across it every once in a while in developing. It might be nice to have a wp_die() if a proper template can't be found in template-loader.

Proof of concept:

if ( $template = apply_filters( 'template_include', $template ) )
	include( $template );
else
	wp_die("Whoops! Looks like you're missing a theme.");

[MikeHansenMe]

I think this is a good idea as most users would not know where to begin looking if they experienced a white screen.

[miqrogroove]

Duplicate of #11282.

[SergeyBiryukov]

Related, but not exactly a duplicate.

[scribu]

I've also run into this. Should be careful with the error message, since it can show up on live sites in certain circumstances.

[ericlewis]

Replying to scribu:

Should be careful with the error message, since it can show up on live sites in certain circumstances.

Can you expand on that? I can't imagine the circumstance where $template is expected to be empty.

[scribu]

I remember I screwed up a deploy once and the theme was renamed. Since I had deleted the twentyeleven theme, I got a WSOD. With this bug, I would get a message instead.

What I'm saying is that the error message shouldn't assume that it will only be seen by developers.

[MikeHansenMe]

what about "Looks like the website is having theme trouble, please notify the site owner" or something to that effect?

[ericlewis]

After chatting with Nacin in IRC a bit, some thought has to be put in here in regard to the ambiguity of the problem, and the ambiguity of the audience. This situation may arise from not just the themes folder missing, it may be a filesystem error, or come from another unknown cause. Therefore, our presentation of the error should respect this fact, and not send developers with possibly misleading debug information.

[scribu]

It's nearly the same thing, but not quite; as the developer, you can at least know that WP was loaded.

And if there's a hook available, you can write a plugin that outputs a more useful message or logs it somewhere.

[SergeyBiryukov]

Could we do something along the lines of what wp_dashboard_right_now() does?
​http://core.trac.wordpress.org/browser/tags/3.4.2/wp-admin/includes/dashboard.php#L365

If the current theme is not found, 21931.2.diff​ shows "The theme directory does not exist".

Related #21670 (for more exact message in the admin).

[MikeHansenMe]

The thing about using Right Now in the dashboard to deliver messages is that it can be turned off. Perhaps there is a problem with that as well. Either not allow right now to be turned off or move error messages above the separate content blocks?

[johnbillion]

Replying to scribu:

What I'm saying is that the error message shouldn't assume that it will only be seen by developers.

Let's make the message capability-dependant then.

if ( current_user_can( 'switch_themes' ) )
	$message = __( 'You just broke everything!' );
else
	$message = __( 'Everything is fine and we will be back shortly.' );

wp_die( $message );

[SergeyBiryukov]

Replying to MikeHansenMe:

The thing about using Right Now in the dashboard to deliver messages is that it can be turned off.

The idea behind 21931.2.diff​ was to display a similar message on the front end.

[MikeHansenMe]

Replying to SergeyBiryukov:

Replying to MikeHansenMe:

The thing about using Right Now in the dashboard to deliver messages is that it can be turned off.

The idea behind 21931.2.diff​ was to display a similar message on the front end.

I thought you were suggesting a user message in the front and developer error in the admin. I do think giving the error on the front end is a good idea because like I said, the Right Now area in the admin can be turned off. I think that may be a problem of it's own though.

[SergeyBiryukov]

Related: #26143

[MikeHansenMe]

Refreshed

[MikeHansenMe]

Just tested this and the WSOD is still the case. Refreshed existing patch.

[SergeyBiryukov]

#13768 was marked as a duplicate.

[SergeyBiryukov]

#32199 was marked as a duplicate.

[danielbachhuber]

+1 to showing different messages based on user capability.

The WSOD still trips me up when I come across it. The first few times it happened, I remember losing an hour trying to figure out why my frontend didn't load.

+100 to showing a message when the theme can't be found.

[ericlewis]

I think we should do something along the lines of @SergeyBiryukov's suggestion, see attachment:21931.3.diff​. which outputs the message The theme directory "{{ theme_directory_name }}" does not exist.

What's wrong with a site visitor seeing this message? Visitors can see the Error establishing a database connection message, this seems in the same vein. A WP site needs a DB connection, a WP site needs a theme. Fail early, fail explicitly.

@nacin, @SergeyBiryukov — thoughts?

[SergeyBiryukov]

This check seems a bit out of place in wp_templating_constants(), and I can't remember why I put it there :)

21931.4.diff​ moves it to template-loader.php, as per the initial suggestion and @MikeHansenMe's first patch.

[nacin]

I don't hate 21931.4.diff​, but I'm not sold on showing this to anonymous users. Minor code thing: a WP_Error object can be passed directly to wp_die().

Way back when (according to lore), WP used to actively switch to the default theme on the frontend when the theme was broken. In some situations (network-based filesystem, perhaps? — I assume an I/O glitch), this would cause a perfectly normal site to suddenly switch to the default theme. This now only happens in the admin in validate_current_theme(). Just providing some additional context, which may help inform whether cap checks are necessary here.

One other question to think about: under what circumstances would this else block be hit? Are we unnecessarily calling wp_get_theme() (a filesystem hit) in a common situation where we shouldn't?

[ericlewis]

Replying to nacin:

I don't hate 21931.4.diff​, but I'm not sold on showing this to anonymous users. Minor code thing: a WP_Error object can be passed directly to wp_die().

Updated in attachment:21931.5.diff​, let's show this to logged-in users.

Replying to nacin:

One other question to think about: under what circumstances would this else block be hit? Are we unnecessarily calling wp_get_theme() (a filesystem hit) in a common situation where we shouldn't?

This should only be hit if a theme template is intended to be loaded as far as I can imagine.

[ericlewis]

In attachmnet:21931.6.diff, ​use `elseif`.

[ericlewis]

In 36335:

Themes: Show an error message to logged-in users if a template file isn't loaded.

On the off-chance the active theme folder is renamed or deleted, a "white screen
of death" was displayed to the user. Instead, the user is shown a useful error
screen displaying any errors the theme has (e.g. if the theme can't be found).

Props MikeHansenMe, SergeyBiryukov.
See #21931.

[joehoyle]

What's the reasoning behind only checking is_user_logged_in() - this doesn't really prove anything about the user being privileged to see anything special. For example, you can be a user without any capabilities whatsoever. IMO is_user_logged_in() shouldn't be used to indicate anything to do with privilege, we should use current_user_can( 'manage_themes' ) or something similar. If there's no need for any permissions to view this, then is_user_logged_in seems arbitrary.

[ericlewis]

The previous attachment shows the error page.

[ericlewis]

Replying to joehoyle:

What's the reasoning behind only checking is_user_logged_in() - this doesn't really prove anything about the user being privileged to see anything special. For example, you can be a user without any capabilities whatsoever. IMO is_user_logged_in() shouldn't be used to indicate anything to do with privilege, we should use current_user_can( 'manage_themes' ) or something similar. If there's no need for any permissions to view this, then is_user_logged_in seems arbitrary.

That's a good point. Added that cap check in attachment:21931.7.diff​.

[ericlewis]

install_themes seems like the right cap check here.

[ericlewis]

In 36338:

Themes: Only users with proper capability should see theme errors.

After [36335], if a template file is not loaded, an error is displayed
to logged-in users. As logged-in users may have no capabilities,
this check is insubstantial. Limit the display of this error to users
with the install_themes capability, i.e. someone who has the capacity
to deal with the error.

See #21931.

[johnbillion]

Why install_themes instead of switch_themes?

[ericlewis]

Replying to johnbillion:

Why install_themes instead of switch_themes?

I was thinking in the worst-case scenario where all themes have been deleted, it will require a user with install_themes to fix the problem. What do you think?

[dd32]

I feel a more basic switch_themes would be more appropriate if anything here.

Keep multisite in mind as a use-case, where a site admin would rarely have the ability to install themes.

[sebastian.pisula]

I use this filter:

<?php
add_filter('template_include', function(){
        return locate_template('test.php');
});

And I have white page. I think that should be this if:

<?php
if ( $template = apply_filters( 'template_include', $template ) ) {
                include( $template );
        } elseif ( current_user_can( 'install_themes' ) ) {
                $theme = wp_get_theme();
                if ( $theme->errors() ) {
                        wp_die( $theme->errors() );
                } else {
                        wp_die( __( 'Template is missing.' ) );
                }
        }

But theme->erros return error if not exists index.php file so better patch is:

<?php
        if ( $template = apply_filters( 'template_include', $template ) ) {
                include( $template );
        } elseif ( current_user_can( 'install_themes' ) ) {
                wp_die( __( 'Template is missing.' ) );
        }

[sebastian.pisula]

Patch for 4.5-alpha-1453085953122

[iseulde]

Love it

[ericlewis]

In 36344:

Themes: Show template loading error to users with switch_themes cap.

In [36338], a template loading error was shown only to users with the install_themes capability. This is now displayed users with the switch_themes capability, as users with this cap can at least switch to a different theme. Also, this will now show for site administrators in multisite, whereas install_themes is limited to superadmins.

Props dd32.
See #21931.

[ericlewis]

Replying to sebastian.pisula:

But theme->erros return error if not exists index.php file so better patch is:

<?php
        if ( $template = apply_filters( 'template_include', $template ) ) {
                include( $template );
        } elseif ( current_user_can( 'install_themes' ) ) {
                wp_die( __( 'Template is missing.' ) );
        }

Thanks for the suggestion here @sebastian.pisula!

This approach is more prone to break things. I could imagine plugins purposefully filtering template_include to a falsy value to, say, output some JSON (or who knows what else). Of course this is not what template_include is intended for, but in support of backwards compatibility, we should respect this functionality by displaying an error only if there are issues with the theme, which is an exceptional problem for the template loader.

[ericlewis]

Going to close this as fixed, thanks to all involved :)

If anyone would like to continue discussion about whether this error should be displayed to anonymous users, feel free to open a new ticket.