WordPress.org

Make WordPress Core

Opened 10 months ago

Last modified 4 days ago

#51857 reopened enhancement

Add rollback for failed plugin/theme updates

Reported by: pbiron Owned by: pbiron
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: early has-testing-info needs-patch needs-unit-tests
Focuses: Cc:

Description (last modified by pbiron)

When a core auto-update fails, core has long had the ability to automatically attempt a "rollback" to the latest stable release (in the branch that the site is running). Note: for core auto-updates, "rollback" is not attempted for certain failures (e.g. "disk full", etc).

This capability should be extended to be available for plugin and theme updates.

[note: originally this description mentioned `auto-updates`...that was a mistake on my part. This is about rollback for both `manual` and `auto-updates`]

The current functionality for core rollbacks relies on the availability of a "rollback" package being returned by the Updates API. It is unclear (to me, at this time) whether extending this functionality to plugins/themes will require changes to the API or not.

Important: This ticket is not about backup/restore in case a site admin later determines that a successful plugin/theme update actually causes problem on a site!!!! Rather it is about covering cases such as #51823, where a plugin/theme update failure happens between the time the existing plugin/theme is deleted but before the new version successfully installed.

Attachments (8)

51857.diff (513 bytes) - added by afragen 10 months ago.
In copy_dir() check $wp_filesystem->dirlist() and return WP_Error if false
51857.2.diff (1.7 KB) - added by afragen 10 months ago.
only test on first pass
51857.3.diff (1.7 KB) - added by afragen 10 months ago.
pass slug in WP_Error instead of FS_METHOD
51857.4.diff (5.9 KB) - added by afragen 10 months ago.
add rollback
51857.5.diff (7.7 KB) - added by afragen 10 months ago.
also update _copy_dir() per @dd32 rec
51857.6.diff (7.6 KB) - added by afragen 9 months ago.
bubble up error for extract_rollback
Screen Shot 2021-09-23 at 10.55.44 am.png (19.1 KB) - added by peterwilsoncc 4 days ago.
upgrade-warnings.png (1.2 MB) - added by noisysocks 4 days ago.

Download all attachments as: .zip

Change History (144)

#1 @pbiron
10 months ago

  • Owner set to pbiron

#2 @afragen
10 months ago

Just a quick thought. The plugin/theme update transient could have an additional element containing the most recent download package. Then some error during the update process might be able to attempt an update using the alternate download package.

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


10 months ago

This ticket was mentioned in Slack in #core-auto-updates by pbiron. View the logs.


10 months ago

#5 @pbiron
10 months ago

  • Description modified (diff)
  • Summary changed from Add rollback for failed plugin/theme auto-updates to Add rollback for failed plugin/themeupdates

#6 @pbiron
10 months ago

  • Summary changed from Add rollback for failed plugin/themeupdates to Add rollback for failed plugin/theme updates

#7 @afragen
10 months ago

It seems that some additional error reporting is needed. In referencing the core failure data provided by @dd32 in https://wordpress.slack.com/archives/CULBN711P/p1606869594134200 it seems that copy_dir() is a focal point of failure.

In digging a little if $wp_filesystem->dirlist( $from ) is false what happens is the contents of the plugin folder is deleted and the folder remains.

I added a patch to test for this and return a WP_Error. At a minimum it can give us more information. Perhaps it can mitigate this type of failure.

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

@afragen
10 months ago

In copy_dir() check $wp_filesystem->dirlist() and return WP_Error if false

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


10 months ago

@afragen
10 months ago

only test on first pass

#9 @afragen
10 months ago

Only running of the first pass of copy_dir() allows plugins to empty subfolders.

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


10 months ago

#11 @audrasjb
10 months ago

@afragen indeed this would at least prevent the updater to delete plugin without being able to upload the new version.

I tested it by changing writing permissions on my test install and it looks good to me (plugin not deleted). Is it the best way to test this patch?

#12 @afragen
10 months ago

@audrasjb part of the issue is that I don't even know if this is issue. It does seem to solve this particular problem ( $dirlist returning false ).

I test by settings a breakpoint after $dirlist = $wp_filesystem->dirlist( $from ); in copy_dir() and then setting $dirlist = false; in VSCode.

I'm working on sending this failure data to dot org but I have no way to tell if they get it. I'll make another ticket for that.

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

#13 @afragen
10 months ago

See #51928 as possible method of data collection by dot org.

@afragen
10 months ago

pass slug in WP_Error instead of FS_METHOD

#14 @afragen
10 months ago

The Automatic Upgrader Skin doesn't really pass any specific data about plugin/theme so I'm passing the slug in WP_Error as something to help.

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

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


10 months ago

#16 @afragen
10 months ago

  • Keywords has-patch dev-feedback needs-testing added

51857.4.diff added rollback of plugin/theme to patch.

To test copy_dir() must return a WP_Error. This would likely happen if $dirlist = false.

@afragen
10 months ago

add rollback

@afragen
10 months ago

also update _copy_dir() per @dd32 rec

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


10 months ago

#18 @lukecarbis
10 months ago

  • Keywords early added

#19 @poena
10 months ago

During today's scrub,
@afragen suggested that:

Easiest way to test is install patch, set a breakpoint in copy_dir() after $dirlist assignment and set $dirlist = false; This will return WP_Error and trigger the rollback during a plugin/theme update.

Last edited 10 months ago by poena (previous) (diff)

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


10 months ago

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


9 months ago

This ticket was mentioned in Slack in #core-auto-updates by hellofromtonya. View the logs.


9 months ago

#23 follow-up: @TimothyBlynJacobs
9 months ago

I'm not sure if zipping a copy is the right approach here. In my experience, most sites that have issues with a partial update failure ( ie some files are not copied resulting in a crash ) are due to resources timing out during the upgrade process. By taking a zip, this will significantly increase the amount of processing that needs to be done during the upgrade process and thus may increase the likelihood of failed updates. Further, since this is a fatal error issue, copy_dir won't return a result and the rollback code won't execute in that case.

#24 @afragen
9 months ago

ATM this is not quite meant to be all encompassing. Some of the issue is getting a WP_Error out of copy_dir().

