Opened 3 years ago
Last modified 3 years ago
#54546 new defect (bug)
Fatal error receive while updating WP 5.8.2 to WP 5.9.
Reported by: |
|
Owned by: | |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 5.9 |
Component: | Upgrade/Install | Keywords: | needs-testing has-patch dev-feedback |
Focuses: | Cc: |
Description (last modified by )
After updating to 5.9-beta1 via the Beta Tester plugin, I got the below error
Fatal error: Uncaught Error: Class 'Requests_Exception' not found in C:\Users\User\Local Sites\first-localhost\app\public\wp-includes\Requests\Transport\cURL.php on line 443
I also updated a few other sites on my local machine and I'm getting that error on the other sites.
Full information:
Unpacking the update... Verifying the unpacked files... Preparing to install the latest version... Enabling Maintenance mode... Copying the required files... Disabling Maintenance mode... Upgrading database... Fatal error: Uncaught Error: Class 'Requests_Exception' not found in C:\Users\User\Local Sites\first-localhost\app\public\wp-includes\Requests\Transport\cURL.php on line 443 Error: Class 'Requests_Exception' not found in C:\Users\User\Local Sites\first-localhost\app\public\wp-includes\Requests\Transport\cURL.php on line 443
Call Stack:
# | Function | Location |
---|---|---|
1 | {main}{} | ..\update-core.php:0 |
2 | do_core_upgrade() | ..\update-core.php:1106 |
3 | Core_Upgrader->upgrade() | ..\update-core.php:887 |
4 | update_core() | ..\class-core-upgrader.php:172 |
5 | wp_remote_post() | ..\update-core.php:1409 |
6 | WP_Http->post() | ..\http.php:179 |
7 | WP_Http->request() | ..\http.php:608 |
8 | Requests->request() | ..\class-http.php:394 |
9 | Requests_Transport_cURL->request() | ..\class-requests.php:381 |
10 | Requests_Transport_cURL->process_response() | ..\cURL.php:179 |
Change History (56)
This ticket was mentioned in Slack in #core-php by hellofromtonya. View the logs.
3 years ago
#2
@
3 years ago
- Component changed from General to Upgrade/Install
- Milestone changed from Awaiting Review to 5.9
- Version set to trunk
#4
follow-ups:
↓ 10
↓ 18
@
3 years ago
- Keywords reporter-feedback added
Requests library:
- Requests pre-2.0.0:
- the "cURL" class was called
Requests_Transport_cURL
and was in a file namedwp-includes/Requests/Transport/cURL.php
. - Line 443 in
cURL.php
file throws a newRequests_Exception()
error
- the "cURL" class was called
- Requests 2.0.0:
- the "cURL" class was renamed to
WpOrg\Requests\Transport\Curl
and its file renamed towp-includes/Requests/Transport/Curl.php
. Requests_Exception
class was removed in this version- The cURL error was changed to throw a new
Exception
- the "cURL" class was renamed to
[52244] #54504 updated the Requests library to 2.0.0.
Look at the fatal error:
Fatal error: Uncaught Error: Class 'Requests_Exception' not found in C:\Users\User\Local Sites\first-localhost\app\public\wp-includes\Requests\Transport\cURL.php on line 443
It's indicating the error happened in cURL.php
file on line 443. That file was renamed in Requests 2.0.0 and line 443 is from the older version, not the new version (i.e. in 5.9.0 Beta 1).
Look at the 2nd error:
Error: Class 'Requests_Exception' not found in C:\Users\User\Local Sites\first-localhost\app\public\wp-includes\Requests\Transport\cURL.php on line 443
That class was removed in newer version (i.e. in 5.9.0 Beta 1).
At Upgrading database...
the old files should have been removed and new files installed. Did this happen?
@apeksha10 if you're able to, can you check if the cURL file is named cURL.php
or Curl.php
? Look here: C:\Users\User\Local Sites\first-localhost\app\public\wp-includes\Requests\Transport\Curl.php
. Is it named cURL.php
or Curl.php
?
#5
@
3 years ago
@costdev reported on Slack the same problem. Copying the message here.
- After clicking "Update", it takes ages (to be exact) for the "Update WordPress" page to load and show the steps for the update process.
- As soon as it finally shows the page, my
debug.log
appeared with:
PHP Fatal error: Uncaught Error: Class 'Requests_Exception' not found in <path>\wp-includes\Requests\Transport\cURL.php:422 Stack trace: #0 <path>\wp-includes\Requests\Transport\cURL.php(177): Requests_Transport_cURL->process_response('', Array) #1 <path>\wp-includes\class-requests.php(379): Requests_Transport_cURL->request('http://...', Array, NULL, Array) #2 <path>\wp-includes\class-http.php(370): Requests::request('http://...', Array, NULL, 'POST', Array) #3 <path>\wp-includes\class-http.php(589): WP_Http->request('http://...', Array) #4 <path>\wp-includes\http.php(187): WP_Http->post('http://...', Array) #5 <path>\wp-admin\includes\update-core.php(1409): wp_remote_post('http in <path>\wp-includes\Requests\Transport\cURL.php on line 422
In my case, I was upgrading from 5.0.14 > 5.9.0 Beta 1.
@costdev can you check if the cURL file is named cURL.php
or Curl.php
(see my comments above?
#7
@
3 years ago
A similar report is found in Support Forum https://wordpress.org/support/topic/fatal-error-during-update-to-5-9-beta-1/. Copying the information here.
---
Got a fatal error during update from 5.8.2 to 5.9 beta 1 via Beta Tester plugin. Here it is:
Fatal error: Uncaught Error: Class "Requests_Exception" not found in \wp-includes\Requests\Transport\cURL.php:443 Stack Trace 1. Requests_Transport_cURL->process_response('', Array) \wp-includes\Requests\Transport\cURL.php:179 2. Requests_Transport_cURL->request('https://test...', Array, NULL, Array) \wp-includes\class-requests.php:381 3. Requests::request('https://test...', Array, NULL, 'POST', Array) \wp-includes\class-http.php:394 4. WP_Http->request('https://test...', Array) \wp-includes\class-http.php:608 5. WP_Http->post('https://test...', Array) \wp-includes\http.php:179 6. wp_remote_post('https://test...', Array) \wp-admin\includes\update-core.php:1409 7. update_core('C:/Programs/lar...', 'C:/Programs/lar...') \wp-admin\includes\class-core-upgrader.php:172 8. Core_Upgrader->upgrade(Object(stdClass), Array) \wp-admin\update-core.php:877 9. do_core_upgrade(false) \wp-admin\update-core.php:1106 10. {main} thrown in \wp-includes\Requests\Transport\cURL.php on line 443
For the local environment I am using Laragon and Windows 10. And I can see Exception.php in Requests folder.
---
#8
in reply to:
↑ 6
@
3 years ago
Replying to costdev:
The cURL file is named
cURL.php
.
Thanks @costdev.
Please open that file. What's the class name and is it namespaced WpOrg\Requests\Transport
?
Also, open the Requests\Exception.php
file. What's the class name and is it namespaced WpOrg\Requests\
?
#9
@
3 years ago
wp-includes/Requests/Transport/cURL.php
:
<?php namespace WpOrg\Requests\Transport; ... final class Curl implements Transport {
wp-includes/Requests/Exception.php
:
<?php namespace WpOrg\Requests; ... class Exception extends PHPException {
#10
in reply to:
↑ 4
;
follow-up:
↓ 22
@
3 years ago
Replying to hellofromTonya:
Requests library:
- Requests pre-2.0.0:
- the "cURL" class was called
Requests_Transport_cURL
and was in a file namedwp-includes/Requests/Transport/cURL.php
.- Line 443 in
cURL.php
file throws a newRequests_Exception()
error- Requests 2.0.0:
- the "cURL" class was renamed to
WpOrg\Requests\Transport\Curl
and its file renamed towp-includes/Requests/Transport/Curl.php
.Requests_Exception
class was removed in this version- The cURL error was changed to throw a new
Exception
Just making sure the base information is correct:
Requests_Exception
was not removed. It became namespaced, just like every other class. The file was not renamed/moved as it was already called Exception.php
.
So the Exception
you see being thrown in the Curl
class is still the same exception, just namespaced now (and imported via a use WpOrg\Requests\Exception
import statement).
As for the issues you are seeing, yes, I agree, these are likely to do with file name casing and that not being taken into account properly during the upgrade.
In the Requests repo itself, we moved all files. The "old" PSR-0 classes lived in a library
directory, the new PSR-4 based classes live in a src
directory.
That prevents these kind of issues from happening as the files are seen as "new" files.
Unfortunately, WP Core had nearly* all the files in the wp-includes/requests
directory, not in wp-includes/requests/libary
, so that didn't really allow for making the same directory move.
Would moving the files to wp-includes/requests/src
be a potential solution ? (to make sure they are seen as new files, not updated files)
* One exception to this: the original entry point class Requests
was in WP Core in the wp-includes/class-requests.php
file (in Requests that class was in library/Requests.php
).
In Requests 2.0 a stripped down version of the "old" library/Requests.php
file still remains in place as part of the BC-layer.
The actual functional code, however, lives in the "new" src/Requests.php
file.
For WP Core, the wp-includes/class-requests.php
file now contains the stripped down version of the library/Requests.php
file and along the same lines as in Requests itself, the real functional code now lives in the wp-includes/requests/Requests.php
file.
#11
follow-up:
↓ 12
@
3 years ago
Running more tests with @costdev. Let me share findings so far.
My Machine (macOS)
On my machine:
- localhost: Local
- OS: macOS
- Plugins: Core Rollback and Beta Tester - both are activated
- Browser: Edge
Upgrading from 5.8.2 to 5.9.0 Beta 1 via the Beta Tester
- No errors and nothing in the logs
- Upgrade is successful
- Requests library is updated with the newest 2.0.0 code
- But the renamed files are not renamed:
wp-includes/Requests/Iri.php
is not renamed towp-includes/Requests/Iri.php
wp-includes/Requests/Transport/cURL.php
is not renamed towp-includes/Requests/Transport/Curl.php
- Site Health's Filesystem Permissions show:
The main WordPress directory Writable The wp-content directory Writable The uploads directory Writable The plugins directory Writable The themes directory Writable
Downgrading from 5.9.0 Beta 1 to 5.8.2 via the Beta Tester:
- The ugly orange PHP error interface flashes
- Upgrade is successful
- Requests library is restored to the older version
- Filenames mentioned above remain, which matches the 5.8 version
- But
debug.log
has the following warning:
[01-Dec-2021 17:21:49 UTC] PHP Warning: rename(/var/tmp/wordpress-5.8.2-no-content-gTRqga.tmp,wordpress-5.8.2-no-content.zip): Operation not permitted in ../Local/test/app/public/wp-admin/includes/file.php on line 1201 [01-Dec-2021 17:21:49 UTC] PHP Stack trace: [01-Dec-2021 17:21:49 UTC] PHP 1. {main}() ../Local/test/app/public/wp-admin/update-core.php:0 [01-Dec-2021 17:21:49 UTC] PHP 2. do_core_upgrade() ../Local/test/app/public/wp-admin/update-core.php:1108 [01-Dec-2021 17:21:49 UTC] PHP 3. Core_Upgrader->upgrade() ../Local/test/app/public/wp-admin/update-core.php:879 [01-Dec-2021 17:21:49 UTC] PHP 4. Core_Upgrader->download_package() ../Local/test/app/public/wp-admin/includes/class-core-upgrader.php:124 [01-Dec-2021 17:21:49 UTC] PHP 5. download_url() ../Local/test/app/public/wp-admin/includes/class-wp-upgrader.php:309 [01-Dec-2021 17:21:49 UTC] PHP 6. rename() ../Local/test/app/public/wp-admin/includes/file.php:1201
- Site Health's Filesystem Permissions show:
The main WordPress directory Writable The wp-content directory Writable The uploads directory Writable The plugins directory Writable The themes directory Writable
So the filesystem is the same as Windows, i.e. meaning the file name case does not seem to be an issue. Files are loading properly.
Colin's machine (Windows)
On my machine:
- localhost: Local
- OS: Windows
- Plugins: Beta Tester
- Browser: Edge
Upgrading from 5.0.14 to 5.9.0 Beta 1
- Fatal error:
PHP Fatal error: Uncaught Error: Class 'Requests_Exception' not found in \wp-includes\Requests\Transport\cURL.php:422 Stack trace: #0 \wp-includes\Requests\Transport\cURL.php(177): Requests_Transport_cURL->process_response('', Array) #1 \wp-includes\class-requests.php(379): Requests_Transport_cURL->request('http://...', Array, NULL, Array) #2 \wp-includes\class-http.php(370): Requests::request('http://...', Array, NULL, 'POST', Array) #3 \wp-includes\class-http.php(589): WP_Http->request('http://...', Array) #4 \wp-includes\http.php(187): WP_Http->post('http://...', Array) #5 \wp-admin\includes\update-core.php(1409): wp_remote_post('http://...', Arra in \wp-includes\Requests\Transport\cURL.php on line 422
- Files are upgraded with the new code.
- Like my machine, the
IRI.php
andcURL.php
files were not renamed.
Downgrading from 5.9.0 Beta 1 to 5.8.2
- Error:
Uncaught Error: Class 'WpOrg\Requests\Exception' not found in \wp-includes\Requests\Transport\cURL.php:492 Stack trace: #0 \wp-includes\Requests\Transport\cURL.php(218): WpOrg\Requests\Transport\Curl->process_response('', Array) #1 \wp-includes\Requests\Requests.php(468): WpOrg\Requests\Transport\Curl->request('http://...', Array, '', Array) #2 \wp-includes\class-wp-http.php(394): WpOrg\Requests\Requests::request('http://...', Array, NULL, 'POST', Array) #3 \wp-includes\class-wp-http.php(614): WP_Http->request('http://...', Array) #4 \wp-includes\http.php(179): WP_Http->post('http://...', Array) #5 \wp-admin\includes\update-core.php(1408): w in \wp-includes\Requests\Transport\cURL.php on line 492
- Files are upgraded with the 5.8.2 code
- File names remain (as they should)
What's interesting though is look at the code that's being executed in the stack trace on his machine:
👉 It's the "from" code not the "to" code.
The code in memory is the "from" code whereas the filesystem has been upgraded with the "to" code. As the exception file is not loaded into memory before upgrading the files on the filesystem, when the cURL error happens, it attempts to throw an exception for a class that does not exist.
Q: What cURL error is happening on Windows machines?
Colin tracked it to:
cURL error 28: Operation timed out after 60006 milliseconds with 0 bytes received
Q: Is the "from" code running instead of the "to" code for the Upgrade process?
WordPress 5.9 is incompatible with Gutenberg plugin less than 11.9. So if Gutenberg plugin version 11.8 is installed, it should get deactivated during the upgrade process to WordPress 5.9.
Steps to test:
- Go to the Gutenberg plugin advanced page and download 11.8.3.
- Install and activate it.
- Start with WordPress 5.7.4 (use the Core Rollback plugin)
- Then upgrade via Beta Tester to WordPress 5.9.0 beta 1.
- Go to Plugins and check that Gutenberg plugin was deactivated.
Test Results:
- My machine: Yes, the older Gutenberg plugin gets deactivated. That code is in 5.9.0 not in 5.8.2 or 5.7.4.
- Colin's machine: same results
So yes, the 5.9 upgrader code is executing.
Q: Why then is the "from" version code running instead of the "to" version code?
Is the "from" version code being loaded into memory before the upgrade process starts?
Looking at above at Colin's results, it seems to be the case for the HTTP and Requests code. Since the "from" version code is running, when the cURL timeout happens, it attempts to throw an exception that hasn't yet been loaded into memory. But the filesystem is now different and doesn't have that class. Fatal error.
Is this only on Windows? Or is this true for all systems?
#12
in reply to:
↑ 11
;
follow-ups:
↓ 15
↓ 16
@
3 years ago
Replying to hellofromTonya:
Q: Is the "from" code running instead of the "to" code for the Upgrade process?
WordPress 5.9 is incompatible with Gutenberg plugin less than 11.9. So if Gutenberg plugin version 11.8 is installed, it should get deactivated during the upgrade process to WordPress 5.9.
Hmm, unless I'm missing something, I don't think the Gutenberg plugin is relevant here, but rather the fact that the updater itself uses an older version of the Requests library during the core update process.
Replying to jrf:
Would moving the files to
wp-includes/requests/src
be a potential solution ? (to make sure they are seen as new files, not updated files)
Good question, let's try that :)
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
3 years ago
#14
@
3 years ago
Hmm, unless I'm missing something, I don't think the Gutenberg plugin is relevant here, but rather the fact that the updater itself uses an older version of the Requests library during the core update process.
This was a test to find out whether the "from" code or the "to" code was being used during the upgrade process.
Going from 5.7.4 to 5.9.0 Beta 1:
- If it was the "from" code, Gutenberg would stay activated as 5.7.4 supports Gutenberg < 11.9.
- If it was the "to" code, Gutenberg would be deactivated as WP 5.9+ doesn't support Gutenberg < 11.9.
When Gutenberg was deactivated, this told us that it's the "to" code that is used during the upgrade process.
#15
in reply to:
↑ 12
;
follow-up:
↓ 17
@
3 years ago
Replying to SergeyBiryukov:
Hmm, unless I'm missing something, I don't think the Gutenberg plugin is relevant here, but rather the fact that the updater itself uses an older version of the Requests library during the core update process.
Yes that is what is expected to happen. The point of the test was to validate that the "to" (i.e. 5.9.0) wp-admin/includes/update-core.php
was loaded into memory and being executed. The fact that the older version of Gutenberg was deactivated proves that 5.9's version of update-core.php
was the file being executed.
Why did we do that check? Because the older versions of HTTP and Requests were the ones being executed in memory. We needed to figure out if the Updater was the older (i.e. the "from" version) or the new 5.9.0 version.
#16
in reply to:
↑ 12
@
3 years ago
Thanks @hellofromTonya and @costdev for your investigation efforts so far!
How I read it, there are two (three) separate, distinct issues in play (previously undiscovered bugs), which just happen to both be triggered during this update:
1. The upgrade process does not isolate itself enough from the files being updated.
What I mean by that is, that any essential WP Core files (potentially) being used during the upgrade, should be loaded into memory (require
-d) prior to the update starting.
This is clearly not happening as otherwise, the Fatal for the Requests_Exception
would not be thrown, which means that you could end up with a mix of "old" and "new" files being used during an upgrade, which can cause race conditions.
For the majority of code in Core, this will not happen, as files are hard require
-d and not being lazily autoloaded. But for external dependencies, like Requests, where an autoloader is used, this is a realistic risk and should be mitigated.
While the "new" upgrader code should always be used, the rest of the files should be consistently from the "old" codebase.
To fix this, some investigation would be needed to figure out exactly which files/parts of Core are used during the upgrade.
Once this is known, any files on that list which depend on an autoloader, should be hard-required.
A DirectoryIterator
with require_once
statements for those parts of Core, could probably solve this in a future-proof way.
2. The upgrade process is not designed to handle file renames involving only the file/directory casing.
Whether or not this causes problems will highly depend on the OS and whether or not the file-system is case-sensitive.
To solve this, it could be considered to unlink
old files and then move the new files in place, instead of plain moving the files (replacing the original files).
I'm by no means saying that these are the solutions to use. Just pointing out some solution directions to explore.
Once these two issues are solved, we can start to try and figure out why the Curl timeout is happening.... (problem three)
#17
in reply to:
↑ 15
;
follow-up:
↓ 20
@
3 years ago
Replying to hellofromTonya:
Yes that is what is expected to happen. The point of the test was to validate that the "to" (i.e. 5.9.0)
wp-admin/includes/update-core.php
was loaded into memory and being executed. The fact that the older version of Gutenberg was deactivated proves that 5.9's version ofupdate-core.php
was the file being executed.
Why did we do that check? Because the older versions of HTTP and Requests were the ones being executed in memory. We needed to figure out if the Updater was the older (i.e. the "from" version) or the new 5.9.0 version.
Ah, that's what I was missing. Thanks for the clarification!
I can confirm that wp-admin/includes/update-core.php
is indeed specifically copied from the new package, however the rest of the code, including the Requests library, appears to be loaded from the current (older) install.
#18
in reply to:
↑ 4
;
follow-up:
↓ 19
@
3 years ago
Replying to hellofromTonya:
Requests library:
- Requests pre-2.0.0:
- the "cURL" class was called
Requests_Transport_cURL
and was in a file namedwp-includes/Requests/Transport/cURL.php
.- Line 443 in
cURL.php
file throws a newRequests_Exception()
error- Requests 2.0.0:
- the "cURL" class was renamed to
WpOrg\Requests\Transport\Curl
and its file renamed towp-includes/Requests/Transport/Curl.php
.Requests_Exception
class was removed in this version- The cURL error was changed to throw a new
Exception
[52244] #54504 updated the Requests library to 2.0.0.
Look at the fatal error:
Fatal error: Uncaught Error: Class 'Requests_Exception' not found in C:\Users\User\Local Sites\first-localhost\app\public\wp-includes\Requests\Transport\cURL.php on line 443It's indicating the error happened in
cURL.php
file on line 443. That file was renamed in Requests 2.0.0 and line 443 is from the older version, not the new version (i.e. in 5.9.0 Beta 1).
Look at the 2nd error:
Error: Class 'Requests_Exception' not found in C:\Users\User\Local Sites\first-localhost\app\public\wp-includes\Requests\Transport\cURL.php on line 443That class was removed in newer version (i.e. in 5.9.0 Beta 1).
At
Upgrading database...
the old files should have been removed and new files installed. Did this happen?
@apeksha10 if you're able to, can you check if the cURL file is named
cURL.php
orCurl.php
? Look here:C:\Users\User\Local Sites\first-localhost\app\public\wp-includes\Requests\Transport\Curl.php
. Is it namedcURL.php
orCurl.php
?
The file name is Curl.php [Curl.php]
https://www.screencast.com/t/BWNZ2gfUsE
#19
in reply to:
↑ 18
@
3 years ago
- Keywords reporter-feedback removed
Replying to apeksha10:
The file name is Curl.php [Curl.php]
https://www.screencast.com/t/BWNZ2gfUsE
Interesting. Thank you for confirming!
#20
in reply to:
↑ 17
;
follow-up:
↓ 23
@
3 years ago
- Keywords needs-patch needs-testing added
Replying to SergeyBiryukov:
I can confirm that
wp-admin/includes/update-core.php
is indeed specifically copied from the new package, however the rest of the code, including the Requests library, appears to be loaded from the current (older) install.
Thank you Sergey for confirming.
What's Known
What's known about the upgrade process:
- The "from" version of the Core code is loaded into memory and is running, including HTTP and Requests
- The "to" version of the
wp-admin/includes/update-core.php
code is running as it's injected/copied to run (makes sense) - But the filesystem no longer has the "from" version files available, meaning if some file was not loaded into memory before
update_core( $from, $to )
runs (i.e. which is in the "to" version of the thewp-admin/includes/update-core.php
file), then a fatal error can happen - And a cURL timeout error happens which so far is isolated to only Windows OS
- And because an error happens, to "to" version of the exception file isn't loaded into memory causing the fatal error
The 2 Known Problems
There are 2 problems surfaced from this ticket:
- Problem 1: Fatal error. Root cause reason is known.
- Root cause reason: Not all of the needed upgrade code is loaded into memory before the filesystem is changed with the new "to" version of the files.
- Note: In this specific case, it's the exception file within Requests. But it could easily be some other code for a different or future scenario.
- Problem 2: cURL timeout error happens. Root cause reason is unknown.
- Root cause reason: ??
Solving Problem 1
Problem 1 is solvable by doing what @jrf recommended: identify what files are needed for the upgrade and then get them loaded into memory before changing the filesystem. This can happen in the "to" version of the wp-admin/includes/update-core.php
file by:
- Identifying which "from" version files needed to be loaded.
- Doing a
require_once
to load each of those files at the top ofupdate_core()
, i.e. before delete and copy filesystem code.
I'm adding needs-patch
and needs-testing
for fixing this problem.
Solving Problem 2
Ideas?
This ticket was mentioned in Slack in #core-auto-updates by hellofromtonya. View the logs.
3 years ago
#22
in reply to:
↑ 10
;
follow-up:
↓ 25
@
3 years ago
Replying to jrf:
Just making sure the base information is correct:
Requests_Exception
was not removed. It became namespaced, just like every other class. The file was not renamed/moved as it was already calledException.php
.
So the
Exception
you see being thrown in theCurl
class is still the same exception, just namespaced now (and imported via ause WpOrg\Requests\Exception
import statement).
Sorry if this has been mentioned before (haven't had time to read all the comments on this ticket), but...
Yes, the file is still there, but the class defined in that file changed from the unnamespaced Request_Exception
to simple Exception
in the new namespace.
#23
in reply to:
↑ 20
@
3 years ago
Replying to hellofromTonya:
The 2 Known Problems
There are 2 problems surfaced from this ticket:
- Problem 1: Fatal error. Root cause reason is known.
- Root cause reason: Not all of the needed upgrade code is loaded into memory before the filesystem is changed with the new "to" version of the files.
- Note: In this specific case, it's the exception file within Requests. But it could easily be some other code for a different or future scenario.
- Problem 2: cURL timeout error happens. Root cause reason is unknown.
- Root cause reason: ??
Has anyone been able to identify what is making the "Problem 2" request? And what is being requested? Is it the updater itself (e.g., trying to update language packs after core has been updated)? Or is it possibly a plugin that has hooked into upgrader_process_complete?
#24
@
3 years ago
@pbiron Sorry, I meant to post this earlier today:
I added some logging and got this just before the fatal occurs:
Error: cURL error 28: Operation timed out after 60002 milliseconds with 0 bytes received curl_getinfo: Array ( [url] => http://582.local/wp-admin/upgrade.php?step=upgrade_db [content_type] => [http_code] => 0 [header_size] => 0 [request_size] => 262 [filetime] => -1 [ssl_verify_result] => 0 [redirect_count] => 0 [total_time] => 60.002039 [namelookup_time] => 0.000425 [connect_time] => 0.000625 [pretransfer_time] => 0.00065 [size_upload] => 0 [size_download] => 0 [speed_download] => 0 [speed_upload] => 0 [download_content_length] => -1 [upload_content_length] => 0 [starttransfer_time] => 0 [redirect_time] => 0 [redirect_url] => [primary_ip] => ::1 [certinfo] => Array ( ) [primary_port] => 80 [local_ip] => ::1 [local_port] => 57417 [http_version] => 0 [protocol] => 1 [ssl_verifyresult] => 0 [scheme] => HTTP [appconnect_time_us] => 0 [connect_time_us] => 625 [namelookup_time_us] => 425 [pretransfer_time_us] => 650 [redirect_time_us] => 0 [starttransfer_time_us] => 0 [total_time_us] => 60002039 )
#25
in reply to:
↑ 22
@
3 years ago
Replying to pbiron:
Yes, the file is still there, but the class defined in that file changed from the unnamespaced
Request_Exception
to simpleException
in the new namespace.
Yes, the file is still there. However, it has not been loaded into memory. The "from" version code is in memory including the autoloader. But the filesystem has copied changed the code. So for this specific issue, the autoloader in Requests loads the "to" version of the exception file into memory, but the code in that file is the "to" version which does not match the needs of the "from" version. It's the wrong class. Mismatch of "from" and "to" versions of the code.
This is why the solution proposed for Problem 1 is to preload all files that are potentially needed for the upgrade process before changing the filesystem files/code. This extra step would ensure all needed code is already in memory so that no additional files are needed to be loaded, such as the exception file in the Requests library.
Does that make sense @pbiron?
This ticket was mentioned in Slack in #core-auto-updates by costdev. View the logs.
3 years ago
#27
@
3 years ago
Update on Fatal Error
The Cause
- The
Requests_Exception
class isn't loaded into memory prior to the upgrade running. - When Requests 2.0 is downloaded and copied, it overwrites the contents of
wp-includes/Requests/Exception.php
with a namespacedWpOrg\Requests\Exception
class. - When the database upgrade is called and
Requests_Transport_cURL->process_response()
results in a cURL timeout, it tries to throwRequests_Exception
. Fatal.
The Solution
We can resolve this fatal by including the following line at the top of wp-admin/includes/update-core.php
in the upgrade package:
<?php require_once ABSPATH . 'wp-includes/Requests/Exception.php';
Why?
Requests_Exception
is used before Requests 2.0.WpOrg\Requests\Exception
is used after Requests 2.0.- Both use the same file.
- The exception class for the current Requests library used by the site needs to be loaded into memory before the upgrade process begins.
What else do we need to do?
- Update every upgrade package on wp.org for WordPress versions that use Requests.
- This has been confirmed as needed for earlier WordPress versions.
- Adding this fix to an earlier upgrade package before rolling back has been confirmed as working by myself and @mkaz.
- This means that we don't have to force a maintenance update on existing installs. When they try to upgrade or rollback, the package will have what they need.
Update on cURL timeout
The Cause
- This appears to be an issue contacting wordpress.org or with Apache.
- Props to @pbiron for reporting this timeout during testing:
PHP Warning: An unexpected error occurred. Something may be wrong with WordPress.org or this server’s configuration. If you continue to have problems, please try the <a href="https://wordpress.org/support/forums/">support forums</a>. (WordPress could not establish a secure connection to WordPress.org. Please contact your server administrator.)
- The timeout also occurred on Apache for me.
- The timeout doesn't happen every time.
- This doesn't occur on NGINX for me or for @mkaz.
The Solution
- Technically, if a timeout occurs, it should throw the above warning.
- @pbiron notes that increasing the timeout for the Requests library has proven useful for avoiding timeouts. However, this timeout should not be happening in the first place. The default timeout is 60 seconds to contact wp.org.
- Resolving the timeout requires more investigation.
Why?
- The timeout, at least in some cases, occurs at the "Upgrading Database..." step.
- I haven't looked into whether or not the database gets upgraded before this fails.
- The timeout, at least in some cases, causes the updater lock to remain in place, blocking repeat updates without deleting the entry in the database.
What else do we need to do?
- Investigate the cause of this particular timeout.
- Determine whether it is only occurring on Windows with Apache.
- Explore a solution.
- Test.
Huge props to @pbiron and @mkaz for helping to test this fix.
This ticket was mentioned in PR #1999 on WordPress/wordpress-develop by costdev.
3 years ago
#28
- Keywords has-patch added; needs-patch removed
Trac ticket: https://core.trac.wordpress.org/ticket/54546
SergeyBiryukov commented on PR #1999:
3 years ago
#29
Thanks for the PR! Just to clarify, this would only be needed for 5.8 and older branches, but not for trunk, right?
Should we also add a comment to explain this line?
3 years ago
#30
Thanks for the PR! Just to clarify, this would only be needed for 5.8 and older branches, but not for trunk, right?
This line will be needed for every branch of WordPress that uses the Requests library (both past and future branches).
Why?
- Every
< 5.9.0
branch can upgrade to> 5.9.0
and will fatal on a missingRequests_Exception
class. - Every
> 5.9.0
branch will be able to rollback to< 5.9.0
and will fatal on a missingWpOrg\Requests\Exception
class.
This line will only need to exist in the upgrade packages.
Why?
- The upgrade package's
wp-admin/includes/update-core.php
file is the one that runs during the upgrade process. - This will provide the fix to all existing installs upon upgrade, negating the need to force a maintenance update on existing installs.
Upgrading to 5.9.0 Beta 1
and rolling back to 5.8.2
has been tested by myself and @mkaz.
This test used a custom upgrade package with the line added to its wp-admin/includes/update-core.php
file.
Should we also add a comment to explain this line?
Absolutely. How does this sound?
{{{php
/
- Namespacing was introduced in Requests 2.0.
- This file MUST be loaded into memory before the upgrade process
- to prevent a fatal error due to class names that no longer exist
- after the upgrade.
- This applies to all WordPress versions that use the Requests library. */
require_once ABSPATH . 'wp-includes/Requests/Exception.php';
}}}
hellofromtonya commented on PR #1999:
3 years ago
#31
This line will be needed for every branch of WordPress that uses the Requests library (both past and future branches).
@costdev The upgrader uses the "to" version wp-admin/includes/update-core.php
file, meaning only 5.9 needs this code. For older WordPress versions upgrading to 5.9, this PR will ensure that missing "from" version exception file is loaded into memory from the older WordPress version.
Why then does this need to be backported to all the older branches?
hellofromtonya commented on PR #1999:
3 years ago
#32
The upgrader uses the "to" version
wp-admin/includes/update-core.php
file, meaning only 5.9 needs this code. For older WordPress versions upgrading to 5.9, this PR will ensure that missing "from" version exception file is loaded into memory from the older WordPress version.
Why then does this need to be backported to all the older WordPress branches?
Sharing a conversation with @costdev.
The Upgrader does load and use "to" version's wp-admin/includes/update-core.php
file. This means upgrading from an older version of WordPress ("from" version) to 5.9 ("to" version) is handled by 5.9.
Back downgrading (rolling back) to an older version from 5.9 will cause a fatal error for the same reason as upgrading to 5.9. So the backports are required to ensure the older ("to") version's wp-admin/includes/update-core.php
file also loads the needed file(s) into memory to avoid the fatal error.
Backporting does not require a point release. Why? The Upgrader fetches the "to" version from wp.org. Backporting then upgrades the upgrader package (wow that was a clumpsy way to phrase it) for use when rolling back from WordPress 5.9.
hellofromtonya commented on PR #1999:
3 years ago
#33
Update:
@costdev and I experimented with creating a "preloader" to load all "from" version files into memory (including the entire Requests library). We have a working model. There are pain points to resolve in downgrading from 5.9. Plus a preloader introduces complexity that needs extensive testing to avoid introducing unexpected upgrade issues. Given both of these points, it's too late in this beta period to introduce a "preloader".
Instead, let's first preload the file that directly caused the reported fatal error. Then continue monitoring feedback for other files that _may_ also need preloading.
A "preloader" can be explored during 5.9 if the entire Requests library does need preloading. Else, it can be explored in a future release.
#34
@
3 years ago
- Keywords commit added
PR 1999 directly resolves the fatal error by preloading the wp-includes/Requests/Exception.php
file from the "to" version of WordPress before the files on the filesystem are upgraded to 5.9.
It's ready for commit.
Note: This PR does not resolve the cURL timeout issue.
#35
follow-up:
↓ 36
@
3 years ago
@costdev @pbiron Should the cURL timeout value be increased to give more time to avoid the timeout error?
#36
in reply to:
↑ 35
@
3 years ago
Replying to hellofromTonya:
@costdev @pbiron Should the cURL timeout value be increased to give more time to avoid the timeout error?
I'm not sure that's necessary. In most "real world" cases, when timeouts occur it's because there is a problem with the connection (or the remote server) and having an unduly long timeout just delays the inevitable timeout error :-)
The default for the wp_remote_{method}()
functions is 5 seconds (filterable by http_request_timeout, see L161, wp-includes/class-wp-http.php), but various different calls to those functions throughout core often pass different timeout values in the $options
param (e.g., wp_version_check() uses 3 seconds).
For me, that is too short, but that's just because my home internet connection is soooooo slow (satellite provider): average ping times for me are in the 600-900 ms range :-(. So, on all the sites I run locally, I hook into http_request_timeout
and add an extra 10 seconds.
For those with normal connection speeds, I think the defaults currently used throughout core are fine...but certainly wouldn't object if others wanted to increase them.
3 years ago
#38
@hellofromtonya see https://github.com/WordPress/wordpress-develop/pull/1999#issuecomment-985265243. I think it would be good to add that inline comment (or something like it) before committing.
SergeyBiryukov commented on PR #1999:
3 years ago
#39
The Upgrader does load and use "to" version's
wp-admin/includes/update-core.php
file. This means upgrading from an older version of WordPress ("from" version) to 5.9 ("to" version) is handled by 5.9.
Back downgrading (rolling back) to an older version from 5.9 will cause a fatal error for the same reason as upgrading to 5.9. So the backports are required to ensure the older ("to") version's
wp-admin/includes/update-core.php
file also loads the needed file(s) into memory to avoid the fatal error.
Backporting does not require a point release. Why? The Upgrader fetches the "to" version from wp.org. Backporting then upgrades the upgrader package (wow that was a clumpsy way to phrase it) for use when rolling back from WordPress 5.9.
I'm not quite sure about the latest part. Is it about a manual downgrade or an automated rollback if the background update didn't work? Isn't the rollback package only updated once a new minor version is released?
#40
@
3 years ago
Would moving the files to wp-includes/requests/src be a potential solution ? (to make sure they are seen as new files, not updated files)
Moving the Requests library into a different directory would solve the issue reported in #54582; however, it would not solve the issue reported in this ticket.
Why? The filesystem is changed and old files removed before `wp_remote_post()` is called to upgrade the database. wp_remote_post()
uses WP_Http
which uses Requests.
#41
@
3 years ago
Bigger picture of this problem:
This ticket revealed a fundamental problem in Core's upgrader: the current ("from") version code needs to be preloaded into memory for use before doing the filesystem and database upgrade to the "to" version.
Why? To avoid an intermingling of "from" and "to" version code and/or missing dependencies that can cause errors or expected behaviors.
Though this ticket is highly specific to a single exception file that is loaded when needed, there also lies other unknown issues where "to" and "from" code could be intermingling leading to unexpected (and hard to debug) behavior during the upgrade.
How so? Core code is changed throughout the release cycle. If any of that code is needed but not preloaded before upgrading, then the "to" code will get loaded and may not behave the same as the "from" code version or may expect different dependencies.
Stop Gap fix
PR 1999 is a stop gap fix. It does not resolve the bigger problem in the upgrader. And there's a risk that other files may also need preloading in different edge cases that could also cause a fatal error.
Audit and Preloader
I agree with @jrf. An audit is needed to identify all of the files the upgrader process will or could use (such as alternate code, e.g. exceptions) and then ensure each is preloaded into memory before doing the filesystem upgrade. A preloader though needs extensive testing and likely should be early
in a release cycle. Touching the upgrade this late is risky.
I'll open a new ticket for the upgrade audit and preloader.
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
3 years ago
#43
@
3 years ago
- Keywords commit removed
- Milestone changed from 5.9 to 6.0
Decision was made to revert Requests 2.0.0 and punt it and all associated tickets to WordPress 6.0. Why? There are 2 issues in the upgrader process that need resolution, which will resolve the fatal error reported in this ticket.
This ticket was mentioned in Slack in #core-auto-updates by hellofromtonya. View the logs.
3 years ago
#45
@
3 years ago
Hi, found this bug while googling for string "wp-admin/includes/file.php on line 1201".
I upgraded to 5.9 yesterday but every now and then I receive an error from cron:
PHP Warning: rename(wordpress-5.9-pl_pl.zip): Failed to open stream: Permission denied in /z/wp-admin/includes/file.php on line 1201
PHP Warning: rename(/tmp/pl_PL-4Lxdqs.tmp,wordpress-5.9-pl_pl.zip): Permission denied in /z/wp-admin/includes/file.php on line 1201
All rights to WP directory and /tmp are fine.
#46
@
3 years ago
@Grzegorz.Janoszka As the changes this ticket refer to were reverted, your issue is unrelated. Please open a new, separate issue.
This ticket was mentioned in Slack in #core-auto-updates by afragen. View the logs.
3 years ago
This ticket was mentioned in Slack in #core-auto-updates by afragen. View the logs.
3 years ago
#50
@
3 years ago
- Milestone changed from 6.0 to Future Release
Moving this ticket to Future Release in line with the Requests 2.0.0 ticket #54504.
This ticket was mentioned in Slack in #core by costdev. View the logs.
3 years ago
#52
@
3 years ago
- Milestone changed from Future Release to 6.1
This ticket was discussed during the recent bug scrub.
As the Requests 2.0 ticket (#54504) has been milestoned for 6.1 and this issue is a blocker for Requests 2.0, I'm moving this ticket to the same milestone.
Note: PR 1999 is a stopgap fix that can be implemented ahead of investigation and implementation of #54589.
While it's possible that there are other Core files required during upgrade which are not preloaded, I believe that we don't have any other reports at this time. A deep-dive to investigate this further is planned as part of #54589.
This ticket was mentioned in Slack in #core by brianhenryie. View the logs.
3 years ago
3 years ago
#54
I've just pushed an update that should cover downgrades without needing to backport the require_once
line.
TL;DR - The newer WordPress version applies the fix in both of the necessary locations and the older WordPress version doesn't need to do anything.
- When upgrading:
wp-admin/includes/update-core.php
is owned by the "to" version. - When downgrading:
wp-admin/includes/class-wp-upgrader.php
is owned by the "from" version.
Therefore:
- Upgrading from 5.8.4 to 6.1.0 will use 6.1.0's
wp-admin/includes/update-core.php
file, which will load 5.8.4'sRequests_Exception
class into memory before overwriting its contents with 6.1.0'sWpOrg\Requests\Exception
class. - Downgrading from 6.1.0 to 5.8.4 will use 6.1.0's
wp-admin/includes/class-wp-upgrader.php
file, which will load 6.1.0'sWpOrg\Requests\Exception
class into memory before overwriting its contents with 5.8.4'sRequests_Exception
class. - The "downgrading" portion is only ever going to be done from a version of WordPress that includes Requests, so it doesn't need a version check.
- The "upgrading" portion could be upgrading from any earlier version, so it needs to keep the version check already included in
wp-admin/includes/update-core.php
.
Happy to answer any questions to clarify anything about this update.
This ticket was mentioned in Slack in #core-auto-updates by costdev. View the logs.
3 years ago
#56
@
3 years ago
- Keywords dev-feedback added
- Milestone changed from 6.1 to Future Release
As the Requests 2.0.0 ticket (#54504) has been moved to Future Release, I'm milestoning this for the same.
To move this ticket forward, I'd appreciate some feedback on this comment on PR 1999.
Hi there, welcome to WordPress Trac! Thanks for the report.
Related: #54547. Moving to the milestone for more visibility.