Upgrade/Install: Roll back a plugin auto-update with a fatal error.

[costdev]

The biggest concern when plugins update automatically is that they may contain a fatal error that crashes a website.

This performs a loopback request to the homepage in an attempt to detect such fatal errors in an active plugin after updating. This ensures that only the plugin's new code has been loaded into memory, and prevents false positives when, for example, a plugin may include a class definition in the main plugin file.

Should a fatal error be detected, the update is restored to the previously installed version.

The loopback request is also performed for the Plugin File Editor and Theme File Editor. However, the implementation here does not distinguish between a failed loopback request and a plugin with no fatal errors, as the aim is to make the best attempt to detect a fatal error.

Additionally, the Site Health error message for the loopback request test states:

Loopback requests are used to run scheduled events, and are also used by the built-in editors for themes and plugins to verify code stability.

So if loopback requests are not possible, automatic updates should not be possible anyway. 🙂

Trac ticket: https://core.trac.wordpress.org/ticket/58281

[afragen]

@costdev and I have been testing all combinations of plugins, fataling plugin, and themes with this, even @peterwilsoncc's list of plugins.

It tests out great!

[github-actions]

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

• The Plugin and Theme Directories cannot be accessed within Playground.
• All changes will be lost when closing a tab with a Playground instance.
• All changes will be lost when refreshing the page.
• A fresh instance is created each time the link below is clicked.
• Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
  it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

[github-actions]

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @[email protected].

Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases.

Core Committers: Use this line as a base for the props when committing in SVN:

Props costdev, johnbillion, mukesh27, afragen, audrasjb, justlevine.

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

[afragen]

This should better integrate without the need to return a WP_Error

[audrasjb]

I added a few comments :)

[johnbillion]

I added some minor code review comments, but in general it's looking good. I'm going to functionally test this over the next few days.

[afragen]

Suggested change

	/*
	* Avoids a race condition when there are 2 sequential plugins that have fatal errors.
	* WP_Automatic_Updater::update() can be called for the second plugin before the first has
	* finished checking for fatal errors. This can cause the second plugin's fatal error checking to be skipped,
	* and may affect subsequent plugins too.
	*/
	/*
	* Avoids a race condition when there are 2 sequential plugins that have fatal errors.
	* It seems a slight delay is required for the loopback to use the updated plugin code in the request.
	* This can cause the second plugin's fatal error checking to be inaccurate,
	* and may also affect subsequent plugin checks.
	*/

[costdev]

I'm not sure that documenting what doesn't work is useful here, but posting on the ticket what's been checked and verified is worth doing for posterity. In the code, we should document the reasoning for the sleep, and ensure that we've verified the details as much as we can.

[afragen]

By all means modify as necessary.

[afragen]

Updated

[johnbillion]

What shall we do with all the error_log() calls in this PR? Strip them out?

[costdev]

For debugging purposes, since this is a background task, it would be great if we could put them behind a WP_DEBUG flag so any reported issues can detail where things went wrong, then remove this logging later in the cycle.

[afragen]

I say leave them for now for broader testing. Somewhere around beta1 we can put the loopback results behind WP_DEBUG and remove most of the others.

[johnbillion]

https://core.trac.wordpress.org/changeset/58128 🚀