It is meant to function if the $dirlist returns false as this is clearly an issue likely relating to the download, extraction , copy process. There's another ticket #51928 to help provide more data for where and when failures happen.

The zipping is done before the any unpacking of the downloaded package and I'm not sure that it should increase the server resources significantly. If this can be demonstrated that would be great. But since the failure seems to be in copy_dir(), zipping and unzipping the current plugin seems like a better solution than copying the data somewhere and then copying it back.

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

#25 @afragen
9 months ago

I did some quick time tests locally and it takes almost no time to create a zip even of something as large as Jetpack.

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


9 months ago

#27 @afragen
9 months ago

It would be helpful to test this on slow servers to see if the zipping process introduces any potential timeout issues.

Please post results here.

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

#28 @audrasjb
9 months ago

I successfully tested the above patch on the worst server I can access to: 1 vCore, RAM 2Gb, 20 Gb SATA SSD. That's one of the cheapest services provided by big hosting companies here in France.
I'll try to find a non-SSD server next week.

But FWIW: rollback worked :-)

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


9 months ago

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


9 months ago

This ticket was mentioned in Slack in #hosting-community by pbiron. View the logs.


9 months ago

This ticket was mentioned in Slack in #hosting-community by crixu. View the logs.


9 months ago

#33 in reply to: ↑ 23 ; follow-up: @mikeschroder
9 months ago

Replying to TimothyBlynJacobs:

I'm not sure if zipping a copy is the right approach here. In my experience, most sites that have issues with a partial update failure ( ie some files are not copied resulting in a crash ) are due to resources timing out during the upgrade process. By taking a zip, this will significantly increase the amount of processing that needs to be done during the upgrade process and thus may increase the likelihood of failed updates. Further, since this is a fatal error issue, copy_dir won't return a result and the rollback code won't execute in that case.

This was brought up in the hosting community channel for discussion and testing, but wanted to leave a quick note that this is also my experience.

#34 follow-up: @afragen
9 months ago

Mike, are you saying you have seen/experienced this problem or that you agree it's a theoretical concern?

This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.


9 months ago

#36 in reply to: ↑ 33 @a2hosting
9 months ago

I'd just like to add in that we've seen what should be very small zip tasks called from PHP fail if there are resource issues and that should be avoided if at all possible in cases such as these.

#37 @afragen
9 months ago

Can we please try to obtain actual data? Let's try to test the actual patch.

I understand theoretical issues, but I also understand that one type of zip task is not the same as another.

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


9 months ago

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


9 months ago

#40 @afragen
9 months ago

From the description.

The current functionality for core rollbacks relies on the availability of a "rollback" package being returned by the Updates API. It is unclear (to me, at this time) whether extending this functionality to plugins/themes will require changes to the API or not.

Obtaining the currently installed version package from dot org is possible but attempting to reinstall it in the same manner as the failure doesn't seem like the practical way forward.

This approach also has the issue of not working with any plugin/theme not hosted on dot org. I think this is a hard block.

#41 @afragen
9 months ago

Lots of discussion between @TimothyBlynJacobs and myself, https://wordpress.slack.com/archives/CULBN711P/p1609968242405800

My impression of the conclusion is that if there is a server resource issue that occurs a result of the zip creation process then a simple page reload brings the site back to it's previous state showing a plugin/theme update being available.

This shouldn't be an issue as long as the frequency of this happening is rare. If this is happens with small plugins it's an issue.

If it happens with large plugins like Mailpoet, Jetpack, Yoast SEO, etc. then perhaps it's not ideal to run a resource intensive site on a resource poor shared environment.

#42 @TimothyBlynJacobs
9 months ago

My impression of the conclusion is that if there is a server resource issue that occurs a result of the zip creation process then a simple page reload brings the site back to it's previous state showing a plugin/theme update being available.

I think this is accurate. But the failure happening _during_ the zip process specifically is not my big worry, but that it consumes resources so that when the file transfer happens, the site no longer has enough resources to complete the upgrade. Which is how you can end up with a fatal erroring site.

@afragen
9 months ago

bubble up error for extract_rollback

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


9 months ago

Moving previous patch(es) to GitHub.

  • Updates copy_dir() and _copy_dir() to include WP_Error return early if the $dirlist is empty.
  • Adds function zip_to_rollback_dir() which creates a zip of the current plugin/theme that is being updated.
  • Adds function extract_rollback() which on a WP_Error returned from copy_dir() will extract the above zip back into the appropriate location.

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

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


9 months ago

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


9 months ago

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


8 months ago

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


8 months ago

#48 in reply to: ↑ 34 @mikeschroder
8 months ago

Replying to afragen:

Mike, are you saying you have seen/experienced this problem or that you agree it's a theoretical concern?

Sorry, I missed this question before. I'm saying I've seen and experienced this problem.
It is not a theoretical concern.

I agree that data would be helpful here, whether from the WordPress project, or hosts.

I'll ask around a bit and see if I can find recent data. It is one of those things that I helped with a lot in tech support, but that was some time ago.

#49 follow-ups: @SergeyBiryukov
8 months ago

Thanks for the patch!

It looks like there are some concerns about performance implications of building (and then potentially unpacking) a zip file with the current plugin version, especially for large plugins. Instead of building a zip file, could we rename the plugin directory to something reasonably unique like plugin-slug.__rollback__, and then delete it on successful update, or rename back on failure? I think that would not require any additional resources. Creating a zip file might save some disk space, but if the space is already low enough for that to make a difference, then it seems like updates on that site would start failing anyway sooner rather than later.

Personally, I would like the code for plugin/theme upgraders to be as close to core as possible. At a glance, it looks like Core_Upgrader::upgrade() has a lot more than Plugin_Upgrader::upgrade() or Theme_Upgrader::upgrade(), so perhaps unifying these methods and documenting the differences would be a good first step. I would prefer to adapt the core upgrader for plugins and themes as appropriate, and not create a completely different implementation.

#50 @afragen
8 months ago

@SergeyBiryukov there’s a fundamental problem with trying to use the same method for a rollback that is the cause of the failure in the first place. In this case copy_dir().

#51 in reply to: ↑ 49 @afragen
8 months ago

Replying to SergeyBiryukov:

