Skip to content

Fix TypeError on before_delete_post when the post was already deleted by a concurrent request - #564

Open
fabiomsnunes wants to merge 1 commit into
Yoast:trunkfrom
fabiomsnunes:563-fix-before-delete-post-typeerror
Open

fabiomsnunes wants to merge 1 commit into
Yoast:trunkfrom
fabiomsnunes:563-fix-before-delete-post-typeerror

Conversation

@fabiomsnunes

Copy link
Copy Markdown

Context

Summary

This PR can be summarized in the following changelog entry:

  • Fixes a bug where a fatal error occurred when the same post was deleted by two requests at the same time.

Relevant technical choices:

  • The callback now receives the WP_Post that wp_delete_post() already passes as the 2nd argument of before_delete_post (10, 2), so it no longer depends on the row still being in the database.
  • $post is optional and the method falls back to get_post() when it is not passed, so direct callers using only the post ID keep working. The instanceof WP_Post check covers that fallback returning null.
  • The new integration test calls the method directly instead of going through wp_delete_post(): with a real concurrent delete, core's own _reset_front_page_settings_for_post() (also on before_delete_post) emits a PHP warning for the same reason, which PHPUnit turns into an error.
  • I don't have permission to add labels: this needs changelog: bugfix.

Test instructions

Test instructions for the acceptance test before the PR gets merged

This PR can be acceptance tested by following these steps:

  • On trunk, with only Duplicate Post active, save this as repro.php and run wp eval-file repro.php. It simulates a concurrent request deleting the post right before this request's before_delete_post callbacks run:
<?php
$post_id = wp_insert_post( [ 'post_title' => 'raced', 'post_status' => 'publish' ] );

$simulate_concurrent_delete = function ( $id ) use ( &$simulate_concurrent_delete ) {
	global $wpdb;
	remove_action( 'before_delete_post', $simulate_concurrent_delete, 1 );
	$wpdb->delete( $wpdb->posts, [ 'ID' => $id ] );
	clean_post_cache( $id );
};
add_action( 'before_delete_post', $simulate_concurrent_delete, 1 );

wp_delete_post( $post_id, true );
  • See the fatal error TypeError: ... is_rewrite_and_republish_copy(): Argument #1 ($post) must be of type WP_Post, null given.
  • Switch to this branch and run the script again: no fatal error (core may still log a PHP warning from _reset_front_page_settings_for_post(), see above).
  • Regression check for Rewrite & Republish: create a Rewrite & Republish copy of a post, move the copy to the trash and delete it permanently, then open the original post: the notice about an existing Rewrite & Republish copy is gone and a new Rewrite & Republish copy can be created.

Relevant test scenarios

  • Changes should be tested with the browser console open
  • Changes should be tested on different posts/pages/taxonomies/custom post types/custom taxonomies
  • Changes should be tested on different editors (Default Block/Gutenberg/Classic/Elementor/other)
  • Changes should be tested on different browsers
  • Changes should be tested on multisite

Test instructions for QA when the code is in the RC

  • QA should use the same steps as above.

QA can test this PR by following these steps:

  • Same steps as above.

Impact check

This PR affects the following parts of the plugin, which may require extra testing:

  • Rewrite & Republish: cleaning up the original post's flag when its copy is permanently deleted.

UI changes

  • This PR changes the UI in the plugin. I have added the 'UI change' label to this PR.

Documentation

  • I have written documentation for this change. For example, comments in the Relevant technical choices, comments in the code, documentation on Confluence / shared Google Drive / Yoast developer portal, or other.

Quality assurance

  • I have tested this code to the best of my abilities

Checks run locally: composer lint, composer test, composer check-cs-thresholds (57/57 errors, 0/0 warnings, same as trunk) and composer test-wp-env. The integration tests ran with --php=8.4, because composer update on my PHP 8.4 host installs a doctrine/instantiator that doesn't parse on the default PHP 8.2 container. The new unit and integration tests fail on trunk with the TypeError and pass on this branch.

@fabiomsnunes
fabiomsnunes force-pushed the 563-fix-before-delete-post-typeerror branch from a4e1807 to a9c55fa Compare October 14, 2026 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

TypeError in Post_Republisher::clean_up_when_copy_manually_deleted() when the post is already gone (concurrent deletes)

1 participant