Make WordPress Core

Opened 14 months ago

Closed 3 weeks ago

Last modified 9 days ago

#58281 closed enhancement ( fixed )

Rollback Auto-Update (Rollback part 3)

Reported by:  afragen's profile afragen Owned by:  johnbillion's profile johnbillion
Milestone: six point six Priority: normal
Severity: normal Version: six point three
Component: Upgrade/Install Keywords: has-patch dev-feedback needs-testing has-testing-info early commit
Focuses: Cc:

Description (last modified by afragen )

This is Rollback part 3. It began with move_dir() in WP 6.2 for part 1. Part 2 was completed with #51857 in WP 6.3. This brings us to part 3.

Part 3 is Rollback for auto-updates. When manually updating plugins if the plugin has a fatal error on reactivation, the plugin is prevented from reactivating. Unfortunately, during an auto-update, this reactivation check doesn't occur and the the next time the site runs users will see the WSOD.

Rollback Auto-Update performs a similar re-activation check and if there is a fatal error it is captured in an error handler and the previously installed plugin is restored. If this occurs an email will be sent notifying the site admin of the failed update and rollback.

After the rollback, the pending auto-updating for core and theme updates are restarted.

This code is currently running for everyone who has the Rollback Update Failure feature plugin installed.

I personally have been testing this using a plugin that will fatal if the update occurs. The plugin is on my test site, active, and set to auto-update. I have been running like this since the beginning of the year.

The PR is slightly different than the code in the feature plugin. Please test, run the feature plugin on your site, and review the code in the PR. Mostly give us your comments and feedback.

props @costdev and @pbiron for continuing code review, rubber ducking, and sanity checks.

Change History (61)

This ticket was mentioned in PR #3958 on WordPress/wordpress-develop by @afragen .


14 months ago
#1

This PR adds the rollback auto update functionality to core. This is already part of the Rollback Update Failure feature plugin.

This requires PR2225 .

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

This ticket was mentioned in Slack in #core-upgrade-install by afragen. View the logs .


14 months ago

#3 @ afragen
14 months ago

  • Owner set to afragen
  • Status changed from new to assigned

This ticket was mentioned in Slack in #core-upgrade-install by afragen. View the logs .


12 months ago

#5 @ afragen
12 months ago

  • Description modified ( diff )

#6 @ SergeyBiryukov
12 months ago

  • Milestone changed from Awaiting Review to six point four

This ticket was mentioned in Slack in #core-upgrade-install by afragen. View the logs .


12 months ago

This ticket was mentioned in Slack in #core-upgrade-install by afragen. View the logs .


11 months ago

This ticket was mentioned in Slack in #core-committers by afragen. View the logs .


10 months ago

This ticket was mentioned in Slack in #core by oglekler. View the logs .


10 months ago

#11 @ kirasong
10 months ago

Thanks everyone for your work on this!

I left a couple of questions / comments in a review on the PR.

I'm going to try to do some further manual testing, but I don't currently see any blockers for getting a first run of this committed into trunk so that we can get wider testing.

This ticket was mentioned in Slack in #core by oglekler. View the logs .


9 months ago

This ticket was mentioned in Slack in #core-test by oglekler. View the logs .


9 months ago

#14 @ peterwilsoncc
9 months ago

I've tested the linked pull request on our old friend Chassis running on VirtualBox and am seeing some apparent timeouts.

I wrote up a mini-plugin to increase the frequency the update cron jobs run so I didn't need to wait twelve hours for each round of testing. This allowed me to use wp-cron.php for events to emulate real world circumstances.

I then installed the following plugins:

  • akismet 4.1.10
  • gutenberg 16.5.1
  • jetpack 11.3.2
  • mailpoet 4.0.1
  • soliloquy-lite 2.5.2.8
  • woocommerce 7.0.0
  • wordpress-seo 19.7
  • wpforms-lite 1.7.0

