WordPress.org

Make WordPress Core

Opened 13 months ago

Closed 4 months ago

Last modified 3 weeks ago

#44458 closed task (blessed) (fixed)

Catch WSODs and provide a means for recovery for end users

Reported by: schlessera Owned by: flixos90
Milestone: 5.2 Priority: normal
Severity: normal Version:
Component: Site Health Keywords: has-dev-note has-patch servehappy
Focuses: multisite Cc:

Description

While discussing PHP version updates at WCEU18, @pento introduced the notion of "sandboxing" plugins & themes to avoid a WSOD for end users doing a PHP update and having a broken plugin active on their site.

The goal of this "sandboxing" should be to catch/track WSODs and then provide a mechanism for users to still get into the site's admin backend to update/deactivate/replace the offending plugin.

Technical details still have to be figured out, but it looks like our best bet so far is to not do actual "sandboxing", but tracking fatal errors, and then entering some form of "safe mode" on subsequent requests.

Attachments (18)

44458-1.diff (4.9 KB) - added by schlessera 13 months ago.
First proof-of-concept implementation (still missing UI integrations)
44458-2.diff (16.0 KB) - added by schlessera 12 months ago.
Second iteration, adding all the basic UI integration (for plugins only for now) [Updated]
44458-3.diff (23.0 KB) - added by schlessera 12 months ago.
Refactored persistence code, added error output, "paused" filter in plugin list and redirect to filtered view
44458-4.diff (23.1 KB) - added by schlessera 12 months ago.
Avoid array derefencing for PHP 5.2 compat
44458.diff (23.4 KB) - added by flixos90 12 months ago.
44458.6.diff (23.2 KB) - added by SergeyBiryukov 12 months ago.
44458.7.diff (29.3 KB) - added by schlessera 11 months ago.
Catch all fatals, use redirection to catch multiple errors and protect AJAX handlers
44458.8.diff (31.4 KB) - added by flixos90 10 months ago.
44458.9.diff (41.8 KB) - added by flixos90 9 months ago.
broken-plugin-site-admin.png (323.1 KB) - added by flixos90 9 months ago.
Example with (manually) broken "Hello Dolly" plugin in the Site Admin
broken-plugin-network-admin.png (341.7 KB) - added by flixos90 9 months ago.
Example with (manually) broken "Hello Dolly" plugin in the Network Admin
44458.10.diff (41.8 KB) - added by flixos90 7 months ago.
44458.11.diff (54.9 KB) - added by flixos90 7 months ago.
44458.12.diff (58.0 KB) - added by flixos90 6 months ago.
44458.13.diff (56.1 KB) - added by flixos90 6 months ago.
memory-plugin-1.php (681 bytes) - added by WFMattR 6 months ago.
Test plugin that uses a lot of memory
memory-plugin-2.php (737 bytes) - added by WFMattR 6 months ago.
Test plugin that uses a little memory, after plugins have loaded
local-file-inclusion-example.php (346 bytes) - added by WFMattR 6 months ago.
Test plugin that has a sample Local File Inclusion bug - do not install on a production site

Download all attachments as: .zip

Change History (105)

This ticket was mentioned in Slack in #core-php by schlessera. View the logs.


13 months ago

#2 @schlessera
13 months ago

(copied from Slack for future reference)

I've started looking into this briefly, and here's what I've found out so far:

  • Exceptions don't help, as they are not fully integrated into the error handling at PHP 5.2.
  • We can use a shutdown handler to detect fatal errors and know where they were triggered: https://3v4l.org/4jWAs .
  • Such a shutdown handler could record a fatal error, and the next page request could then detect a recorded fatal error and decide based on some heuristics whether to initiate "safe mode". (edited)

There's an obvious security-related issue here:
This "safe mode" will skip loading plugins & themes, so it will also skip any security-related enhancements that were done to the site. An attacker might forcefully trigger the "safe mode" to get past security mechanisms.

Some other assumptions I'm making right now:

  • We cannot just act on plugin activation/deactivation, as this will still take the site down if we update PHP.
  • We cannot disable a single plugin, as we cannot reliably detect who the actual culprit is in all cases.
  • We might be able to disable a single plugin in those cases where we hit a parse error in a file of a plugin.

#3 @schlessera
13 months ago

