#59652 closed task (blessed) (fixed)
Replace trigger_error() with wp_trigger_error() for 6.6
Reported by: |
|
Owned by: |
|
---|---|---|---|
Milestone: | 6.6 | Priority: | normal |
Severity: | normal | Version: | |
Component: | General | Keywords: | has-patch commit |
Focuses: | Cc: |
Description
This ticket tracks an initiative to replace each instance of trigger_error()
with wp_trigger_error()
. The initiative seeks to avoid generating E_USER family errors unless WP_DEBUG
is on. In doing so, users should not see these messages in normal production.
Previous releases:
- #59455 (6.4)
References:
Change History (14)
This ticket was mentioned in Slack in #core by rajinsharwar. View the logs.
12 months ago
#3
@
12 months ago
- Milestone changed from 6.5 to 6.6
- Summary changed from Replace trigger_error() with wp_trigger_error() for 6.5 to Replace trigger_error() with wp_trigger_error() for 6.6
This ticket was mentioned in PR #6621 on WordPress/wordpress-develop by @prasadkarmalkar.
9 months ago
#4
- Keywords has-patch added
#5
@
9 months ago
Hi @swissspidy , I have Raised PR for this issue. In this PR I have replaced trigger_error with wp_trigger_error and for the function name first parameter in wp_trigger_error.
#6
@
9 months ago
- Keywords changes-requested added
Hi @prasadkarmalkar, thanks for the PR. As you might be aware, wp_trigger_error can also return HTML tags, so we need to strip the tags before using it. That's the reason the unit tests are failing.
EG:
wp_strip_all_tags( wp_trigger_error( __FUNCTION__, $error_message ) );
@thelovekesh commented on PR #6621:
9 months ago
#7
Hi @Rajinsharwar!
Is there a specific reason to strip HTML tags from triggered error messages? I believe the HTML tags are expected, allowing the front end to render the error messages makeup(e.g. links) appropriately for the user.
Regarding the failed tests, wp_trigger_error()
includes the method or function name in the error message, which is causing the tests to fail.
_Please note, how function/method name is being appended in the front of error message._
I believe the test cases need to be updated to account for the __FUNCTION__
name prefix if it is being passed to wp_trigger_error()
.
#8
@
9 months ago
I have Updated test cases for wp_opcache_invalidate_directory and clean_dirsize_cache
@hellofromTonya commented on PR #6621:
8 months ago
#9
Hello and thanks for working on this effort 👋 Some of the changes are in external packages / libraries that are maintained outside of Core, e.g. Requests, PHPMailer, SimplePie, etc.
For this first round of replacements (especially this late in the beta cycle), I'm thinking to only replace within Core's maintained codebase. For these external libraries, discussion can happen whether in their repo (such as with Requests) or within 6.7+ trac ticket.
I'll push a change to this PR shortly that reverts the changes to external libraries. And then review the remaining changes for 6.6 commit consideration.
#10
@
8 months ago
- Owner set to hellofromTonya
- Status changed from new to reviewing
Patch: https://github.com/WordPress/wordpress-develop/pull/6621
Self-assigning ownership for review and 6.6 commit consideration.
@hellofromTonya commented on PR #6621:
8 months ago
#11
I did a force push because it easiest and most clean way for me to restore the changes in external libraries from the original first commit https://github.com/WordPress/wordpress-develop/pull/6621/commits/c99f02736e745cddc5e23cea572a2d8a542d5d2b. Also rebased it on top of the current WP trunk
just in case.
#12
@
8 months ago
- Keywords commit added; changes-requested removed
Patch: https://github.com/WordPress/wordpress-develop/pull/6621
Ready for commit. Preparing it now.
@hellofromTonya commented on PR #6621:
8 months ago
#14
Thank you everyone for your contributions 🌟
Committed via https://core.trac.wordpress.org/changeset/58409 to ship in 6.6.
In this PR I have replaced trigger_error with wp_trigger_error and for the function name first parameter in wp_trigger_error I have used METHOD and FUNCTION based on either this is in function or class
Trac ticket: https://core.trac.wordpress.org/ticket/59652