Running with the pull request checked out, the updates were failing with the setup mentioned above. Aksimet, Gutenberg and Jetpack would update but the rest of them would fail to upgrade.

As the upgrade failed, no email notification was sent and the auto_updater.lock option would stay in the database preventing the next run from completing.

I am wondering if the PHP request is timing out. In wp-cron.php, one of the first things that runs is fast-finish code to prevent the requests from blocking sites. I am wondering if this is preventing the timeout from being extended when the updates begin running.

This ticket was mentioned in PR #5287 on WordPress/wordpress-develop by @costdev .


9 months ago
#15

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

This performs two loopback requests, one to the dashboard, and one to the homepage, in an attempt to detect such fatal errors.

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

@afragen commented on PR #5287 :


9 months ago
#16

@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!

This ticket was mentioned in Slack in #hosting by javier. View the logs .


9 months ago

#19 @ peterwilsoncc
9 months ago

@costdev @afragen I tested the refreshed PR with the list of plugins I included in comment #14

With the plugins inactive, the updates ran successfully and I got the auto update email.

With the plugins active, it appears to have stalled after the WooCommerce update and I did not get the auto update email and the auto_updater.lock option remains. wordpress-seo and wpforms-lite were not updated.

With the plugins active I get 190ish queries on the plugins admin screen so it makes sense that being active may cause problems as the site is doing a lot more work than while they are inactive.

The backups of the successfully updated plugins remain in the upgrade-temp-backup/plugins folder.

As usual, this is testing on VirtualBox (using Chassis ).

This ticket was mentioned in Slack in #core-upgrade-install by peterwilsoncc. View the logs .


9 months ago

#21 @ afragen
9 months ago

@peterwilsoncc I've run this using all of your plugins and several others, 14 in total including 2 plugins that will throw fatal errors.

My runs on LocalWP and the WP docker env have both been successful and the upgrade-temp-backups/plugins is empty.

Perhaps it's the method you use to help trigger the update? On LocalWP I'm able to use @costdev's cli-force-auto-updates.php plugin. On Docker I use npm run env:cli cron event run wp_version_check .

Last edited 9 months ago by afragen ( previous ) ( diff )

This ticket was mentioned in Slack in #core by oglekler. View the logs .


9 months ago

#23 @ costdev
9 months ago

  • Milestone changed from six point four to six point five

With additional testing, issues were reproduced.

Issues occur when browsing the site during an auto-update, and investigation is ongoing to find a solution. We'll continue work and try again for WordPress 6.5.

Thanks to all for your work and interest in Rollback Auto Update so far!

#24 @ kirasong
9 months ago

Thanks so much @costdev @afragen @peterwilsoncc and everyone who has been working on and testing this feature.

I agree that given the regression in updates (updates not happening that would have succeeded) with the feature applied, and Beta 1 today/tomorrow, it makes sense to punt this to 6.5.

I'm planning to continue to follow this, and happy to help out in 6.5!

This ticket was mentioned in Slack in #core-upgrade-install by afragen. View the logs .


9 months ago

This ticket was mentioned in Slack in #core-upgrade-install by afragen. View the logs .


8 months ago

This ticket was mentioned in Slack in #core by afragen. View the logs .


8 months ago

This ticket was mentioned in Slack in #core-upgrade-install by pbiron. View the logs .


7 months ago

This ticket was mentioned in Slack in #core by afragen. View the logs .


7 months ago

#30 @ annezazu
5 months ago

Checking in here as we're less than a month out from beta 1. Is this still slated for 6.5? I want to ensure it's covered for testing if so.

#31 @ costdev
5 months ago

@annezazu Thanks for checking in!

Yes the feature is still slated for 6.5. Unfortunately, I've been snowed under with agency work and haven't had time to circle back. In the Upgrade/Install Slack channel, I've reached out to other committers to see if they might have bandwidth to take a look at the PR .