Another proposal:
Only allow for safe mode on logged-in /wp-admin/* requests, and build in the following way:

  • a shutdown handler is registered that will redirect to the same URL with a nonce-based safe mode URL argument added.
  • If page loads correctly, shutdown handler is removed again before it is executed.
  • If page would WSOD, it will trigger the shutdown handler, which will redirect to the safe-mode-version of that same page.

This ticket was mentioned in Slack in #core-php by flixos90. View the logs.


13 months ago

#5 @MKSnMKS
13 months ago

Hi schlessera,

I was referred by Joy (Joyously) who replied to my topic at

https://wordpress.org/support/topic/wp-core-plugin-update-fail-white-screen-od-method-to-detect-and-repair/

to make a comment in your topic here.

My suggestion in short, is to ;
a) let the whole process of failure take place.
b) detect the white screen of death, by using a server generated page request
c) check the result of the page request
d) if the result is bad, then automatically to the recovery process (which presently must be manually done)
e) this retrieves front and back end access to the website, so the admin can diagnose from the wordpress dashboard, and authors can carry on undisturbed, and (most importantly) the website is back on line.

I describe several instances where this approach could be used.

This should be fairly easy to do right now - the fix is known i.e. rename the folders of recently updated plugins (but automate the timing/event of this process, and use a script to do it automatically).

e.g.
for script flow to begin the recovery action
i) and no action from recent actioning admin
ii) and 'delay time' has passed
iii) so automatically have the server send a page request to admin or front-end
iv) detect bad page response to page request, so (if possible) display a simple text page (much like the maintenance notice - with an estimate of the number of minutes till back online, then run the recovery script (below)

for script flow to recover from white screen of death
i) search plugins folders for plugins whose folders were altered in the last time period
note - this time period could be set to a user chosen default, or left to a system default of the 'delay time (above)' plus 3 hours
ii) rename all those folders (found in (i)), by prepending "Disabled_due_to_WSOD--" to the names of each folder (the main folder for each plugin).
iii) optionally (user chosen setting or leave for prompting during live run) send an email to admin, saying what happened and the site should be live now.

The web site should then be back to near normal.

The first script could be made to be manually run, in various situations, and made easy to run by use of a button, for example in cPanel admin "auto repair WordPress White Screen of Death", or send a coded email to a special email address that then carries out the action.

Also
It would be handy to have an automated system to re-update a failed update, that could be done from the wordpress dashboard after access is recovered, or from cPanel by button, which would update the folders previously detected as recently altered.
Precautions may be needed to not loose user's custom settings, or database, but they may be able to be scripted also.

Plugin authors could be invited to supply scripts for recovery in the event that their plugin needs to be repaired after after a WSOD event (this could be made a requirement at some future time).

I hope this helps

#6 @MKSnMKS
13 months ago

Hi,

Having made the suggestion described mostly above, originating from the original suggestion at

https://wordpress.org/support/topic/wp-core-plugin-update-fail-white-screen-od-method-to-detect-and-repair/

You may see how difficult it can be to get suggestions through to where ever they need to go.
The topic has been closed, and there is no indication of where it can go within the wordpress development cycle, that would be of any use.

The support forum allows a topic to be set as “this topic is not a support question” in the submission, which I had done.
So clearly the support forum can take topics which are 'not support questions'.
And the particular forum was for making suggestions (requests and feedback), yet I am now told "but it’s not really appropriate for support forum." and the topic has been closed.

@schlessera
13 months ago

First proof-of-concept implementation (still missing UI integrations)

#7 @schlessera
13 months ago

I uploaded a first proof-of-concept of the safe mode mechanism.

I had to find out that the debug_backtrace is useless, as the stack is completely unrolled before the shutdown handler is being called.

However, error_get_last() returns both the type of error as well as the file where it was triggered in.

With that information, we can do some deduction, like a parse error only ever happens in the file that is actually the cause of the problem, so that can be directly tied to a plugin/mu-plugin/theme.

Also, I built it so that it can be customized on sites just like with advanced-cache.php. You can drop a shutdown-handler.php file into the wp-contents folder that provides a wp_handle_shutdown() function. This lets you customize/override/disable the shutdown handling logic.

I'm currently adding the detected culprits to a DB option called skip_on_admin. When loading the admin or when you're on the login page, these plugins/mu-plugins/themes will always be skipped until you manually declare them to be safe again.

The front-end will not be impacted at all by this. We're only filtering the loading of these when we're inside the admin backend.

The basic system seems to work. When I produce a parse error in a plugin, my frontend breaks and I get a WSOD. The plugin that causes the error is stored in the DB in the skip_on_admin option, though. Then, when I go to either a login or an admin screen, that particular plugin is filtered out of the active plugins list and doesn't get loaded. So although the frontend is broken and shows a WSOD, logging in and working in the admin backend still works correctly.

What needs to be added now is:

  • an admin notice to let the user know that something is broken
  • an integration into the plugin list that lets the user remove the plugin from the skip_on_admin list.

Also, I'm still not sure whether I should include must-use plugins in that mechanism or not...

This ticket was mentioned in Slack in #core-php by schlessera. View the logs.


13 months ago

#9 follow-up: @schlessera
12 months ago

The second iteration contains basic UI integration, but currently only for plugins.

On the frontend, when an error is triggered, WordPress will display a corresponding message, and provide a link to log into the admin dashboard to fix the issue.

The message is filterable via 'wp_technical_issues_display', and the entire page can be replaced via a drop-in called php-error.php.

https://i.imgur.com/S06ErFV.png

Then, once the site owner has logged into the admin backend, they see an admin notice letting them know about the problem and providing a link to go to the plugins screen to manipulate the plugins.

https://i.imgur.com/alGWnNQ.png

Finally, once they are on the Plugins screen, they can see the plugins that have been "paused" within the admin backend because of an error. The site owner will be able to do all sorts of manipulations, like updating the plugin, deactivating it, deleting, etc... Once the site owner thinks they solved th problem, they can choose to "Resume execution" for the problematic plugin. If the plugin still contains an error, it will be caught and recorded again.

https://i.imgur.com/y3k8ZDc.png

Questions & remaining issues:

  • We should decide on specific language (on the technical side just as well as facing the site owners). Right now, the code talks about "pausing" a plugin and "resuming its execution" (with caps like 'resume_plugin' and similar).
  • Once the language has been decided, we should implement the same UI mechanisms for themes as well.
  • Multisite has been largely ignored for now. This will probably require specific handling of network-activated plugins.
  • We need to decide whether must-use plugins should be pausable or not. I have no clear answer for this yet.
  • In an event like a PHP update, multiple plugins might break at the same time. This would lead to a very confusing back & forth until all problematic plugins have been properly detected. We should think about how we can handle this (an AJAX loop with the paused plugins actually paused, to detect new issues?).

This ticket was mentioned in Slack in #core-php by schlessera. View the logs.


12 months ago

@schlessera
12 months ago

Second iteration, adding all the basic UI integration (for plugins only for now) [Updated]

#11 in reply to: ↑ 9 @flixos90
12 months ago

  • Focuses multisite added
  • Keywords has-patch needs-unit-tests needs-testing added

Replying to schlessera:

The second iteration contains basic UI integration, but currently only for plugins.

I really like the direction this approach is going, thanks for doing all that work on it!

  • Once the language has been decided, we should implement the same UI mechanisms for themes as well.
  • Multisite has been largely ignored for now. This will probably require specific handling of network-activated plugins.

I can see this project becoming really big and complex for a single patch and commit. In order to maintain a good overview as well as easy workflow, I think it would be a good idea to move work on this over to a GitHub fork of WordPress. Then we can handle the respective integrations with themes, multisite and possibly MU plugins via separate branches/PRs.

  • We need to decide whether must-use plugins should be pausable or not. I have no clear answer for this yet.

I would advise against this, for three reasons:

  • MU plugins often contain very foundational functionality of a website.
  • Someone who uses MU plugins usually has a little more technical knowledge than who the safe mode is most beneficial for.
  • There is no way to manage MU plugins through the admin. Of course we could introduce a way to resume a MU plugin, but for the two previous reasons, I don't think it's worth the hassle.
  • In an event like a PHP update, multiple plugins might break at the same time. This would lead to a very confusing back & forth until all problematic plugins have been properly detected. We should think about how we can handle this (an AJAX loop with the paused plugins actually paused, to detect new issues?).

For now, in my opinion we should try to focus on detecting in the most accurate way possible which plugin(s) or theme(s) are causing the error. An AJAX loop is what I thought about too.

A general suggestion for improvement I have is that we should consider keeping the user in the backend when they are already there while something breaks. For example, if you're in the Plugins screen, resuming a plugin, and then it's still broken, it would be great if the user could automatically stay on that page, getting the feedback message there that it was paused again (instead of seeing the message like in the frontend and having to click a link). We could try to redirect the user to the page they're currently on after pausing the broken plugin(s) and theme(s), if that is already a logged-in admin page. Of course this only works under certain circumstances, if we know who the current user is and if we're still before output. This could very well happen a little later, when we improve the UX further.

#12 follow-up: @schlessera
12 months ago

I can see this project becoming really big and complex for a single patch and commit. In order to maintain a good overview as well as easy workflow, I think it would be a good idea to move work on this over to a GitHub fork of WordPress. Then we can handle the respective integrations with themes, multisite and possibly MU plugins via separate branches/PRs.

I'm not sure that is the case. I think right now, we cover already 60-70% of the needed code. Half of the multisite plugin stuff is already covered, and themes can mostly reuse the same code.

I would advise against this, for three reasons:

  1. MU plugins often contain very foundational functionality of a website.
  2. Someone who uses MU plugins usually has a little more technical knowledge than who the safe mode is most beneficial for.
  3. There is no way to manage MU plugins through the admin. Of course we could introduce a way to resume a MU plugin, but for the two previous reasons, I don't think it's worth the hassle.

I'm not entirely sure these are valid reasons.

  1. Yes, but a parse error means they just take down the site either way. The importance of the plugin is irrelevant in that case.
  2. Sometimes MU plugins ar not installed by the site owner themselves, but by hosting or another plugin.
  3. This is the only argument that plays a role here I think. What's the point of making people enter th eplugin screen if they have no valid options. I wonder though (not tested yet) whether they could modify the MU plugin through the built in code editor...

For now, in my opinion we should try to focus on detecting in the most accurate way possible which plugin(s) or theme(s) are causing the error. An AJAX loop is what I thought about too.

Yes, agree.

A general suggestion for improvement I have is that we should consider keeping the user in the backend when they are already there while something breaks. For example, if you're in the Plugins screen, resuming a plugin, and then it's still broken, it would be great if the user could automatically stay on that page, getting the feedback message there that it was paused again (instead of seeing the message like in the frontend and having to click a link). We could try to redirect the user to the page they're currently on after pausing the broken plugin(s) and theme(s), if that is already a logged-in admin page. Of course this only works under certain circumstances, if we know who the current user is and if we're still before output. This could very well happen a little later, when we improve the UX further.

Yes, I had already thought about adding something like that. It could be similar to the Shiny Updates plugin, where it triggers a sandboxed request to see whether the plugin now works correctly again.

@schlessera
12 months ago

Refactored persistence code, added error output, "paused" filter in plugin list and redirect to filtered view

#13 @schlessera
12 months ago

I refactored the persistence code. Recorded errors are now stored in the database. To avoid filling up the database, I added automatic pruning that get rids of any records that are not active plugins.

The recorded error message is now shown in the plugin list. Not sure whether that's a good or a bad thing, but it definitely needs better styling at the very least. I had planned to make the error display collapsible to not scare away users immediately, but decided to skip introducing JS code for now.

The plugin list is now filterable by 'paused' status, and the admin notice will bring users automatically to a filtered view of the plugin list. This helps find the problem right away on sites that have multiple pages of plugins.

https://i.imgur.com/wlZ6TB2.png

#14 @ibenic
12 months ago

Such a great feature! Thank you for working on this.

What would happen if such an error comes from a function/method hooked on an action/filter in the admin area?

My guess would be the first run would cause WOSD and a page refresh would then get the user back in the admin area (unless another plugin causes WOSD again in the admin area). I have patched my local install and will do some runs tomorrow since it's already 1:30am so I might miss something :)

This ticket was mentioned in Slack in #core-php by schlessera. View the logs.


12 months ago

#16 @schlessera
12 months ago

@ibenic Parse errors happen when the PHP file is parsed so that the functions and methods can be declared.

For an action/filter to work, the function/method it uses as a callback needs to be declared before the action/filter is triggered, and this is usually done right when the plugin is being loaded (but not always). So parse errors should in almost all cases be detected during the plugin loading stage.

We're not currently doing anything with runtime errors. The main issue with runtime errors is that they are neither reliably broken (it could just have been a temporary hiccup) nor can they be properly traced back to their root cause (as the bug is often contained in one plugin, but another plugin then trips over the previous plugin's bad data and throws a fatal error).

We'll have to discuss in more detail what types of errors we want to catch and how we want to handle them. For now, I opted to go with the safest route to test the mechanism.

#17 @MKSnMKS
12 months ago

Hi,
Any thoughts on my thoughts, or are they not worth commenting on?
They are not elegant, nor too deep in the system, but they are probably easier to get going for now.
The central process is done manually by many users, and is practically the only way it done (that I have found - described in many places by numerous authors, on all sorts of websites).

The diagnosis method is
a) swap in/swap out, till it works, so the 'problem area has been located'
rather than
b) system level fault reporting to find the 'cause'.

The rest of it is the user assisted/automated processes.
At any time in future, the method of diagnosis can be changed, or combined, with this process.

This ticket was mentioned in Slack in #core-php by flixos90. View the logs.


12 months ago

@schlessera
12 months ago

Avoid array derefencing for PHP 5.2 compat

@flixos90
12 months ago

#19 @flixos90
12 months ago

44458.diff is an update based on 44458-4.diff that fixes two issues:

  • In wp_record_extension_error() support the plugin directory constants and registered theme directories correctly, as we can't rely on them having the default mu-plugins, plugins and themes names.
  • In wp_forget_extension_error() fix a bug where an extension would never be resumed because the option itself would not be changed.

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


12 months ago

#21 @SergeyBiryukov
12 months ago

In 44458.6.diff:

  • Adjust wp_record_extension_error() to work with Windows paths as well (string manipulations should use wp_normalize_path() to work as expected).
  • Fix PHP 5.2 warnings in class-wp-plugins-list-table.php:
    • Before 5.3, core constants used to be in the internal group.
    • On 5.2 there's less than 15 error constants, so an array_slice() of the first 15 items includes TRUE and FALSE, which then causes a warning in array_flip(): "Can only flip STRING and INTEGER values". Instead of hardcoding 15 items, the patch builds an array with all core constants starting with E_.
Last edited 12 months ago by SergeyBiryukov (previous) (diff)

#22 @schlessera
12 months ago

I made a more thorough examination of the potential errors we can catch:

https://www.notion.so/brightnucleus/WP-Sandbox-88738b62e9e947a7aeb8271d958a5497

As a conclusion, here's what I think we should try to catch:

E_ERROR             => register_shutdown_function()
E_PARSE             => register_shutdown_function()
E_RECOVERABLE_ERROR => set_error_handler() (turn into E_ERROR ?)
Exception           => try/catch
Error               => try/catch (PHP7+)

The error handler for E_RECOVERABLE_ERROR and the try/catch block for the exceptions can just turn them into an E_ERROR so that the shutdown handler can take over. This allows us to guide everything through one singular funnel.

This ticket was mentioned in Slack in #core-php by flixos90. View the logs.


12 months ago

This ticket was mentioned in Slack in #core-php by flixos90. View the logs.


11 months ago

@schlessera
11 months ago

Catch all fatals, use redirection to catch multiple errors and protect AJAX handlers

#25 @schlessera
11 months ago

The latest version fixes many minor issues and introduces redirection to catch multiple errors in one go.

https://video.twimg.com/tweet_video/DlINMm7VsAExJte.mp4

This ticket was mentioned in Slack in #core-php by schlessera. View the logs.


11 months ago

This ticket was mentioned in Slack in #core-php by flixos90. View the logs.


11 months ago

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


10 months ago

#29 follow-up: @bradleyt
10 months ago

This looks like a really positive change. A few comments regarding 44458.7.diff.

  • In plugin.php "shutdown-handler" is listed without the ".php", which is inconsistent with the other drop-in files
  • It feels like a shame that is_protected_endpoint is not filterable, to allow plugin authors to specify their own critical routes. (For example, an SEO plugin may wish to ensure robots.txt is correctly returned even if the site enters safe-mode)
  • In wp_shutdown_handler_wrapper you check if the options API has been loaded before trying to load php-error.php which feels like a mistake to me (why would you not want a custom error page, even if the issue cannot be recorded)
  • You suggest that custom php-error.php pages should die() so that the default wp_die() is not run. However, this means that sites with a custom php-error.php won't get the is_protected_endpoint redirect loop which feels a shame.
  • You default to HTTPS if is_ssl has not been loaded - This feels like a mistake as if the server does not have HTTPs setup they will be redirected to a browser error page, with no indication of what went wrong. As this feature is targeted at less advanced users, it feels more sensible to not do the redirect at all if is_ssl is not available.

This ticket was mentioned in Slack in #forums by sheri. View the logs.


10 months ago

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


10 months ago

This ticket was mentioned in Slack in #core-php by schlessera. View the logs.


10 months ago

#33 in reply to: ↑ 29 @flixos90
10 months ago

Replying to bradleyt:

In plugin.php "shutdown-handler" is listed without the ".php", which is inconsistent with the other drop-in files

Good catch!

It feels like a shame that is_protected_endpoint is not filterable, to allow plugin authors to specify their own critical routes. (For example, an SEO plugin may wish to ensure robots.txt is correctly returned even if the site enters safe-mode)

I agree a filter would be valuable. However it should not be possible to alter the default protected endpoints, to make them "unprotected" as this could have severe results.

In wp_shutdown_handler_wrapper you check if the options API has been loaded before trying to load php-error.php which feels like a mistake to me (why would you not want a custom error page, even if the issue cannot be recorded)

This is a tricky one. I think the decision on this depends on whether we see the new 500 error markup as something that should always be rendered in case a WSOD would otherwise occur, or whether we see it as something that should only be rendered in case we can actually do something about it and point the user to the admin area where they can fix the issue. I'm not sure about this one yet.

You suggest that custom php-error.php pages should die() so that the default wp_die() is not run. However, this means that sites with a custom php-error.php won't get the is_protected_endpoint redirect loop which feels a shame.

I think the redirect logic should be executed before it is checked whether a php-error.php drop-in exists. This would maintain support for the redirects in all cases, and furthermore make more obvious that php-error.php is simply a template that should output (status) headers and markup, similar to the existing db-error.php for example.

You default to HTTPS if is_ssl has not been loaded - This feels like a mistake as if the server does not have HTTPs setup they will be redirected to a browser error page, with no indication of what went wrong. As this feature is targeted at less advanced users, it feels more sensible to not do the redirect at all if is_ssl is not available.

I actually think this check can be removed as is_ssl() is always available when the shutdown handler is active, since it is part of the load.php file as well.

@flixos90
10 months ago

#34 @flixos90
10 months ago

44458.8.diff makes the following adjustments:

  • Fix shutdown-handler to appear as shutdown-handler.php in drop-ins list.
  • Always show PHP error page even if the recorded error could not be persisted.
  • Introduce is_protected_endpoint filter that allows providing further protected endpoints in addition to core's.
  • Remove unnecessary function_exists() checks for is_protected_endpoint() and is_ssl(), since those reside in the same file.
  • Ensure that, even when using a custom php-error.php template, the redirect routine for detecting multiple errors in one go fires. Make including that template similar to how db-error.php is handled: It should control the HTTP status code and print markup (either through custom HTML or wp_die()).
  • Fix WP Coding Standard violations and minor tweaks.

As always, for details please refer to https://github.com/wp-core-php/wordpress-develop/pull/3 where you can see individual commits.

A few more things to consider:

  • We need to figure out whether we want to show the error page if the detected error could not be recorded successfully. wp_record_extension_error() already returns a boolean to indicate success or failure, which we don't use at the moment. We could though.
  • In the same trail of thought, I haven't played this through yet, but I think we can possibly run into a redirect loop right now if the error could not be recorded.

I think in multisite the resume_plugin capability should not be tied to whether the user can activate a plugin in a multisite. Plugins are currently paused only for a site, so I'd argue that means a site administrator should already have access to resume one. We would also need to adjust the plugin row actions then, as in even when a plugin is network active, it should render a Resume execution link if the current user has the capabilities. Note that this is just an initial observation I made - we will discuss multisite compatibility in detail in today's office hours.

This ticket was mentioned in Slack in #core-multisite by jeremyfelt. View the logs.


10 months ago

#36 @flixos90
10 months ago

We discussed this ticket in multisite office-hours today. Here are our thoughts:

  • It should be possible to resume plugins for a regular site administrator in every case, since the paused plugins are stored per site (and that should continue to happen). Therefore, in map_meta_cap(), resume_plugin should receive its own case where it always maps to activate_plugins, no matter what (without the extra network handling the other plugin capabilities have).
  • Network-active plugins should also look at the list of paused plugins and be paused accordingly. We explicitly decided that, even though these plugins are network-active, they should still only be paused per site as it might be due to a conflict with another plugin that is only active on that site.
  • As a follow-up to the above two points, in the admin plugins list table, the Resume Execution link must also be added for network-active plugins that are paused.
  • MU-plugins should be skipped entirely from the WSOD protection. We primarily implement this feature for less tech-savvy users, and those should never use MU-plugins, unless it's those default ones from their hosting provider, and those shouldn't cause errors (and otherwise the hosting provider would quickly be aware). MU-plugins often contain critical functionality, which might be security-related and their name indicates that they are must-use. Also, less things to implement for us. We should skip those entirely.
  • The option pause_on_admin should be split into separate options for plugins and themes. pause_on_admin is not an accurate name since there are other protected endpoints too. We suggest having two options of serialized arrays, paused_plugins and paused_themes. Wrapper functions add_paused_plugin( $plugin, $error ), remove_paused_plugin( $plugin ), get_paused_plugins(), add_paused_theme( $theme, $error ), remove_paused_theme( $theme ), get_paused_themes() should be added. Alternatively use set_* and unset_* for the names instead of add_* and remove_*.
  • The main benefit of the above separation and abstraction is the following: A major need for multisite would be a way for a network administrator to be notified/aware of issues caused by plugins, since they are the only ones with the permissions to fix the issues most likely. On multisite, the paused plugins and themes should not be stored in a paused_plugins / paused_themes option - instead the new blogmeta table (i.e. site metadata) should be used, which enables the network admin panel to query:
    • which/how many sites do have paused plugins?
    • which/how many sites do have paused themes?
    • which/how many sites do have plugin x paused?
    • which/how many sites do have theme y paused?
  • The above queries being possible allow us to implement a cron hook that sends a notification email based on the paused plugins and themes on the network to the network administrator/s, with information for each plugin/theme for how many or even which sites are affected.
  • In the future, it might be useful to allow network administrators to automatically resume a plugin for the entire network, which would also be made possible by using blogmeta (site metadata). This would be v2 material though.

I'll wait for a few days to get some initial feedback on the above suggestions, and then will start implementing something along these lines. I should have most of the changes done by next week's meeting so that we can review then - having the code might also help in making a decision.

@flixos90
9 months ago

#37 @flixos90
9 months ago

  • Milestone changed from 5.0 to 5.1

44458.9.diff implements the aforementioned improvements:

  • Site administrators can always resume plugins, even in a multisite.
  • Site administrators can resume network-active plugins for their own site.
  • Network administrators see information in the table if a plugin has been paused for at least 1 site in the network.
  • Network administrators can network-resume plugins that are paused on at least 1 site in the network.
  • Introduce a WP_Paused_Extensions_Storage class that contains the API for storing extension errors and takes care of using site metadata on multisite (instead of an option). Options are now independent (paused_plugins and paused_themes). The two class instances can be retrieved via wp_paused_plugins() and wp_paused_themes() respectively.
  • Move related functions into a new error-protection.php file.
  • Do not handle MU plugins.
  • Fix a couple minor bugs.
  • Adjust version numbers because this is now part of 5.1.

Testing for these changes is much appreciated. For more details, I recommend reviewing the code at https://github.com/wp-core-php/wordpress-develop/pull/3.

With these changes, the patch introduces its first multisite-specific features: Network administrators can now see whether a plugin has caused errors on at least one site in the network. They can also network-resume a plugin that has caused errors somewhere, essentially force-resuming it for all sites. If a site later detects that there are still issues, it will once again be paused on that very site.

Note that, although plugins can be "network-resumed", plugins are still only paused per site, and that is intended.

At the moment, the network administrator can see an information that a plugin has caused a problem on x sites. If the main site is also affected, they also see which error caused it for that site. We should think about how we can improve this, for example in terms of:

  • When a plugin causes an error on another site but the main site, the admin should at least still see one of the errors (especially because often times they will be the same for all sites).
  • We need to keep in mind though, that it's likely not a great idea showing all errors because there might be a large number of sites.
  • We need to think about a mechanism for notifying a network administrator about such errors. At the moment, they will only "randomly" notice it by accessing the Network Plugins screen.

@flixos90
9 months ago

Example with (manually) broken "Hello Dolly" plugin in the Site Admin

@flixos90
9 months ago

Example with (manually) broken "Hello Dolly" plugin in the Network Admin

This ticket was mentioned in Slack in #core-php by flixos90. View the logs.


9 months ago

#39 @afragen
8 months ago

  • Keywords servehappy added

@flixos90
7 months ago

#40 @flixos90
7 months ago

44458.10.diff is an update without any changes other than working against current trunk without issues.

This ticket was mentioned in Slack in #core-php by flixos90. View the logs.


7 months ago

This ticket was mentioned in Slack in #core-multisite by flixos90. View the logs.


7 months ago

#43 in reply to: ↑ 12 @vizkr
7 months ago

Replying to schlessera:

I can see this project becoming really big and complex for a single patch and commit. In order to maintain a good overview as well as easy workflow, I think it would be a good idea to move work on this over to a GitHub fork of WordPress. Then we can handle the respective integrations with themes, multisite and possibly MU plugins via separate branches/PRs.

I'm not sure that is the case. I think right now, we cover already 60-70% of the needed code. Half of the multisite plugin stuff is already covered, and themes can mostly reuse the same code.

I would advise against this, for three reasons:

  1. MU plugins often contain very foundational functionality of a website.
  2. Someone who uses MU plugins usually has a little more technical knowledge than who the safe mode is most beneficial for.
  3. There is no way to manage MU plugins through the admin. Of course we could introduce a way to resume a MU plugin, but for the two previous reasons, I don't think it's worth the hassle.

I'm not entirely sure these are valid reasons.

  1. Yes, but a parse error means they just take down the site either way. The importance of the plugin is irrelevant in that case.
  2. Sometimes MU plugins ar not installed by the site owner themselves, but by hosting or another plugin.
  3. This is the only argument that plays a role here I think. What's the point of making people enter th eplugin screen if they have no valid options. I wonder though (not tested yet) whether they could modify the MU plugin through the built in code editor...

This is a valid point. Most mu-plugins I use are installed by me directly, however WP Engine does include their own suite and WP Migrate DB Pro also installs a mu-plugin of their own. However in my opinion this is a bonus item and probably not a MVP requirement.

For now, in my opinion we should try to focus on detecting in the most accurate way possible which plugin(s) or theme(s) are causing the error. An AJAX loop is what I thought about too.

Yes, agree.

A general suggestion for improvement I have is that we should consider keeping the user in the backend when they are already there while something breaks. For example, if you're in the Plugins screen, resuming a plugin, and then it's still broken, it would be great if the user could automatically stay on that page, getting the feedback message there that it was paused again (instead of seeing the message like in the frontend and having to click a link). We could try to redirect the user to the page they're currently on after pausing the broken plugin(s) and theme(s), if that is already a logged-in admin page. Of course this only works under certain circumstances, if we know who the current user is and if we're still before output. This could very well happen a little later, when we improve the UX further.

Yes, I had already thought about adding something like that. It could be similar to the Shiny Updates plugin, where it triggers a sandboxed request to see whether the plugin now works correctly again.

#44 @vizkr
7 months ago

I really like the enhance plugin error data. I am curious though:

What are the performance implications of this change?

Will it be possible to config switch the WSOD feature off on production systems?

This ticket was mentioned in Slack in #core-php by flixos90. View the logs.


7 months ago

#46 @spacedmonkey
7 months ago

I have left a couple of comments on the PR here.

My biggest issue, the LIKE meta query here. This is because you store don't store a list of paused plugins, in site meta as well.

I would store a list of plugins, using the same key but multiple values. By doing this, you wouldn't have to do a LIKE or this loop.

The only other major feedback, is site meta check here.

@flixos90
7 months ago

#48 @flixos90
7 months ago

44458.11.diff fixes a few things and includes first steps to adding support for pausing and resuming themes. As usual, a better overview about the changes is available via https://github.com/wp-core-php/wordpress-develop/pull/3.

  • Fix a few minor issues @spacedmonkey pointed out on the PR.
  • Centralize logic to retrieve active themes in a new function wp_get_active_and_valid_themes(). If on a protected endpoint, paused themes are filtered out, just like plugins are in wp_get_active_and_valid_plugins(). The theme's functios.php file/s is/are loaded based on this function's result.
  • Introduce a wp_using_themes() function, that relies on WP_USE_THEMES, but in addition makes that filterable. If both parent and child theme are paused, this will be filtered to false on a protected endpoint so that no template loads in such a case (by default there are no protected endpoints in the frontend, but that is customizable, so we need to account for it).
  • Display an admin notice when a theme is paused.
  • Ensure a WP_Theme instance contains an error theme_paused if that theme is paused.
  • In wp-admin/themes.php, show paused themes as broken (via the last bullet point) and introduce an action link to resume them. When clicked, the theme is resumed appropriately.

This is a start for supporting paused themes. It probably needs a few improvements and testing, especially for multisite.

#49 @spacedmonkey
7 months ago

I have added some more comments to the pr, for your review.

Looking back on the messages, I disagree on the mu-plugins. A network activated plugin and mu-plugin are basically the same in my mind. We disable network activated plugins and not mu-plugins are weird. You are right mu-plugins and normally added by hosting companies, but if you host pushes at a bad mu-plugin or an upgrade to PHP 7.3 breaks a working mu-plugin, we have the possibility to handle that error and make sure people can use they site and not WSOD.

A similar agreement could be made for drop-ins...

#50 @desrosj
7 months ago

  • Keywords needs-dev-note added

Just marking this needs-dev-note so we remember to publish a Make post for this feature.

This ticket was mentioned in Slack in #core-php by flixos90. View the logs.


6 months ago

@flixos90
6 months ago

#52 @flixos90
6 months ago

  • Keywords needs-unit-tests removed

44458.12.diff introduces a WP_Shutdown_Handler class that represents the shutdown handler. Its handle() method is the only public method and is used as the callback for register_shutdown_function().

Relying a class like this makes it easier to provide a custom implementation via the new shutdown-handler.php drop-in. Since the distinct steps the default handler performs are separate methods, you can easily reuse what you like while overriding the parts you want to change.

The patch also hardens the logic a bit further, for example covering the case where the Translation APIs have not been loaded yet. It also prevents an error where the default error template would use invalid HTML markup. Last but not least, it clarifies documentation, particularly important for the two drop-ins that the WSOD protection will introduce.

As usual, you can get a better overview of this change in particular on GitHub, via https://github.com/wp-core-php/wordpress-develop/pull/3/commits/851cfe7001cf64636cab04d5a23f4fb12a3e1fee.

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

#53 @flixos90
6 months ago

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

#54 @ocean90
6 months ago

In 44458.12.diff, src/wp-admin/themes.php, is a text change from Sorry, you are not allowed to delete this item. to Sorry, you are not allowed to resume execution of this theme.' while the check is for current_user_can( 'delete_themes' ). Also Sorry, you are not allowed to resume this item. should probably be Sorry, you are not allowed to resume execution of this theme..

I'm not sure, but do you think we can drop "execution" from all the UI strings? It has some negative meanings but also "executing a theme" just sounds weird. Just using resume/paused should be good enough.

@flixos90
6 months ago

#55 @flixos90
6 months ago

44458.13.diff adds several small tweaks to make the code more error-proof and stubs __() if not available to avoid having to deal with translation strings in separation from their actual appliance. It also fixes the accidentally replaced translation string and removes usage of the term "execution" in user-facing language, as highlighted in the last comment. This also ensures the new links in the Plugins list tables match the existing ones better.

#56 @flixos90
6 months ago

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

In 44524:

Bootstrap/Load: Introduce fatal error recovery mechanism allowing users to still log in to their admin dashboard.

This changeset introduces a WP_Shutdown_Handler class that detects fatal errors and which extension (plugin or theme) causes them. Such an error is then recorded, and an error message is displayed. Subsequently, in certain protected areas, for example the admin, the broken extension will be paused, ensuring that the website is still usable in the respective area. The major benefit is that this mechanism allows site owners to still log in to their website, to fix the problem by either disabling the extension or solving the bug and then resuming the extension.

Extensions are only paused in certain designated areas. The frontend for example stays unaffected, as it is impossible to know what pausing the extension would cause to be missing, so it might be preferrable to clearly see that the website is temporarily not accessible instead.

The fatal error recovery is especially important in scope of encouraging the switch to a maintained PHP version, as not necessarily every WordPress extension is compatible with all PHP versions. If problems occur now, non-technical site owners that do not have immediate access to the codebase are not locked out of their site and can at least temporarily solve the problem quickly.

Websites that have custom requirements in that regard can implement their own shutdown handler by adding a shutdown-handler.php drop-in that returns the handler instance to use, which must be based on a class that inherits WP_Shutdown_Handler. That handler will then be used in place of the default one.

Websites that would like to modify specifically the error template displayed in the frontend can add a php-error.php drop-in that works similarly to the existing db-error.php drop-in.

Props afragen, bradleyt, flixos90, ocean90, schlessera, SergeyBiryukov, spacedmonkey.
Fixes #44458.

#57 @flixos90
6 months ago

In 44525:

Bootstrap/Load: Fix missing object argument for singular capability checks and support capability tests overlooked in [44524].

See #44458.

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


6 months ago

This ticket was mentioned in Slack in #core-restapi by spacedmonkey. View the logs.


6 months ago

#60 @spacedmonkey
6 months ago

I have created a couple of bugs against this feature.

Related: #45932 #45933

#61 @spacedmonkey
6 months ago

Another bug.

Related: #45958

#62 @flixos90
6 months ago

  • Keywords needs-testing needs-dev-note removed

#63 @WFMattR
6 months ago

I was testing the WP 5.1 beta and found some cases where plugins can be incorrectly paused. Some could be random, due to reaching the PHP memory_limit (depending on varying amounts that core+plugins+themes use in a random hit), or a plugin that only uses excess memory in limited circumstances could get paused even if it won't actually break the login form or wp-admin.

For login and security-related plugins, pausing the plugins could be worse than letting them run normally, especially if the error is only caused by bad input from an attack or a bot. I'm most concerned about security plugins getting paused (e.g., an attacker could exploit the pausing to get a security plugin disabled that's providing the only protection for another installed plugin with a known vulnerability), but it looks like it could also affect other plugins built for subscribers or other non-admins too. Is it possible it could be limited to only parser errors initially, or be an opt-in feature?

(Above is the main question; below are examples of the issues.)

Example issues with plugins being incorrectly paused, including sometimes affecting security:

(I'll attach the sample files referenced below -- I hope this is the right place for these.)

  1. A plugin may be paused because another plugin used a lot of memory. When a site's memory_limit is reached, the plugin that happened to be running at the time can be paused, even if it's not using excessive memory. That might cause security issues, or may just be confusing for the admin, since the paused plugin(s) aren't necessarily the cause of the issue.
  2. Local File Inclusion vulnerabilities in any plugin/theme will give the attacker the ability to pause many plugins at will. When any plugin/theme is vulnerable to "Local File Inclusion (LFI)", an attacker often cannot use that to make changes to the site, but if plugins can be paused by WP 5.1 for redeclaring an existing class, an attacker can choose specific plugins to pause, even if those plugins are not vulnerable. I've included examples for Jetpack, WPS Hide Login, and Akismet, with a demo plugin with a simple LFI vulnerability. (There are over 1100 entries on Exploit DB at www.exploit-db.com when searching "local file inclusion" without quotes -- some are old or are not WP plugins, but it's common enough to be a concern.)
  3. It might be possible that max_execution_time has the same issue as memory_limit. I haven't made a test case yet. On a shared host that is running slowly, or any server under heavy load (including during intentional DoS or brute force attacks), many of the requests could cause timeouts, which could occur in random plugins' code or the theme's code.

Test environment:

  • WordPress nightly build: 5.1-beta1-44575
  • Active plugins: Akismet, Jetpack, WordPress Beta Tester, WP Crontrol, Yoast SEO
  • Twentynineteen theme
  • PHP memory_limit: 128M

Steps to reproduce the memory_limit issue:

  1. This issue requires some tuning of the memory used in the first plugin file, to reproduce it on a given site. It may occur randomly on sites that use memory near the memory_limit.
  2. Install the sample plugins memory-plugin-1.php and memory-plugin-2.php -- this is just to simulate high usage of memory by multiple plugins. The first plugin uses a lot of memory, and the second plugin uses only a little.
  3. Activate the first plugin, and check the error log. The "Memory after allocation" should be a few MB below your site's memory_limit. If it's not, adjust the value in the plugin file define('WASTE_MEMORY_MB', 119);.
  4. Activate the second plugin -- if it runs out of memory and can't even be activated, decrease the memory amount in the first plugin.
  5. Browse around the admin pages, and see if a plugin gets paused. Since the second plugin uses a fairly small amount of memory during a hook, it gets paused instead of the one that uses large amounts of memory.
    • Yoast SEO's "Search Console" page or the WP dashboard usually triggers it for me, but it may vary by site. (There is nothing wrong with either page, but they just use some memory.)
    • Depending on how close you are to the memory limit, some ajax calls may also trigger it. Jobs in wp-cron might also use higher memory at times.

Steps to reproduce LFI example:

  1. Install and activate the "Local File Inclusion Vulnerability Example", a single-file plugin: local-file-inclusion-example.php
  2. Install and activate Jetpack, Akismet, and WPS Hide Login (example plugins to be paused)
  3. Visit URLs below from another browser without logging in -- normally the "Cannot declare class ..." message would only break the attacker's pageload (and in these examples, local file inclusion of this type normally has no effect on the function of the plugins for other users), but in WP 5.1 the attacker can cause these plugins to be paused.
  4. Visit example.com/?lfi_example=wp-content/plugins/jetpack/class.jetpack.php
    • Jetpack is paused in the backend
    • Message: The plugin caused an error of type E_ERROR in line 27 of the file /home/wp/public_html/wp-5.1/wp-content/plugins/jetpack/class.jetpack.php. Error message: Cannot declare class Jetpack, because the name is already in use
  5. Visit example.com/?lfi_example=wp-content/plugins/akismet/class.akismet.php
    • Akismet is paused in the backend
    • Message: The plugin caused an error of type E_ERROR in line 3 of the file /home/wp/public_html/wp-5.1/wp-content/plugins/akismet/class.akismet.php. Error message: Cannot declare class Akismet, because the name is already in use
  6. Visit example.com/?lfi_example=wp-content/plugins/wps-hide-login/vendor/composer/ClassLoader.php
    • WPS Hide Login is paused in the back end, including reverting the custom login page, so although this plugin normally changes the login URL, wp-login.php is accessible again
    • Message: The plugin caused an error of type E_ERROR in line 43 of the file /home/wp/public_html/wp-5.1/wp-content/plugins/wps-hide-login/vendor/composer/ClassLoader.php. Error message: Cannot declare class Composer\Autoload\ClassLoader, because the name is already in use

@WFMattR
6 months ago

Test plugin that uses a lot of memory

@WFMattR
6 months ago

Test plugin that uses a little memory, after plugins have loaded

@WFMattR
6 months ago

Test plugin that has a sample Local File Inclusion bug - do not install on a production site

#64 @flixos90
6 months ago

In 44622:

Bootstrap/Load: Use wp_load_translations_early() when Translation API is not yet loaded.

Props spacedmonkey.
Fixes #45932. See #44458.

#65 @flixos90
6 months ago

In 44623:

Bootstrap/Load: Only pause extensions when they cause a crash on a protected endpoint.

This is a first step on pausing extensions less aggressively. If a plugin or theme only causes a crash in the frontend, there is no point in pausing it in the admin backend.

See #45940, #44458.

#66 @flixos90
6 months ago

In 44624:

Bootstrap/Load: Fix workaround to display admin link in PHP error template by introducing $link_url and $link_text arguments to wp_die().

This changeset removes the hack that was used before to display more complex HTML markup than a simple message in the default PHP error template via wp_die(). By removing HTML markup from the arguments passed to wp_die() it furthermore paves the way for supporting other content types than the default.

The message and arguments can be modified with new wp_php_error_message and wp_php_error_args filters respectively.

Furthermore this changeset fixes a few issues of functions not existing which could potentially have caused fatal errors when executed early in the WordPress bootstrap process.

Props flixos90, spacedmonkey.
See #45933, #44458.

#67 @flixos90
6 months ago

In 44625:

Bootstrap/Load: Add support for JSON requests to wp_die().

In addition to AJAX and XML-RPC requests, wp_die() now handles JSON requests correctly, returning information in the expected content type.

Props spacedmonkey.
See #45933, #44458.

#68 @desrosj
6 months ago

  • Keywords has-dev-note added

#69 @flixos90
6 months ago

In 44666:

Bootstrap/Load: Support WP_Error and $args passed to wp_die() consistently in all handlers.

Prior to this change, each wp_die() handler had their own logic for how to parse arguments, causing inconsistencies and even breakage because the arguments possible to pass to wp_die() depended on the request context. Passing a WP_Error as $message for example used to be only support by the default handler, but not the AJAX and XML-RPC handlers.

With the fatal error protection, plus the new wp_die() handlers related to that, improving this support and compatibility has become more significant. Therefore this changeset introduces a private _wp_die_process_input() function that handles all function parameters consistently.

Props spacedmonkey, flixos90, schlessera.
Fixes #45933. See #44458.

#70 @flixos90
6 months ago

In 44671:

Bootstrap/Load: Ensure that the fatal error shutdown handler does not prevent other shutdown handlers from being called.

This changeset adds support for a new wp_die() argument exit, which defaults to true and determines whether wp_die() should actually terminate the request. The new fatal error handler then calls wp_die() with that argument set to false, as calling die() or exit from a PHP shutdown function prevents other shutdown functions from being called.

Props schlessera, johnbillion.
Fixes #46038. See #44458.

#71 @flixos90
6 months ago

In 44674:

Bootstrap/Load: Change shutdown handler naming to final fatal error handler and allow disabling the handler entirely via a constant.

The WP_Shutdown_Handler name plus related function names were premature when originally committed, as there can be multiple shutdown handlers in PHP, and WordPress makes use of that feature. This changeset modifies the name to a more appropriate WP_Fatal_Error_Handler, and related to that changes the following names:

  • The drop-in to override the handler is now called fatal-error-handler.php.
  • The internal function wp_register_premature_shutdown_handler is now called wp_register_fatal_error_handler().

In addition to these naming changes, a new constant WP_DISABLE_FATAL_ERROR_HANDLER is introduced that can be set in wp-config.php to entirely disable the fatal error handler. That constant's value is and should be accessed indirectly via a new wp_is_fatal_error_handler_enabled() function and is filterable via a new wp_fatal_error_handler_enabled hook. Note that disabling the fatal error handler will skip the new functionality entirely, including the potentially used fatal-error-handler.php drop-in.

The new set of constant, filter and function provide for an easier-to-use mechanism to disable the fatal error handler altogether, rather than requiring developers to implement a drop-in for purely that purpose.

Props afragen, flixos90, joyously, knutsp, markjaquith, ocean90, schlessera, spacedmonkey.
Fixes #46047. See #44458.

#72 @flixos90
6 months ago

In 44706:

Bootstrap/Load: Fix notice when theme directories are not populated yet.

When a fatal error occurs outside of a regular plugin in a stage where theme directories are not set up yet (for example in a MU plugin), this would previously trigger a notice.

Props johnbillion.
Fixes #46068. See #44458.

#73 @desrosj
6 months ago

The site metadata aspect of this ticket was referenced in another dev note: https://make.wordpress.org/core/2019/01/28/multisite-support-for-site-metadata-in-5-1/.

#74 @flixos90
6 months ago

In 44717:

Bootstrap/Load: Revert fatal error recovery mechanism from 5.1 to polish for 5.2.

Due to the high number of follow-up tickets and associated security concerns, it was decided to reschedule the fatal error recovery feature for WordPress 5.2, in order to address these issues properly. The feature will continue to be developed, with iterations being merged into trunk early in the 5.2 release cycle.

Fixes #46141. See #44458, #45932, #45940, #46038, #46047, #46068.

#75 @flixos90
6 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

#76 @flixos90
6 months ago

  • Milestone changed from 5.1 to 5.2

#77 @flixos90
6 months ago

https://github.com/wp-core-php/wordpress-develop/pull/5 is a new PR that contains the WSOD code in the current stage in which it can theoretically be committed back to trunk later for 5.2. That commit itself would "re-close" #44458, #45932, #46047, and #46068.

The additional efforts would then go towards #46130.

#78 @flixos90
4 months ago

  • Milestone changed from 5.2 to 5.3

#79 @flixos90
4 months ago

  • Milestone changed from 5.3 to 5.2

#80 @flixos90
4 months ago

In 44962:

Bootstrap/Load: Introduce fatal error handler.

This changeset introduces a WP_Fatal_Error_Handler class that detects fatal errors and displays a more user-friendly message about the site experiencing technical difficulties.

Websites that have custom requirements in that regard can implement their own fatal error handler by adding a fatal-error-handler.php drop-in that returns the handler instance to use, which must be based on a class that inherits WP_Fatal_Error_Handler. That handler will then be used in place of the default one. Alternatively, the fatal error handler feature can be completely disable through a constant WP_DISABLE_FATAL_ERROR_HANDLER.

Websites that would like to modify specifically the error template displayed in the frontend can add a php-error.php drop-in that works similarly to the existing db-error.php drop-in. For more granular customization, the fatal error handler also includes new filters wp_should_handle_php_error, wp_php_error_message and wp_php_error_args.

Props afragen, bradleyt, flixos90, ocean90, schlessera, SergeyBiryukov, spacedmonkey, timothyblynjacobs.
See #46130, #44458.

#81 @flixos90
4 months ago

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

In 44973:

Bootstrap/Load: Introduce a recovery mode for fixing fatal errors.

Using the new fatal handler introduced in [44962], an email is sent to the admin when a fatal error occurs. This email includes a secret link to enter recovery mode. When clicked, the link will be validated and on success a cookie will be placed on the client, enabling recovery mode for that user. This functionality is executed early before plugins and themes are loaded, in order to be unaffected by potential fatal errors these might be causing.

When in recovery mode, broken plugins and themes will be paused for that client, so that they are able to access the admin backend despite of these errors. They are notified about the broken extensions and the errors caused, and can then decide whether they would like to temporarily deactivate the extension or fix the problem and resume the extension.

A link in the admin bar allows the client to exit recovery mode.

Props timothyblynjacobs, afragen, flixos90, nerrad, miss_jwo, schlessera, spacedmonkey, swissspidy.
Fixes #46130, #44458.

#82 @SergeyBiryukov
4 months ago

In 45014:

Bootstrap/Load: Always run the fatal error handler at shutdown, but don't display the PHP error template once headers are sent.

If a fatal error occurs midway through a page load, or in a REST API request, it still needs to be handled internally for the recovery mode, but the custom message may conflict with already rendered output, e.g. by displaying HTML markup in an XML or JSON request.

Props spacedmonkey, flixos90, TimothyBlynJacobs.
Fixes #45989. See #44458.

#83 @SergeyBiryukov
4 months ago

In 45015:

Bootstrap/Load: Add support for JSONP requests to wp_die().

In addition to AJAX, XML-RPC, and JSON requests, wp_die() now handles JSONP requests correctly, returning information in the expected content type.

Props spacedmonkey, TimothyBlynJacobs.
Fixes #46025. See #44458.

#84 @SergeyBiryukov
4 months ago

In 45016:

Bootstrap/Load: Add support for XML requests to wp_die().

In addition to AJAX, XML-RPC, JSON, and JSONP requests, wp_die() now handles XML requests correctly, returning information in the expected content type.

Props spacedmonkey, birgire.
Fixes #46026. See #44458.

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


4 months ago

#86 @johnbillion
3 months ago

In 45125:

Bootstrap/Load: Ignore wp-content/fatal-error-handler.php and wp-content/php-error.php from version control.

See #44458

#87 @spacedmonkey
3 weeks ago

  • Component changed from Bootstrap/Load to Site Health
Note: See TracTickets for help on using tickets.