Personally, I would like the code for plugin/theme upgraders to be as close to core as possible. At a glance, it looks like Core_Upgrader::upgrade() has a lot more than Plugin_Upgrader::upgrade() or Theme_Upgrader::upgrade(), so perhaps unifying these methods and documenting the differences would be a good first step. I would prefer to adapt the core upgrader for plugins and themes as appropriate, and not create a completely different implementation.

Sounds like this should be another ticket.

#52 in reply to: ↑ 49 ; follow-up: @afragen
8 months ago

Replying to SergeyBiryukov:

Thanks for the patch!

It looks like there are some concerns about performance implications of building (and then potentially unpacking) a zip file with the current plugin version, especially for large plugins. Instead of building a zip file, could we rename the plugin directory to something reasonably unique like plugin-slug.__rollback__, and then delete it on successful update, or rename back on failure? I think that would not require any additional resources. Creating a zip file might save some disk space, but if the space is already low enough for that to make a difference, then it seems like updates on that site would start failing anyway sooner rather than later.

Thanks for the feedback.

Unfortunately rename() can be flaky/unreliable and fallback from rename() involves file copy which is the likely cause of the failure in the first place. Without testing the patch are we looking to solve a problem that doesn't exist?

A server timeout issue is mostly fixed in a page reload and the site isn't likley any worse off than it currently would be.

Isn't this what alpha is for? Unless there's some other concern?

#53 @TimothyBlynJacobs
8 months ago

A server timeout issue is mostly fixed in a page reload and the site isn't likley any worse off than it currently would be.

Calling this out again.

But the failure happening _during_ the zip process specifically is not my big worry, but that it consumes resources so that when the file transfer happens, the site no longer has enough resources to complete the upgrade. Which is how you can end up with a fatal erroring site.

This ticket was mentioned in Slack in #core-auto-updates by hellofromtonya. View the logs.


8 months ago

#55 in reply to: ↑ 52 @dd32
8 months ago

Replying to SergeyBiryukov:

It looks like there are some concerns about performance implications [...] could we rename the plugin directory to something reasonably unique like plugin-slug.__rollback__, and then delete it on successful update, or rename back on failure?

This looks like the best option from a filesystem-io perspective to me.

Replying to afragen:

Unfortunately rename() can be flaky/unreliable and fallback from rename() involves file copy which is the likely cause of the failure in the first place.

I'm not sure what the flaky/unreliable experience is here, but I feel like creating a ZIP backup is solving a problem that doesn't actually exist while adding significant CPU/IO into the process at the same time that's not needed.

Looking at the filesystem handlers move(), it looks like the Direct IO one has some behaviour to use a copy, which _probably_ isn't needed and is probably not useful, [13001] added the usage of rename() into it. A lot has changed in the last decade host IO wise.

Also of note, is that the FTP variants don't check the overwrite functionality.. but that's mostly irrelevant here.

I would probably suggest just cleaning up those move() methods to be consistent and not fallback on a copy() if it's not actually performing a move()/removing the source directory.

IMHO if move() fails, either the delete of the existing directory will also fail, or rollback should just be skipped for those edge-cases.

#56 follow-ups: @dd32
8 months ago

An alternative option would be to flip this around, and not add a rollback functionality into it, but rather install into a temporary directory first, and then remove existing/rename new into place.

(edit: I guess that's kind of how it works now, except the temporary install directory is wp-content/upgrade/somethinghere. It's the copy from there to the plugins directory that sometimes fails, so avoiding that copy by extracting into the plugins directory in the first place and then performing a move() would mean far less IO is needed and as a result could be faster too)

That would result in less downtime of the plugin not existing on the site, and also not delete the plugin from the site in the event of a failure to create the new files.

eg:

  • Plugin update into .hello-dolly.upgrade.1234..
  • Once successful:
    • hello-dolly directory is deleted.
    • move( '.hello-dolly.upgrade.1234', 'hello-dolly' );
    • return true;
  • Upon failure:
    • Delete .hello-dolly.upgrade.1234
    • return false
Last edited 8 months ago by dd32 (previous) (diff)

#57 in reply to: ↑ 56 @dd32
8 months ago

Replying to dd32:

I guess that's kind of how it works now, except the temporary install directory is wp-content/upgrade/somethinghere. It's the copy from there to the plugins directory that sometimes fails

I think this might have originally been a copy, rather than a move, as moving items cross-partition isn't possible. Sometimes each site would have it's own wp-content directory, but the wp-content/plugins or themes folder was actually a symlink to another disk or some such.

I could also be mis-remembering it, and it was purely bad planning and development skills that I wrote it like this :) If I was doing it again, I'd do it how I've outlined in comment 56 above

Last edited 8 months ago by dd32 (previous) (diff)

This ticket was mentioned in Slack in #core-auto-updates by hellofromtonya. View the logs.


8 months ago

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


8 months ago

#60 in reply to: ↑ 56 @hellofromTonya
8 months ago

From core scrub today, @afragen noted that he'll open a new ticket to address the copy_dir changes Dion mentioned above.

For this ticket though, just want to make sure we all understand. Replying to dd32:

An alternative option would be to flip this around, and not add a rollback functionality into it, but rather install into a temporary directory first, and then remove existing/rename new into place.

Hey @dd32 > Is the proposed implementation how it currently works?

If yes, then nothing else needs to be done in this ticket.

Last edited 8 months ago by hellofromTonya (previous) (diff)

#61 @hellofromTonya
8 months ago

  • Keywords close added

Marking as a close candidate. Why? Our understanding is that Dion's proposed solution does not require any additional work in this ticket. And it resolves the server resources concerns.

#62 @SergeyBiryukov
8 months ago

It's my understanding that comment:56 is not exactly how it currently works and still needs to be implemented, so I don't think the ticket should be closed yet.

#63 @hellofromTonya
8 months ago

  • Keywords close removed

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


8 months ago

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


8 months ago

#66 @hellofromTonya
8 months ago

  • Milestone 5.7 deleted
  • Resolution set to maybelater
  • Status changed from assigned to closed

Closing this ticket for now as a maybelater. Why?

  • #52342 is the new direction proposed by Dion.
  • #52381 proposes a new filter to enable moving this ticket into a feature plugin.

