Customize: Keep stored custom CSS post ID when the lookup fails. - #14225

Should fix: This changes behaviour but has no test yet, as @webd-uk also asked. A DB failure is easy to simulate with the query filter. The test below fails on trunk (Failed asserting that -1 is identical to <ID>) and passes with this PR. I ran it locally, so it could go into tests/phpunit/tests/customize/custom-css-setting.php or a new tests/phpunit/tests/theme/wpGetCustomCssPost.php:

/**
 * @ticket 54443
 *
 * @covers ::wp_get_custom_css_post
 */
public function test_stored_post_id_is_kept_when_lookup_fails() {
	global $wpdb;

	$post = wp_update_custom_css_post( 'body { color: red; }' );

	$suppress = $wpdb->suppress_errors( true );
	$break    = static function ( $query ) use ( $wpdb ) {
		return str_contains( $query, $wpdb->posts ) ? 'SELECT broken FROM missing_table' : $query;
	};
	add_filter( 'query', $break );
	clean_post_cache( $post->ID );

	$this->assertNull( wp_get_custom_css_post(), 'The lookup should fail.' );

	remove_filter( 'query', $break );
	$wpdb->suppress_errors( $suppress );

	$this->assertSame( $post->ID, get_theme_mod( 'custom_css_post_id' ), 'The stored post ID should be kept.' );
}

A second test for a site with no custom CSS post caching -1 would cover the other branch.

Question: Would checking $wpdb->last_error right after the WP_Query work here instead? In the failure simulation above, last_error is set at that point. Skipping set_theme_mod() only when the query errored would protect the stored ID and still let a really deleted post fall back to -1, so there are no repeated lookups. I know @dlh found diagnosing DB errors here a bit out of scope in comment:3, so this is only an option for whoever commits it. The current approach is fine for the case in the ticket. Roughly (with global $wpdb; added to the function):

if ( ! $post && $wpdb->last_error ) {
	// The lookup failed; keep the stored ID rather than caching "no post".
} else {
	set_theme_mod( 'custom_css_post_id', $post ? $post->ID : -1 );
}

Just to add some real-world context to this trade-off.

I've encountered orphaned custom CSS posts dozens of times over the years across both my own and clients' WordPress installations. I can't 100% say every instance was caused by this particular bug, but it's certainly a recurring problem.

On the other hand, I've never encountered anyone manually deleting a custom CSS post directly from the database. Doing so bypasses WordPress's normal housekeeping anyway, so some resulting inconsistencies are to be expected.

Personally, I'd favour keeping the fix simple rather than introducing additional complexity to accommodate an edge case that, in my experience at least, is considerably less likely than the problem we're trying to solve.

On the manually deleted case, I agree with @webd-uk. Deleting the custom_css post directly in the database skips WordPress's own housekeeping, so it's already "doing it wrong". Once the CSS is saved again it fixes itself, until then the cost is a few extra queries per request. I think that's a better trade-off than a site losing its CSS completely.

I originally thought about checking $wpdb->last_error first and decided against it for the following reasons:

1. @dlh pointed out in comment:3 that diagnosing database errors in this function is out of scope, and his comment:6 points to the approach this PR takes instead.
2. It doesn't catch every failure. If the connection drops and reconnecting fails after template_redirect (custom CSS loads on wp_head), wpdb::query() returns false without setting last_error. -1 would still be cached in that case.
3. last_error only describes the most recent query. A plugin running its own query from posts_results or the_posts would overwrite it before the check.