With eyes and possible input from more committers, this feature can absolutely land in 6.5.

#32 @ swissspidy
5 months ago

There hasn't been any traction here recently and beta 1 is only a week away. Do you perhaps want to raise this in dev chat, maybe with some testing instructions?

#33 @ costdev
5 months ago

@swissspidy Thanks for flagging the lack of updates posted on the ticket!

There were additional rounds of testing done after I reached out in Slack. @peterwilsoncc will also be doing additional testing today. Let's wait and see if Peter finds any issues and see how this feature looks for getting into Beta 1.

#34 @ costdev
5 months ago

  • Keywords has-testing-info added

Testing instructions can be found here .

This ticket was mentioned in Slack in #core by audrasjb. View the logs .


5 months ago

#36 @ swissspidy
4 months ago

  • Keywords early added
  • Milestone changed from six point five to six point six

No traction here since testing instructions were added.

Let's ensure we get this in early in 6.6 instead!

This ticket was mentioned in Slack in #core-upgrade-install by afragen. View the logs .


4 months ago

This ticket was mentioned in Slack in #core-upgrade-install by afragen. View the logs .


3 months ago

#39 @ audrasjb
2 months ago

Thanks for the great work on this feature!
I added a few comments in the PR :)

#40 @ audrasjb
2 months ago

Thanks for the detailed testing infos!

I tested the PR with the this-plugin-should-not-be-used plugin and the rollback works as expected and the upgrade-temp-backup is empty at the end of the process.

I also tested the PR with a plugin located on custom repositories (with or without a fatal error in the new version) and it looks like it works fine as well.

@johnbillion commented on PR #5287 :


8 weeks ago
#41

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.

@johnbillion commented on PR #5287 :


7 weeks ago
#42

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

@costdev commented on PR #5287 :


7 weeks ago
#43

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 commented on PR #5287 :


7 weeks ago
#44

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.

#45 @ johnbillion
7 weeks ago

  • Keywords commit added
  • Owner changed from afragen to johnbillion
  • Status changed from assigned to reviewing

#46 @ johnbillion
7 weeks ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In fifty-eight thousand one hundred and twenty-eight :

Upgrade/Install: Automatically roll back to the previous version when an automatic plugin update results in a fatal error on the front end of the site.

This builds on the temporary backup system introduced in 6.3 to allow automatic updates to benefit from fatal error protection. A loopback request is performed to the home page of the site and the plugin is rolled back to its backed up version if a fatal error is observed.

For debugging and observability during beta, this change includes several calls to error_log() during the upgrade and rollback stages. These calls can be removed or placed behind a flag once we're ready for RC1.

Props costdev, johnbillion, mukesh27, afragen, audrasjb, justlevine, kirasong, peterwilsoncc

Fixes #58281

#48 @ swissspidy
6 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Just as an FYI, these error_log calls are causing some WP-CLI tests to fail as there is now a lot of unexpected output when running commands.

This ticket was mentioned in PR #6542 on WordPress/wordpress-develop by @costdev .


6 weeks ago
#49

In r58128 , error_log() calls were added. One of these is intended to persist beyond release, the rest are for testing purposes during the 6.6 development cycle.

The testing error_log() calls caused test failures due to unexpected output.

This change guards the testing calls and the post-release call with WP_DEBUG and WP_DEBUG_LOG , similar to various block files' use of WP_DEBUG && WP_DEBUG_DISPLAY guards.

@afragen commented on PR #6542 :


6 weeks ago
#50

Looks good

#51 @ johnbillion
6 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In fifty-eight thousand one hundred and thirty-nine :

Bootstrap/Load: Place the debugging output for automatic plugin and theme updates behind a debugging flag.

This change means this debugging output will only be shown when both the WP_DEBUG and WP_DEBUG_LOG constants are defined as true.

Props costdev, afragen, swissspidy

Fixes #58281

#54 @ costdev
5 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