#67 @dd32
8 months ago

Just noting, that while this has been closed as maybelater, #52342 is not the suggestion as I mentioned above, and #52381 is a suggestion for a filter to make this ticket possible as a feature plugin, which could've just been kept here.

#68 @SergeyBiryukov
8 months ago

  • Milestone set to 5.8
  • Resolution maybelater deleted
  • Status changed from closed to reopened

Reopening per comment:62 and comment:67.

#69 follow-ups: @SergeyBiryukov
8 months ago

  • Keywords needs-patch added; has-patch dev-feedback needs-testing removed

I think comment:56 is the preferred direction here.

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


8 months ago

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


8 months ago

This ticket was mentioned in Slack in #hosting-community by jadonn. View the logs.


8 months ago

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


7 months ago

#74 @KZeni
7 months ago

Question... does this also accommodate a plugin/theme update that's fully completed (no issue during copying of files or anything like the example stated), but then actually has a bug within the code of the latest release that causes a fatal server error?

I know WP has precautions in place when trying to activate a plugin that'll cause a server error so I'm thinking this might be the case for what's being proposed here (just applying to when updating a plugin with it then rolling back if an error is encountered.)

If that isn't being accommodated, currently, is there anything preventing that situation from being included in this functionality as well?

Last edited 7 months ago by KZeni (previous) (diff)

#75 follow-up: @afragen
7 months ago

The intent of this is currently accommodating update failures, not complete updates where the new version causes a fatal.

#76 in reply to: ↑ 75 @KZeni
7 months ago

Replying to afragen:

The intent of this is currently accommodating update failures, not complete updates where the new version causes a fatal.

I wanted to check since I'm not certain on what it might entail to expand this to include that scenario as well if it didn't already (seems like it could be a natural expansion on the idea.)

For what it's worth, I'm thinking it could be an incredibly helpful thing that could greatly improve ongoing reliability of WordPress sites if it's a reasonable thing to consider for inclusion as part/expansion of this. I mean, unfortunately, there are still plugins out there that have a release buggy release from time to time (ex. I encountered 2 in the last month with them being widely used [one having 100,000+ active installs] plugins where the issue really did live in the plugin release itself), and this is more important now that auto-updates are becoming more of a thing since a buggy release that auto-updates (or even is encountered during a manual update) means everyone encounters the issue rather than the sporadic cases where a copy didn't complete properly due to a server shortcoming/hiccup like this is currently set to address.

#77 @pbiron
7 months ago

@KZeni thanx for the comment.

Personally, I would love to be able to detect whether a plugin/theme update will cause a fatal. However, that would be very hard to do so, since in many cases, such fatals won't occur until the plugin is actually used (e.g., they often don't happen until an admin- or end-user actually does something with the plugin/theme).

Since WP 5.1 was release, there is Fatal Error Protection built into core, where a plugin will be "suspended" if a fatal occurs at run-time and admins will be notified (see the "Fatal Error Protection" section of PHP Site Health Mechanisms in 5.1 for more info).

#78 @KZeni
7 months ago

@pbiron Gotcha. I was just hoping for this since, in my recent case, the Simple History plugin had a syntax error in a recent release where it took down all sites using it for me per the sites being set to auto-update. The other case was the hCaptcha for WordPress plugin where they left in development code where it was trying to require a file that didn't exist.

Both cases were involving a very straightforward & immediate server error simply by way of simply having errors when active without any further action needed (errors in the code itself), and WordPress (running 5.6.2) simply applied the update & had things break site-wide as a result (when it'd be ideal to have it prevent activation or even rollback to the previous version, if encountered.)

Meanwhile, it seems like the kind of situation that the error-checking upon attempting to activate a plugin that'd cause a server error would prevent, but then that protection isn't happening on update & is only on activation (seems like partway towards having plugins not cause server errors where this ticket focuses on issues during installing the new version while it could/should also ideally include the actual plugin being at fault during update just like it does upon the attempted first activation.)

I guess I was hoping that type of error prevention could be inserted during the update process as well somehow while I was uncertain if this particular ticket would take care of that situation (with this ticket's rollback behavior being ideal, if an issue is encountered), it'd be possible to expand this ticket to include that scenario, and/or this ticket is using a different mechanism from that where it'd be a wholly separate request/implementation (with it sounding like the latter, currently & unfortunately, unless these details have helped clarify things.)

Last edited 7 months ago by KZeni (previous) (diff)

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


6 months ago

#80 in reply to: ↑ 69 @SergeyBiryukov
6 months ago

Replying to SergeyBiryukov:

I think comment:56 is the preferred direction here.

Related: #52832

#81 @galbaras
6 months ago

How about treating old versions like trashed items and deleting them later?

This can be easily done by a background job, as well as manual links, e.g. "remove previous version backup" in the plugin list and a "remove previous version backups" action for selected plugins?

Last edited 6 months ago by galbaras (previous) (diff)

#82 follow-up: @SergeyBiryukov
6 months ago

#52832 was marked as a duplicate.

#83 in reply to: ↑ 69 @galbaras
6 months ago

I think comment:56 is the preferred direction here.

This great approach can be combined with avoiding copy_dir() altogether and using move() again.

  1. Unpack the zip file to hello.update.1234
  2. If unsuccessful, return false
  3. If hello.old exists, remove it
  4. Move hello to hello.old
  5. Move hello.update.1234 to hello
  6. Later on (manually and/or scheduled), remove hello.old.

move() should fail a lot less often than copy_dir(), especially since it uses delete & copy anyway when direct renaming fails.

This should greatly simplify the entire upgrade process, make it faster and provide a way to recover from problems by quickly restoring hello.old to whence it came, also by deleting and renaming.

#84 @desrosj
5 months ago

  • Milestone changed from 5.8 to Future Release

As far as I can tell, this effort has been moved to a feature plugin. With 5.8 feature freeze fast approaching (a few short weeks away) and no published merge proposal, I think the reasonable early cutoff has passed. I'm going to punt this to Future Release for now.

If that's an incorrect assessment, please do move back and add the needed context.

This ticket was mentioned in Slack in #core-auto-updates by francina. View the logs.


4 months ago

#86 follow-up: @richards1052
4 months ago

I am a victim of this auto update failure. Two plug-ins, Yoast and Mailpoet, when they update have failed to do so multiple times AND disappeared from my directory.

So when I discover they're missing (long after they've been deleted) I have to manually reinstall them.

I'm surprised a roll back hasn't been implemented for failed plugin upgrades. Also disappointed the auto updater doesn't notify the user when such failures occur. It would permit me to discover the failure, notice the disappearance and reinstall immediately, rather than waiting till I notice the plugin is gone.

#87 in reply to: ↑ 86 @afragen
4 months ago

Replying to richards1052:

I am a victim of this auto update failure. Two plug-ins, Yoast and Mailpoet, when they update have failed to do so multiple times AND disappeared from my directory.

So when I discover they're missing (long after they've been deleted) I have to manually reinstall them.

