-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Handle errors emitted while applying wp_template_enhancement_output_buffer filters doing the wp_finalized_template_enhancement_output_buffer action
#10310
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
…into trac-64108-errors-emitted-during-output-buffer-callback
…into trac-64108-errors-emitted-during-output-buffer-callback
wp_template_enhancement_output_buffer filterswp_template_enhancement_output_buffer filters doing the wp_finalized_template_enhancement_output_buffer action
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
src/wp-includes/template.php
Outdated
| * @param string $output Original HTML template output buffer. | ||
| */ | ||
| $filtered_output = (string) apply_filters( 'wp_template_enhancement_output_buffer', $filtered_output, $output ); | ||
| } catch ( Exception $exception ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would you want to catch ( Throwable $throwable ) here to catch exceptions, errors, and all kinds of throwable things?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I suppose so. Although the error hander is already catching user errors. Would there be any other throwable thing?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I think I made the change you had in mind: 2d9bdd4
src/wp-includes/template.php
Outdated
| default: | ||
| $type = 'Error'; | ||
| } | ||
| $format = "<br />\n<b>%s</b>: %s in <b>%s</b> on line <b>%d</b><br />"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can’t pass it up: the / on the <br /> is a benign misunderstanding and back-reading of XML into HTML. it’s ignored in HTML and its presence doesn’t turn HTML into “safe” XHTML. it’s only a concession to the widespread practice of adding these self-closing flags to HTML void elements that HTML5 didn’t codify them as actual syntax errors.
the concept didn’t exist until XML, which came after HTML.
just thoughts for anyone who sees them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right. I'm only including it here because it's what PHP outputs. I tried to exactly replicate the native PHP error printing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've made it explicit why those self-closing tags are being used in 2fe5542
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no worries 😄
I have a personal mission to remove them from everywhere, but those aren’t blocking comments, just stubborn idealism.
src/wp-includes/template.php
Outdated
| } | ||
| $format = "<br />\n<b>%s</b>: %s in <b>%s</b> on line <b>%d</b><br />"; | ||
| if ( ! ini_get( 'html_errors' ) ) { | ||
| $format = strip_tags( $format ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would we not want to use wp_strip_all_tags() here, or use some HTML API processor to get something as close to innerText as we can?
echo strip_tags( 'The <em>dog</em> walked & barked.' );
// The dog walked & barked.or is the goal to try and prevent the possibility of throwing through some user-space bug?
of note, strip_tags() does not parse HTML particularly well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately wp_strip_all_tags() also does trim() at the end, so then it removes the initial newline.
The goal here is just to emulate what PHP outputs when the html_errors setting is turned off.
I'm only looking to remove the B and BR tags from the format string. It's not operating on the underlying error message being printed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've eliminated this in 2fe5542
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this reads clearer to me, and removes the complications of misparsing and leaving character references un-decoded.
src/wp-includes/template.php
Outdated
| } | ||
| ); | ||
| $display_errors = ini_get( 'display_errors' ); | ||
| if ( $display_errors ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if this is stderr is it a problem for the output buffering?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, interesting. I wasn't aware that was an option. I suppose it should be then be:
| if ( $display_errors ) { | |
| if ( $display_errors && 'stderr' !== $display_errors ) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, I just tried this and it still is going to the frontend:
add_action( 'init', function () {
ini_set( 'display_errors', 'stderr' );
} );
add_action( 'wp_footer', function () {
trigger_error( 'You are not doing this right!!', E_USER_WARNING );
} );Result:
...
</svg>
<br />
<b>Warning</b>: You are not doing this right!! in <b>/var/www/src/wp-content/plugins/try-display-errors-stderr.php</b> on line <b>11</b><br />
<script type="importmap" id="wp-importmap">
...This is in the wordpress-develop environment.
However, when I use PHP's built-in web server, when run this PHP coe:
ini_set( 'display_errors', 'stderr' );
trigger_error( 'You are not doing this right!!', E_USER_WARNING );The result is that the error is not emitted to the frontend. It is sent to the terminal after the regular log entry:
[Sat Nov 1 19:59:25 2025] PHP Warning: You are not doing this right!! in /private/tmp/try-stderr/try.php on line 4
[Sat Nov 1 19:59:25 2025] PHP Stack trace:
[Sat Nov 1 19:59:25 2025] PHP 1. {main}() /private/tmp/try-stderr/try.php:0
[Sat Nov 1 19:59:25 2025] PHP 2. trigger_error($message = 'You are not doing this right!!', $error_level = 512) /private/tmp/try-stderr/try.php:4
<br />
<font size='1'><table class='xdebug-error xe-warning' dir='ltr' border='1' cellspacing='0' cellpadding='1'>
<tr><th align='left' bgcolor='#f57900' colspan="5"><span style='background-color: #cc0000; color: #fce94f; font-size: x-large;'>( ! )</span> Warning: You are not doing this right!! in /private/tmp/try-stderr/try.php on line <i>4</i></th></tr>
<tr><th align='left' bgcolor='#e9b96e' colspan='5'>Call Stack</th></tr>
<tr><th align='center' bgcolor='#eeeeec'>#</th><th align='left' bgcolor='#eeeeec'>Time</th><th align='left' bgcolor='#eeeeec'>Memory</th><th align='left' bgcolor='#eeeeec'>Function</th><th align='left' bgcolor='#eeeeec'>Location</th></tr>
<tr><td bgcolor='#eeeeec' align='center'>1</td><td bgcolor='#eeeeec' align='center'>0.0001</td><td bgcolor='#eeeeec' align='right'>359408</td><td bgcolor='#eeeeec'>{main}( )</td><td title='/private/tmp/try-stderr/try.php' bgcolor='#eeeeec'>.../try.php<b>:</b>0</td></tr>
<tr><td bgcolor='#eeeeec' align='center'>2</td><td bgcolor='#eeeeec' align='center'>0.0001</td><td bgcolor='#eeeeec' align='right'>359816</td><td bgcolor='#eeeeec'><a href='http://www.php.net/function.trigger-error' target='_new'>trigger_error</a>( <span>$message = </span><span>'You are not doing this right!!'</span>, <span>$error_level = </span><span>512</span> )</td><td title='/private/tmp/try-stderr/try.php' bgcolor='#eeeeec'>.../try.php<b>:</b>4</td></tr>
</table></font>
So there is an inconsistency here.
It seems better safe to not emit in the case of stderr than to assume it is OK. So I've updated the logic in 5b13a71 to not append the errors when it is stderr.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there might be some difference when running as php -ddisplay_errors=stderr than when calling ini_set( 'display_errors', 'stderr' ) based on a note in the PHP manual page, but I think that’s only saying we don’t trap fatals because we’re catching them in the environment in which we’re throwing them, so probably doesn’t impact where the output is set. it wold not be the first time I’ve found the PHP manual page to be incomplete or incorrect in its assertions though.
I wish I understood the potential values here better. Seems like there are only two modes: STDOUT and STDERR, and you made the proper detection, because the INI value is either exactly stderr (in which case it uses the STDERR mode) or it’s anything else in which case it’s STDOUT.
https://github.com/php/php-src/blob/a979e9f897a90a580e883b1f39ce5673686ffc67/main/main.c#L489-L518
…ing/error_append_string
Co-authored-by: Dennis Snell <dmsnell@git.wordpress.org>
Co-authored-by: Dennis Snell <dmsnell@git.wordpress.org>
…into trac-64108-errors-emitted-during-output-buffer-callback
|
@dmsnell Are you happy to approve the changes here? I had Gemini CLI do a review, and it gave me the green light: The changes introduce robust error handling for the
Conclusion: The changes are well-implemented, adhere to WordPress coding standards, and significantly improve the robustness of the template enhancement output buffering mechanism. The accompanying tests provide good coverage for the new error handling logic. |
|
Since I believe all concerns have been addressed, I'm going ahead and committing this for beta3. Any additional feedback I can address over the next week. |
…ation of the template enhancement output buffer. When `display_errors` (`WP_DEBUG_DISPLAY`) is enabled, errors (including notices, warnings, and deprecations) that are triggered during the `wp_template_enhancement_output_buffer` filter or the `wp_finalized_template_enhancement_output_buffer` action have not been displayed on the frontend since they are emitted in an output buffer callback. Furthermore, as of PHP 8.5 attempting to print anything in an output buffer callback causes a deprecation notice. This introduces an error handler and try/catch block to capture any errors and exceptions that occur during these hooks. If `display_errors` is enabled, these captured errors are then appended to the output buffer so they are visible on the frontend, using the same internal format PHP uses for printing errors. Any exceptions or user errors are converted to warnings so that the template enhancement buffer is not prevented from being flushed. Developed in #10310 Follow-up to [61111], [61088], [60936]. Props westonruter, dmsnell. See #43258, #64126. Fixes #64108. git-svn-id: https://develop.svn.wordpress.org/trunk@61120 602fd350-edb4-49c9-b593-d223f7449a82
…ation of the template enhancement output buffer. When `display_errors` (`WP_DEBUG_DISPLAY`) is enabled, errors (including notices, warnings, and deprecations) that are triggered during the `wp_template_enhancement_output_buffer` filter or the `wp_finalized_template_enhancement_output_buffer` action have not been displayed on the frontend since they are emitted in an output buffer callback. Furthermore, as of PHP 8.5 attempting to print anything in an output buffer callback causes a deprecation notice. This introduces an error handler and try/catch block to capture any errors and exceptions that occur during these hooks. If `display_errors` is enabled, these captured errors are then appended to the output buffer so they are visible on the frontend, using the same internal format PHP uses for printing errors. Any exceptions or user errors are converted to warnings so that the template enhancement buffer is not prevented from being flushed. Developed in WordPress/wordpress-develop#10310 Follow-up to [61111], [61088], [60936]. Props westonruter, dmsnell. See #43258, #64126. Fixes #64108. Built from https://develop.svn.wordpress.org/trunk@61120 git-svn-id: http://core.svn.wordpress.org/trunk@60456 1a063a9b-81f0-0310-95a4-ce76da25c4cd
…ation of the template enhancement output buffer. When `display_errors` (`WP_DEBUG_DISPLAY`) is enabled, errors (including notices, warnings, and deprecations) that are triggered during the `wp_template_enhancement_output_buffer` filter or the `wp_finalized_template_enhancement_output_buffer` action have not been displayed on the frontend since they are emitted in an output buffer callback. Furthermore, as of PHP 8.5 attempting to print anything in an output buffer callback causes a deprecation notice. This introduces an error handler and try/catch block to capture any errors and exceptions that occur during these hooks. If `display_errors` is enabled, these captured errors are then appended to the output buffer so they are visible on the frontend, using the same internal format PHP uses for printing errors. Any exceptions or user errors are converted to warnings so that the template enhancement buffer is not prevented from being flushed. Developed in WordPress/wordpress-develop#10310 Follow-up to [61111], [61088], [60936]. Props westonruter, dmsnell. See #43258, #64126. Fixes #64108. Built from https://develop.svn.wordpress.org/trunk@61120 git-svn-id: https://core.svn.wordpress.org/trunk@60456 1a063a9b-81f0-0310-95a4-ce76da25c4cd
dmsnell
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@westonruter sorry I couldn’t attend to this earlier. thanks for working through it. there’s nothing I have to add here, but I can no longer approve it since it’s closed/merged.
Look forward to the improved error-handling though. 🙇♂️
mindctrl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Late to submit but just a small phpdoc suggestion.
| } | ||
|
|
||
| /** | ||
| * Data provider for data_provider_to_test_wp_finalize_template_enhancement_output_buffer_with_errors_while_processing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| * Data provider for data_provider_to_test_wp_finalize_template_enhancement_output_buffer_with_errors_while_processing. | |
| * Data provider for test_wp_finalize_template_enhancement_output_buffer_with_errors_while_processing. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Committed in r61140
| ); | ||
| $original_display_errors = ini_get( 'display_errors' ); | ||
| if ( $original_display_errors ) { | ||
| ini_set( 'display_errors', 0 ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this nit matters, so asking here before creating a new ticket +PR, but officially for PHP <8.1, the second param should be a string: https://www.php.net/manual/en/function.ini-set.php
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, thanks. Yeah, good point. Go ahead and open a PR.


When
display_errorsis enabled, this temporarily turns off that setting and then adds an error handler to capture errors while applyingwp_template_enhancement_output_bufferfilters or doing thewp_finalized_template_enhancement_output_bufferaction. After hooks have fired, it then appends any captured errors to the buffer. This fixes an issue where errors emitted during an output buffer callback are suppressed from the output which even though a site may haveWP_DEBUG_DISPLAYenabled. In PHP 8.5, a deprecation notice is shown but the emitted errors are not displayed.When an error is emitted in thewp_finalized_template_enhancement_output_bufferaction, the error will be sent to the error log; it won't ever be printed (even ifdisplay_errorsis enabled) because the output has been finalized.Trac ticket: https://core.trac.wordpress.org/ticket/64108
Follow-up to r61111 and r61088.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.