WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 5 months ago

#14028 new enhancement

Maintenance mode nag persists after successful upgrade

Reported by: mrmist Owned by:
Milestone: Future Release Priority: normal
Severity: minor Version: 3.0
Component: Upgrade/Install Keywords:
Focuses: Cc:

Description

A number of forum users are reporting that the maintenance nag

"An automated WordPress update has failed to complete" etc.. persists after failed auto-updates even if they then successfully perform manual updates.

Looks like something is leaving $upgrading set.

Attachments (3)

14028.diff (816 bytes) - added by Denis-de-Bernardy 4 years ago.
14028.2.diff (1.4 KB) - added by SergeyBiryukov 5 months ago.
14028.3.diff (808 bytes) - added by SergeyBiryukov 5 months ago.

Download all attachments as: .zip

Change History (21)

comment:1 nacin4 years ago

This would mean we're not removing the .maintenance file after a manual upgrade, which... we can't exactly detect.

comment:2 Denis-de-Bernardy4 years ago

  • Milestone changed from Unassigned to 3.0.1

No no, it means the update_core (site) transient didn't get reset as it should.

comment:3 Denis-de-Bernardy4 years ago

  • Milestone changed from 3.0.1 to Future Release
  • Type changed from defect (bug) to enhancement

Woops, nevermind. You're absolutely correct.

Marking this as an enhancement. We should change the message to something that prompts to delete the .maintenance file after a successful manual upgrade.

Denis-de-Bernardy4 years ago

comment:4 Denis-de-Bernardy4 years ago

  • Keywords has-patch added

comment:5 nacin4 years ago

  • Keywords needs-codex added

On one hand, I would rather there just be a "Dismiss" option (which deletes the file) instead of forcing them to go into FTP and delete the file.

I suppose if they're updating manually, they're already logged in via FTP, in which case part of those instructions should be to remove .maintenance if it exists. We can add that to the Codex as a quick fix.

comment:6 follow-up: mrmist4 years ago

If removing .maintenance will sort it, then I can add that to codex docs etc.

I see I can't set milestones here any more, trac permissions have changed?

comment:7 in reply to: ↑ 6 nacin4 years ago

Replying to mrmist:

If removing .maintenance will sort it, then I can add that to codex docs etc.

It will indeed.

I see I can't set milestones here any more, trac permissions have changed?

Yes.

comment:8 mrmist4 years ago

  • Keywords needs-codex removed

Fair enough. I've altered the Codex manual update instructions to reference the .maintenance file.

comment:9 dd323 years ago

  • Component changed from General to Upgrade/Install

comment:10 dd327 months ago

#17999 was marked as a duplicate.

comment:11 follow-ups: Viper007Bond5 months ago

My WordPress 3.7 install just tired to upgrade itself to 3.7.1 but failed as I don't have my files writable.

Rather than making them writable I just went back to managing it via SVN. I deleted all my Core files, did a checkout, and now I'm running 3.7.1.

However the message persists at the top of my admin area and there is no .maintenance file in my WordPress directory.

With auto-updates now in Core, we should give this ticket some more love.

comment:12 in reply to: ↑ 11 SergeyBiryukov5 months ago

  • Milestone changed from Future Release to 3.8

Replying to Viper007Bond:

However the message persists at the top of my admin area and there is no .maintenance file in my WordPress directory.

Introduced in [25872]. Before that changeset, the message was only displayed if there was a .maintenance file left.

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

comment:13 Viper007Bond5 months ago

That changeset (mostly) makes sense, but the problem now is that there is no way I can get rid of the message without going into the database.

WordPress expects me to run the upgrade again in order to remove it but I can't -- that's the whole reason it failed in the place.

I suspect my case is not unique.

SergeyBiryukov5 months ago

comment:14 SergeyBiryukov5 months ago

14028.2.diff is an attempt to use Core_Upgrader::check_files() to verify that all the files were successfully copied in case of a manual upgrade.

  • When I tested this on a localized install, readme.html checksum turned out to be different than expected in get_core_checksums(), since the file is translated. I guess it can be excluded from the check.
  • On my install, I also had to exclude the two files changed by the patch, since they obviously won't have the right checksums.
  • Noticed that $comparison doesn't actually affect anything. See #25887.
Last edited 5 months ago by SergeyBiryukov (previous) (diff)

comment:15 follow-up: dd325 months ago

When I tested this on a localized install, readme.html checksum turned out to be different than expected in get_core_checksums(), since the file is translated. I guess it can be excluded from the check.

It's supposed to respect get_locale() (get_core_checksums() will return the checksums on a per-locale basis)

SergeyBiryukov5 months ago

comment:16 in reply to: ↑ 15 SergeyBiryukov5 months ago

Replying to dd32:

It's supposed to respect get_locale() (get_core_checksums() will return the checksums on a per-locale basis)

Indeed, I had some other changes in my readme.html file. 14028.3.diff excludes that part.

comment:17 in reply to: ↑ 11 dd325 months ago

  • Keywords has-patch removed

Replying to Viper007Bond:

My WordPress 3.7 install just tired to upgrade itself to 3.7.1 but failed as I don't have my files writable.

Rather than making them writable I just went back to managing it via SVN. I deleted all my Core files, did a checkout, and now I'm running 3.7.1.

It looks like this was caused by this point by Sergey:

  • Noticed that $comparison doesn't actually affect anything. See #25887.

That ultimately meant any early-failures (ie. prior to copying files, as your error would've been) the nag would get shown when it shouldn't have.

Looking at 14028.3.diff further, we can't go with that, it'll cause a checksum API request every pageload, and md5_file() every file every page load.

With #25887 fixed, the instances of getting the non-dismissible message would be limited to:

  • .maintanence file exists, or
  • Background Update attempted, failed DURING copying files, and, user updates via SVN/FTP instead. (we do a files_are_writable() check before we copy)

comment:18 nacin5 months ago

  • Milestone changed from 3.8 to Future Release
  • Severity changed from normal to minor

With #25887 handled, this is very minor at this point.

Note: See TracTickets for help on using tickets.