I'm surprised a roll back hasn't been implemented for failed plugin upgrades. Also disappointed the auto updater doesn't notify the user when such failures occur. It would permit me to discover the failure, notice the disappearance and reinstall immediately, rather than waiting till I notice the plugin is gone.

Have you tried installing the feature plugin Rollback Update Failure??

#88 @richards1052
4 months ago

I probably will. But I have so many plug-ins that I try to restrain myself from adding another.

#89 @pbiron
4 months ago

Richard, please do install and test/use Rollback Update Failure.

It is basically a proposal for how core could do rollbacks and we need real world usage of the proposed solution before we're comfortable incorporating it into core.

#90 in reply to: ↑ 82 ; follow-up: @galbaras
4 months ago

Replying to SergeyBiryukov:

#52832 was marked as a duplicate.

It's related, but not a duplicate, unless my suggestions are integrated into this change. So far, the feature plugin isn't covering them, though, because it's somewhat of an alternative approach to the same thing.

Does anyone disagree that moving is quicker and more flexible than copying?

#91 in reply to: ↑ 90 @SergeyBiryukov
4 months ago

Replying to galbaras:

Replying to SergeyBiryukov:

#52832 was marked as a duplicate.

It's related, but not a duplicate, unless my suggestions are integrated into this change. So far, the feature plugin isn't covering them, though, because it's somewhat of an alternative approach to the same thing.

Does anyone disagree that moving is quicker and more flexible than copying?

I think moving instead of copying is exactly what was proposed in comment:56, and is the preferred direction for this ticket, as previously noted in comment:69.

The feature plugin indeed offers an alternative approach that could be added later, if still necessary after the approach from comment:56 is implemented.

#92 @afragen
4 months ago

comment:56 is not a rollback provision per se but a new method in the course of plugin updating.

#93 @galbaras
4 months ago

Looks like this ticket is getting so long that we just look at the tail end of it. Thank you @SergeyBiryukov for clarifying.

@afragen Given the preferred direction, the only failure scenarios I see are when WP fails to delete the existing directory or move the temporary directory to the live one, likely due to insufficient permissions, which should almost never happen.

An even rarer case is when part of the directory is deleted/moved, and some fails to be deleted/moved.

In the entire deletion fails, WP should return false (with some meaningful message), but at least the old plugin is still there. If only some of the deletion fails, or the move fails (partly or completely), WP may be crippled, but rolling back may suffer the same fate :(

This ticket was mentioned in PR #1492 on WordPress/wordpress-develop by aristath.


6 weeks ago

  • Keywords has-patch added; needs-patch removed

This patch aims to improve the stability of plugin/theme updates, by adding the ability to "rollback" failed updates.

  • When updating a plugin/theme, the old version of the plugin/theme gets moved to a wp-content/upgrade/rollback/plugins/PLUGINNAME or wp-content/upgrade/rollback/themes/THEMENAME folder. The reason we chose to move instead of zip, is because zipping/unzipping are very resources-intensive processes, and would increase the risk on low-end, shared hosts. Moving on the other hand is performed instantly and won't be a bottleneck.
  • If the update fails, then the "backup" we kept in the upgrade/rollback folder gets restored to its original location
  • If the update succeeds, then the "backup" is deleted
  • A new section was added in the site-health screen, to make sure that the rollbacks folder is writable.

To avoid confusion: The "rollback" folder will NOT be used to "roll-back" a plugin to a previous version after an update. This folder will simply contain a transient backup of the previous version of a plugins/themes getting updated, and as soon as the update process finishes, the folder will be empty.

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

#95 @pbiron
6 weeks ago

Following on the core-auto-updates slack discussion of this today, see this comment on the PHP manual page for rename() for some of the pitfalls when the $source and $destination directions are on different filesystems.

I don't think that's much of a problem for the PR, but it does mention there can be cases where rename() returns true even though there were "problems" with the operation. We may want to add some logic to WP_Filesystem_Direct::move() to trap those warnings and deal with them appropriately.

#96 in reply to: ↑ 56 @SergeyBiryukov
6 weeks ago

Replying to dd32:

An alternative option would be to flip this around, and not add a rollback functionality into it, but rather install into a temporary directory first, and then remove existing/rename new into place.

(edit: I guess that's kind of how it works now, except the temporary install directory is wp-content/upgrade/somethinghere. It's the copy from there to the plugins directory that sometimes fails, so avoiding that copy by extracting into the plugins directory in the first place and then performing a move() would mean far less IO is needed and as a result could be faster too)

That would result in less downtime of the plugin not existing on the site, and also not delete the plugin from the site in the event of a failure to create the new files.

@dd32 When you have a moment, could you take a look at PR 1492?

In this implementation, we've explored the approach you suggested, with a notable difference that we chose not to use wp-content/plugins as a temporary directory, as that might lead to multiple copies of the plugin being displayed on the Plugins screen. Even if it's only for a short period of time, I think it might still cause confusion if someone tries to activate a temporary copy of the plugin.

We did replace copying with a call to ::move() though, for better perfomance. Most of the time, that should be a simple rename() call. If the rollback directory is on another filesystem, I guess a rename() would not work, but in that case the ::move() method uses copy()/delete() as a fallback, which I think should work :)