@johnbillion Looks like we had some inconsistencies in the $is_debug definitions.

PR 6632 should resolve the inconsistencies so that all $is_debug definitions used WP_DEBUG && WP_DEBUG_LOG .

@costdev commented on PR #6632 :


4 weeks ago
#55

@swissspidy Noticed that we had some inconsistencies in the debug guards and some of them checked WP_DEBUG_DISPLAY instead of WP_DEBUG_LOG .

Can you confirm that correcting the code to use WP_DEBUG && WP_DEBUG_LOG for all of the instances (which this PR does) will still prevent unexpected output in WP-CLI tests?

Thanks!

@swissspidy commented on PR #6632 :


4 weeks ago
#56

I think so, yes, When I check out this PR & run wp cron event run wp_version_check wp_update_plugins (which triggered these issues the last time) 👍

#57 @ costdev
3 weeks ago

In fifty-eight thousand three hundred and eight :

Upgrade/Install: Don't toggle maintenance mode on translation updates.

The Rollback Auto-Update feature introduced additional maintenance mode toggling.

After installing WordPress in a non-English (US) language, translation updates are performed automatically. As there may be a large number of updates for Core and bundled themes, users will be presented with a maintenance notice upon visiting the newly installed website.

To avoid concerning users that the website has failed to install correctly, this excludes translation updates from triggering the additional maintenance mode toggling.

Follow-up to [58128] .

Props benniledl, afragen, rajinsharwar, costdev.
Fixes #61260 . See #58281 .

#58 @ costdev
3 weeks ago

PR 6632 has been reviewed by another component maintainer and has been tested by another committer.

Ready for commit . Note: @rogermedia revealed this issue during testing in the hosting Slack channel, and will be included in the props list.

Last edited 3 weeks ago by costdev ( previous ) ( diff )

#59 @ costdev
3 weeks ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In fifty-eight thousand three hundred and nine :

Upgrade/Install: Make $is_debug consistent in WP_Automatic_Updater .

[58139] introduced debugging flags to ensure debugging output would only be shown when both the WP_DEBUG and WP_DEBUG_LOG constants are defined as true. However, some of the flags incorrectly use WP_DEBUG_DISPLAY rather than WP_DEBUG_LOG .

This fixes the flags to consistently use WP_DEBUG and WP_DEBUG_LOG as intended.

Follow-up to [58128] , [58139] .

Props rogermedia, afragen, swissspidy, costdev.
Fixes #58281 .

#60 @ costdev
9 days ago

In fifty-eight thousand four hundred and thirty-five :

Upgrade/Install: Delay automatic updates after installation.

After installation, the user is directed to the Log In page. This triggers the wp_schedule_update_checks() function which is hooked to init and schedules updates to run immediately if no other events exist. As a result of more robust use of maintenance mode for automatic updates added in [58128] , the user may be presented with a maintenance mode screen just after installing WordPress.

To improve the user experience, this schedules core updates for 1 hour, plugin updates for 1.5 hours, and theme updates for 2 hours after installation.

Follow-up to [58128] , [58139] , [58308] , [58309] .

Props afragen, hellofromTonya, peterwilsoncc, nithi22, dd32.
Fixes #61457 . See #58281 , #61391 .

#61 @ costdev
9 days ago

In fifty-eight thousand four hundred and thirty-six :

Upgrade/Install: Disable maintenance mode when core auto-update fails.

In [58128] , additional maintenance mode calls were added to the automatic updates process. However, there is an early return if a 'core' automatic update fails.

Maintenance mode isn't disabled until later in the WP_Automatic_Updater::update() method. This means that maintenance mode may continue to be enabled despite the core update being treated as a skipped update.

This disables maintenance mode before the early return.

Follow-up to [58128] .

Props costdev, hellofromTonya, peterwilsoncc.
Fixes #61459 . See #58281 .

Note: See TracTickets for help on using tickets.