Make WordPress Core

Opened 2 years ago

Last modified 13 months ago

#55414 new enhancement

Filter error suppression for individual DB query errors

Reported by: bpayton's profile bpayton Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 5.9.2
Component: General Keywords: has-patch has-unit-tests needs-dev-note
Focuses: Cc:

Description

Sometimes it is useful to suppress certain DB errors while logging others.

For example, we have a set of standalone, core-like sites on WordPress.com where users can install arbitrary plugins and themes, and sometimes there are situations where query errors from third-party plugins flood a site's error logs. In some of those cases, the source of the error is not straightforward to address, and we would like to silence or throttle the errors so the site can continue using a given plugin without consuming its available disk space.

Targeted suppression would be possible if WordPress supported a suppress_query_error filter that receives:

  • the current error suppression setting, defaulting to $wpdb->suppress_errors
  • the raw error string
  • the DB query associated with the error

The result of filtering would determine whether $wpdb->print_error() returns early or logs the error, like $wpdb->suppress_errors does today.

I have a patch for this and plan to submit a PR shortly after creating this ticket.

Change History (19)

This ticket was mentioned in PR #2426 on WordPress/wordpress-develop by brandonpayton.


2 years ago
#1

  • Keywords has-patch has-unit-tests added

This PR adds a suppress_query_error filter that can be used to selectively suppress or un-suppress logging of a specific query error. The motivation for adding this filter is to allow logging of most query errors while completely suppressing (or maybe just throttling) specific query errors due to third-party plugins and themes that are not easily fixed or changed. But the flexibility offered by this filter will likely be useful for other purposes as well.

This PR also adds a log_query_error action for the purposes of testing the filter, but this also could be useful to customize how query errors are logged or relayed.

Trac ticket: https://core.trac.wordpress.org/ticket/55414

#2 follow-up: @bpayton
2 years ago

Created a PR here:
https://github.com/WordPress/wordpress-develop/pull/2426

This is the first time I've posted a PR instead of a patch. The PR seems easier to iterate on, but if you as maintainers prefer patches in Trac, please let me know. I'm happy to switch back to patches.

#3 in reply to: ↑ 2 @peterwilsoncc
2 years ago

Replying to bpayton:

This is the first time I've posted a PR instead of a patch. The PR seems easier to iterate on, but if you as maintainers prefer patches in Trac, please let me know. I'm happy to switch back to patches.

A PR is perfect, many of the maintainers and committers use GitHub rather than patches for the same reason.

#4 @azaozz
2 years ago

  • Milestone changed from Awaiting Review to 6.1

The patch/PR looks good here. Thinking that being able to filter or redirect DB errors will be pretty useful in some cases, especially on bigger/busier sites. Can also be used by a plugin designed to categorize errors and send them to appropriate places, trigger alerts/warnings about specific errors, etc. all from whitin WP.

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


20 months ago

#6 @audrasjb
20 months ago

  • Keywords commit added

AS per today's bug scrub: the PR was reviewed and approved, so let's mark this for commit.

#7 @audrasjb
20 months ago

  • Keywords changes-requested added; commit removed

Removing commit keyword for now, as there is a conflict to resolve in the above pull request.

desrosj commented on PR #2426:


20 months ago
#8

The wp-db.php file was renamed in https://core.trac.wordpress.org/changeset/53749/ to class-wpdb.php. You should be able to just move these changes to that file.

#9 @desrosj
20 months ago

Added some inline suggestions to the PR as well.

#10 @desrosj
20 months ago

  • Milestone changed from 6.1 to 6.2

This one is real close, but beta 1 is incoming. Let's work on wrapping this one up for 6.2.

brandonpayton commented on PR #2426:


18 months ago
#11

@desrosj and @dream-encode, thank you for your review. I pushed some changes to address your feedback and added a temporary redirection of error logging to avoid messages being printed in the middle of PHPUNIT output. How does this look?

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


16 months ago

#13 @mukesh27
16 months ago

  • Keywords needs-dev-note added

#14 @mukesh27
16 months ago

  • Keywords changes-requested removed

This ticket was discussed in the recent bug scrub.

Thanks @bpayton for the ticket.

Looks like the review feedback have addressed and the PR just needs another look.

Props to @costdev

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


16 months ago

#16 @costdev
16 months ago

This ticket was discussed during the bug scrub. It looks like PR 2426 just needs a re-review.

@davidbaumwald @desrosj Do you have time to take another look?

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


16 months ago

#18 @costdev
16 months ago

  • Milestone changed from 6.2 to Future Release

This ticket was discussed during the bug scrub. As the PR still needs reviews, I'll move this to Future Release. If this is reviewed and is ready prior to the 6.2 Beta 1 release party later today, feel free to move it back into the milestone for commit.

Additional props: @mukesh27

brandonpayton commented on PR #2426:


13 months ago
#19

Thanks for the updates @brandonpayton! LGTM! +1

Thanks for your thoughtful guidance and example! This PR is much better now.

Note: See TracTickets for help on using tickets.