As per the PR description:

  • When updating a plugin/theme, the old version of the plugin/theme gets moved to a wp-content/upgrade/rollback/plugins/PLUGINNAME or wp-content/upgrade/rollback/themes/THEMENAME folder. The reason we chose to move instead of zip, is because zipping/unzipping are very resources-intensive processes, and would increase the risk on low-end, shared hosts. Moving on the other hand is performed instantly and won't be a bottleneck.
  • If the update fails, then the "backup" we kept in the upgrade/rollback folder gets restored to its original location.
  • If the update succeeds, then the "backup" is deleted.
  • A new section was added in the site-health screen, to make sure that the rollbacks folder is writable.

To avoid confusion: The "rollback" folder will NOT be used to "roll-back" a plugin to a previous version after an update. This folder will simply contain a transient backup of the previous version of a plugin/theme getting updated, and as soon as the update process finishes, the folder will be empty.

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


6 weeks ago

#98 follow-up: @mikeschroder
6 weeks ago

I surfaced this at GDDY for more feedback and testing by appropriate folks.

Haven't done deep testing, but did an initial review of PR 1492.

These look like great changes + checks to improve the reliability of the updater.

Thank you!!

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


6 weeks ago

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


5 weeks ago

#101 @Boniu91
5 weeks ago

  • Keywords has-testing-info added

Copying testing instructions from the PR:

  • If the wp-content/upgrade/temp-backup folder is not writable, there should be an error in the site-health screen.
  • If the server has less than 20MB available, there should be an error in the site-health screen that updates may fail. If the server has less than 100MB, it should be a notice that disk space is running low.
  • When updating a plugin, you should be able to see the old plugin in the wp-content/upgrade/temp-backup/plugins/PLUGINNAME folder. The same should apply for themes. Since updates sometimes run fast and we may miss the folder creation during testing, you can add return true; as the 1st line inside the WP_Upgrader->delete_temp_backup() method. This will return early and skip deleting the backup on update-success.
  • When a plugin update fails, the previous version should be restored. To test that, change the version of a plugin to a previous number, run the update, and on fail the previous version (the one where you changed the version number) should still be installed on the site. To simulate an update failure and confirm this works, you can use the snippet below:
add_filter( 'upgrader_install_package_result', function() {
   return new WP_Error( 'simulated_error', 'Simulated Error' );
});
Last edited 5 weeks ago by Boniu91 (previous) (diff)

#102 @Boniu91
5 weeks ago

Test report

Environment

  • WordPress 5.9-alpha-51272-src
  • WordPress-develop environment used

Tested

  1. wp-content/upgrade/temp-backup and wp-content/upgrade/temp-backup/plugins not writeable, the error is displayed in the Site Health
  2. The error and notice is displayed correctly when disk_free_space returns mentioned values.
  3. Worked as described
  4. Worked as described

Question related to 2nd point:
Is this function 100% reliable in all environments?

Question/suggestion related to 3rd point:
If we change permissions of /upgrade/temp-backup/plugins/plugin_name/ it won't be possible to update the plugin plugin_name anymore. It will end up with error:
Could not move old version to the temp-backup directory.
There's no message inside the Site Health section about that. I think we could add kind of check, that would display a notice when /upgrade/temp-backup/plugins/ or /upgrade/temp-backup/themes/ are not empty and error when their content is not writeable.

#103 in reply to: ↑ 98 @mai21
5 weeks ago

Replying to mikeschroder:

I surfaced this at GDDY for more feedback and testing by appropriate folks.

Haven't done deep testing, but did an initial review of PR 1492.

These look like great changes + checks to improve the reliability of the updater.

Thank you!!

Checked the following on https://github.com/WordPress/wordpress-develop/pull/1492:

  • If the wp-content/temp-backup folder is not writable, there should be an error in the site-health screen.(pass)
  • When updating a plugin, you should be able to see the old plugin in the wp-content/upgrade/temp-backup/plugins/PLUGINNAME folder. The same should apply for themes. Since updates sometimes run fast and we may miss the folder creation during testing, you can add return true; as the 1st line inside the WP_Upgrader->delete_temp_backup() method. This will return early and skip deleting the backup on update-success. (pass)
  • If the server has less than 20MB available, there should be an error in the site-health screen that updates may fail. If the server has less than 100MB, it should be noticed that disk space is running low. => Any ideas on how to control server size while using https://localwp.com/? (blocked)
  • For the failed update I tried the following: (Inprogress)

1- add return true;

as the 1st line inside the WP_Upgrader->delete_temp_backup()

2- upgrade theme
3- turn off the internet
4- notice the upgrade folder
5- turn on the network after awhile
===> update failed and the theme was never added to the upgrade folder in the 1st place which I think is fine unless if backup shall be there while processing then will be deleted /or restored in case of success /failure

Env: development version (5.8.1-alpha-51643)

Finally having 2 questions:

1- How to do the needed prequest of server size?
2- What is exactly expected with failure of upgrade regarding backup file? is it ever added to the upgrade folder?

Thanks!

Last edited 5 weeks ago by mai21 (previous) (diff)

#104 @SergeyBiryukov
5 weeks ago

  • Milestone changed from Future Release to 5.9

Moving to the milestone for more visibility and discussion of PR #1492.

#105 @afragen
5 weeks ago

Tested PR #1492.

I was unable to test the server memory conditions.

If both the wp-content/upgrade/temp-backup/plugins and wp-content/upgrade/temp-backup/themes are both un-writeable, only the plugins folder is noted to be un-writeable in Site Health.

If either is un-writeable the correct notice is displayed.

If wp-content/upgrade/temp-backups is un-writeable there is no notice in Site Health.

Updating and restoring a failed backup works great!

#106 @Boniu91
5 weeks ago

@mai21 @afragen I think you can simulate the available space by changing the value of $available_space to expected value in bytes here:
https://github.com/WordPress/wordpress-develop/blob/429ed3a8fe56d470ae5a8f4bdd3e46ce4046cf03/src/wp-admin/includes/class-wp-site-health.php#L1889

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


5 weeks ago

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


4 weeks ago

#109 @aristath
4 weeks ago

Replying to feedback/questions on https://core.trac.wordpress.org/ticket/51857#comment:102

Question related to 2nd point:
Is this function 100% reliable in all environments?

