wpcomsh: bail to core's default fatal screen on memory-exhaustion fatals#50086
wpcomsh: bail to core's default fatal screen on memory-exhaustion fatals#50086arthur791004 wants to merge 1 commit into
Conversation
|
Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.
Interested in more tips and information?
|
|
Thank you for your PR! When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:
This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖 Follow this PR Review Process:
If you have questions about anything, reach out in #jetpack-developers for guidance! Wpcomsh plugin:
If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack. |
Code Coverage SummaryThis PR did not change code coverage! That could be good or bad, depending on the situation. Everything covered before, and still is? Great! Nothing was covered before? Not so great. 🤷 |
…errors When the underlying fatal is a PHP OOM, the request is already at its memory ceiling. Building the branded screen (textdomain load, plugin-header reads, user bootstrap, output buffering) needs more memory than remains, so it re-fatals mid-render: the visitor gets a blank page, the re-fatal is logged against fatal-error-screen.php (masking the real culprit in logstash), and a PHP OOM is not a catchable Throwable so the render can't be guarded. On OOM, raise memory_limit a bounded amount for headroom, then run the normal path unchanged so the admin still sees the error and the logstash event still fires. If the platform refuses the raise, return core's default screen (PHP still logs the fatal) instead of blanking out. A direct ini_set is used rather than wp_raise_memory_limit(), which runs filters in the fatal path and no-ops when the limit is already high (as on Atomic). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
93b7b35 to
ccbdc27
Compare
Fixes #
Proposed changes
When the underlying fatal is a PHP memory-exhaustion error (OOM), the request is already at its memory ceiling. The wpcomsh branded fatal-error screen hooks
wp_php_error_messageand does memory-heavy work (textdomain load, plugin-header reads viaglob/get_plugin_data, user bootstrap,ob_start), so on an OOM it re-fatals mid-render, which:fatal-error-screen.php, masking the real culprit in logstash (this is what produced the fatal noise in the escalation).Key constraint: a PHP OOM is not a catchable
Throwable, so the render can't be wrapped intry/catchand recovered — the only way to let the screen render at the memory ceiling is to create headroom first.This PR adds a single guard at the top of
wpcomsh_customize_fatal_error_message():wpcomsh_fatal_is_oom()— detects memory-exhaustion fatals (bothAllowed memory size of N bytes exhaustedandOut of memory).wpcomsh_fatal_raise_memory_limit()— bumpsmemory_limitby a bounded +32 MB viaini_set(which takes effect inside the fatal handler) and returns whether headroom is available.Behavior by case:
wpcomsh_fatal_signaturelogstash event still fires.ini_setcapped/disabled)Notes:
$messageis only core's generic wrapper HTML ("There has been a critical error on this website.") — no trace/error info — so returning it in the fallback loses nothing; the real error lives in$error(shown to admins) and PHP's fatal log.ini_setis used rather thanwp_raise_memory_limit(), which runsapply_filtersin the fatal path and no-ops when the limit is already high (as on Atomic).Related product discussion/links
Does this pull request change what data or activity we track or use?
No new data is collected. In the rare fallback case (OOM where
memory_limitcan't be raised), our customwpcomsh_fatal_signaturelogstash event does not fire — but the actual fatal is still logged to logstash by PHP regardless, and for an OOM that signature would be unreliable anyway (the error file is "wherever the next allocation landed", not the real cause).Testing instructions
On an Atomic / wpcomsh test site (or local wpcomsh install):
Reproduce the regression (before this change):
trunkyou get a blank page, and logstash records the fatal againstwpcom-fatal-error/fatal-error-screen.php.Verify the fix (with this branch):
wpcomsh_fatal_signaturelogstash event still fires.fatal-error-screen.php.Regression check — non-OOM fatals are unchanged:
add_action( 'init', function () { undefined_function_xyz(); } );