I searched and didn't find any gotchas for that function... I haven't tested it on all environments because I only have access to a limited pool of testing environments.
That being said, if the disk_free_space function can't get a proper result it returns false, in which case the message shown to users in site-health is Could not determine available disk space for updates.

If we change permissions of /upgrade/temp-backup/plugins/plugin_name/ it won't be possible to update the plugin plugin_name anymore. It will end up with error:
Could not move old version to the temp-backup directory.
There's no message inside the Site Health section about that. I think we could add kind of check, that would display a notice when /upgrade/temp-backup/plugins/ or /upgrade/temp-backup/themes/ are not empty and error when their content is not writeable.

The only problem with that is the fact that there is a chance an update is running at the same time... Maybe an automatic-update is triggered on the background, or we have 2 tabs open to do multiple tasks at once and we're updating plugins on another tab, or maybe the site has multiple admins and someone else is running an update at the same time.
I agree that ideally we'd have that message, but at this stage I don't see how we can safely do it without false-positives. It would be possible if there was a function like is_currently_updating() which would allow us to check if there's an update currently in progress, but so far I haven't found a way to do that efficiently.


Replying to questions from https://core.trac.wordpress.org/ticket/51857#comment:103

1- How to do the needed prequest of server size?

That one is tricky... The easiest solution would be to edit this line:

if ( 20 > $available_space_in_mb ) {

and replace the number 20 with something like 200000 (or some other value depending on how much disk space you have available on your machine) to make sure the error gets properly triggered when the defined number is larger than the available disk-space.

2- What is exactly expected with failure of upgrade regarding backup file? is it ever added to the upgrade folder?

If the failure happens while downloading the update, then the backup is never created, it fails before we get to the backup step
If the backup fails for some reason like file/folder permissions, low disk space etc, then the backup gets restored to the plugin's original location. So on failure the backup folder will be empty, but the original plugin will be restored (as opposed to the previous behavior which had the plugin removed or in some cases half-installed)

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


4 weeks ago

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


4 weeks ago

#112 @afragen
4 weeks ago

@SergeyBiryukov

I'm using the updated Rollback Update Failure plugin which now has similar code as this patch.

I discovered that my server has disabled the disk_free_space() function and now anytime I visit the Site Health page I get the following error.

PHP Warning disk_free_space() has been disabled for security reasons in /sites/example.com/files/wp-content/plugins/rollback-update-failure/plugin.php on line 257

which corresponds to the following line.

$available_space = (int) disk_free_space( WP_CONTENT_DIR . '/upgrade/' );

Perhaps a server check to ini_get( 'disabled_functions' ) and setting to false if the function is not available?

I'll see what I can test in the plugin.

Last edited 4 weeks ago by afragen (previous) (diff)

#113 follow-up: @afragen
4 weeks ago

The following change fixes this issue in the plugin.

$available_space = (int) disk_free_space( WP_CONTENT_DIR . '/upgrade/' );

changed to

$disabled = explode( ',', ini_get('disable_functions') );
$available_space = ! in_array( 'disk_free_space', $disabled, true ) ? (int) disk_free_space( WP_CONTENT_DIR . '/upgrade/' ) : false;

Currently testing in https://github.com/WordPress/rollback-update-failure/

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

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


4 weeks ago

#115 @aristath
3 weeks ago

I backported the disk_free_space fix to the patch, so it looks like we're good to go 👍

#116 in reply to: ↑ 113 @SergeyBiryukov
3 weeks ago

Replying to afragen:

$disabled = explode( ',', ini_get('disable_functions') );
$available_space = ! in_array( 'disk_free_space', $disabled, true ) ? (int) disk_free_space( WP_CONTENT_DIR . '/upgrade/' ) : false;

I wanted to see whether we have a precedent of checking the disable_functions setting this way in core.

We do in apache_mod_loaded() for phpinfo() as of [29330] / #26772, but that appears to be the only instance. As noted in comment:3:ticket:42085, this seems a bit redundant and a simple function_exists() check should suffice.

Per the comments in the PHP manual, function_exists() should return false for functions disabled via disable_functions setting.

As also noted in comment:3:ticket:42085, it is possible for the function_exists() check to return true if Suhosin is in use. However, that would be an edge case, as Suhosin was only officially available for PHP 5.4 to 5.6, and its development was discontinued in 2015.

It's also worth noting that as of PHP 8, disabled functions can be redeclared. Unless the function is redeclared, function_exists() still returns false.

It looks like most of the other instances in core where we need to check for a function that might be disabled just use a simple function_exists() check:

  • wp_is_ini_value_changeable() takes this approach for ini_get_all() in [38431] / #37680.
  • Site Health also uses this approach for ini_get() in [48535] / #50038.

Related: #33112, #37680, #37978, #48693, #52226.

So I would suggest just using a function_exists() check here as well, optionally combined with the error suppression operator just in case:

// Sometimes `disk_free_space()` is disabled via the `disable_functions` option for "security purposes".
if ( function_exists( 'disk_free_space' ) ) {
	$available_space = (int) @disk_free_space( WP_CONTENT_DIR . '/upgrade/' );
} else {
	$available_space = false;
}

This works in my testing on PHP 5.6 to 8.1.

#117 @aristath
3 weeks ago

Thank you for the research @SergeyBiryukov!
Pushed the fix to the patch 👍

#118 @afragen
3 weeks ago

I can confirm that the updated patch works. Thanks @SergeyBiryukov and @aristath

Rollback Update Failure plugin updated.

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

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


13 days ago

#120 @SergeyBiryukov
11 days ago

In 51815:

Upgrade/Install: Create a temporary backup of plugins and themes before updating.

This aims to make the update process more reliable and ensures that if a plugin or theme update fails, the previous version can be safely restored.

  • When updating a plugin or theme, the old version is moved to a temporary backup directory:
    • wp-content/upgrade/temp-backup/plugins/[plugin-slug] for plugins
    • wp-content/upgrade/temp-backup/themes/[theme-slug] for themes.
  • If the update fails, then the temporary backup kept in the upgrade/temp-backup directory is restored to its original location.
  • If the update succeeds, the temporary backup is deleted.

To further help troubleshoot plugin and theme updates, two new checks were added to the Site Health screen:

  • A check to make sure that the temp-backup directory is writable.
  • A check that there is enough disk space available to safely perform updates.

To avoid confusion: The temp-backup directory will NOT be used to "roll back" a plugin to a previous version after a completed update. This directory will simply contain a transient backup of the previous version of a plugin or theme being updated, and as soon as the update process finishes, the directory will be empty.

Props aristath, afragen, pbiron, dd32, poena, TimothyBlynJacobs, audrasjb, mikeschroder, a2hosting, hellofromTonya, KZeni, galbaras, richards1052, Boniu91, mai21, francina, SergeyBiryukov.
See #51857.

#121 @SergeyBiryukov
11 days ago

Thanks everyone! Keeping the ticket open for now just in case any follow-up adjustments are needed.

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


11 days ago

#124 @TobiasBg
11 days ago

The falsey value in the function_exists() check https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-site-health.php?rev=51815#L1893 should probably be 0 instead of false.
Otherwise, we potentially have a type problem in the next line (boolean divided by a number) (ok, that's still 0), and we are potentially passing false to size_format() which is documented to accept ints or strings only.

#125 follow-up: @desrosj
11 days ago

Just wanted to report that this has been running without issue on my site since committing.

One thing I did notice is that the temp-backup directory remains in upgrade after an update completes. When a Core auto-update happens, the upgrades directory is completely emptied. Should the plugin/theme process just clean up the temp-backup directory?

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


10 days ago

#127 in reply to: ↑ 125 @aristath
5 days ago

Replying to desrosj:

Just wanted to report that this has been running without issue on my site since committing.

That's great to hear, thank you for verifying!

One thing I did notice is that the temp-backup directory remains in upgrade after an update completes. When a Core auto-update happens, the upgrades directory is completely emptied. Should the plugin/theme process just clean up the temp-backup directory?

The PR added an exception for the temp-backup folder so when an update runs, that folder does not get deleted (see https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/class-wp-upgrader.php?rev=51815#L337). This was done to ensure that in case multiple updates run at the same time, the backups won't get deleted before the update finishes running. This also decreases the effect of #53705: The downloaded/to-be-installed files get deleted, but the backup remains and is restored on failure so there are no errors.

Last edited 5 days ago by SergeyBiryukov (previous) (diff)

#128 @peterwilsoncc
4 days ago

  • Keywords needs-patch needs-unit-tests added; has-patch removed

I'm seeing some bugs following [51815] on my local development environment:

  • Dev environment: Chassis
  • Config file for Chassis at https://gist.github.com/peterwilsoncc/53c197cb5ef3e3dd9d649fb1d2069d0f
  • Replaced default wp directory with a checkout of the build server
  • Runs in sub-directory mode (ie, site url: wp-build.local/wp, home url: wp-build.local, content: wp-build.local/content)
  • Plugins installed pre test: User Switching (latest), Query Monitor (latest), Gutenberg (old, 11.1.0)

When updating plugins that contain sub-directories (such as Gutenberg), the top level of the plugin extracts but none of the sub-directories contain files. See See Screen Shot 2021-09-23 at 10.55.44 am.png.

I've tested with various workflows to update the plugins, most cause a fatal error after the update fails. The results of the various workflows are:

Plugin screen

  • view version details, click install: Fails, stays active and triggers critical error
  • update now link: Fails, stays active and triggers critical error
  • checkbox, select update, apply: Fails, stays active and triggers critical error

Update screen:

  • checkbox, click update plugins: fails, stays active and triggers critical error
  • view version details, click install: fails, deactivates.

I've asked some other contributors to test on their local environments but the result is consistent for me with multiple plugins that contain a sub-directory structure using Chassis.

There are some plugin tests with hello dolly but it would be good if something a little more complex could be tested against. It's probably a poor choice to include all of Gutenberg in the tests but something with a slightly more complex structure than dolly would be helpful.

This ticket was mentioned in Slack in #core-auto-updates by peterwilsoncc. View the logs.


4 days ago

#130 @galbaras
4 days ago

@peterwilsoncc Any errors logged during the update? Is it possible debug/trace to catch where things go wrong?

It's interesting that the "view version details" results are different for the two screens. Shouldn't be doing exactly the same thing?

Can you test with a different hierarchical plugin, just to rule out some Gutenberg-specific issue?

Last edited 4 days ago by galbaras (previous) (diff)

#131 @afragen
4 days ago

@peterwilsoncc I tested locally using Akismet. I updated from plugins page using the view details iframe and using the shiny updates. I also tested from updates page and and tested from view details iframe and using checkbox.

Everything worked.

I wonder if something in your dev environment is causing an issue. Have you tried in the WP docker test environment?

#132 @peterwilsoncc
4 days ago

@galbaras No errors or notices during download, only the fatal error when the update procedure attempts to reactivate the plugin and the file doesn't exist.

@afragen Akismet worked for me too using Chassis. It's much smaller than Gutenberg so maybe something is timing out for larger plugins. I'll test with the following:

  • Small plugin but several sub-directories and nested sub-directories
  • Large plugin with a flat file structure

I just need to find some examples of each.

#133 @peterwilsoncc
4 days ago

@afragen As mentioned via Slack, a correction on the Akismet update.

The sub-directories remain empty but Akismet uses include rather than require so doesn't fatal, it only throws warnings.

#134 follow-up: @noisysocks
4 days ago

I updated Gutenberg via the Dashboard → Updates → View version details modal and, while the upgrade worked, there were some warnings. See upgrade-warnings.png. I'm running a semi-recent trunk version of WordPress in Apache on macOS.

#135 @aristath
4 days ago

For reference, the issue reported above now has a new ticket on trac: #54166

Last edited 4 days ago by SergeyBiryukov (previous) (diff)

#136 in reply to: ↑ 134 @SergeyBiryukov
4 days ago

Replying to noisysocks:

I updated Gutenberg via the Dashboard → Updates → View version details modal and, while the upgrade worked, there were some warnings. See upgrade-warnings.png. I'm running a semi-recent trunk version of WordPress in Apache on macOS.

Just noting that I'm also getting these warnings with the current Gutenberg version, but they appear to be unrelated to this ticket. I think they are being tracked in #53565.

Note: See TracTickets for help on using tickets.