johnbillion / query-monitor

The developer tools panel for WordPress
https://querymonitor.com
GNU General Public License v2.0
1.58k stars 207 forks source link

QM hijack _doing_it_wrong(), _deprecated_function() from PHP logs #831

Open hawkidoki opened 9 months ago

hawkidoki commented 9 months ago

Hello,

I would to report an issue. It's not really a bug tho, as I believe this is an intended behavior since the rework of _doing_it_wrong(), _deprecated_function() etc... handling in a recent update this summer (see #739).

Since this patch, any _doing_it_wrong(), _deprecated_function() thrown by the WP core aren't logged in PHP logs file or in /wp-content/debug.log when WP_DEBUG_LOG is enabled. It seems that only QM is able to catch them.

I believe QM should leave errors in logs file and not hijack them, as many developers watch the error logs file when coding. Sure, it is nice to have a separate tab in QM for "Doing it wrong", but QM admin bar is not always available (in ajax queries etc...) which means some "Doing it wrong" could be never seen.

As developers, we need to trust our PHP logs as the ultimate source of truth when it comes to errors/warnings/notices.

Steps to reproduce:

// wp-config.php
define('WP_DEBUG', true);
define('WP_DEBUG_LOG', true);
define('WP_DEBUG_DISPLAY', false);
// functions.php
add_action('init', 'my_init');
function my_init(){

    _deprecated_function('test', '1.0');

    _doing_it_wrong('test', 'test', '1.0');

}

With QM enabled:

// wp-content/debug.log
(empty)

Without QM:

// wp-content/debug.log
[11-Nov-2023 09:12:10 UTC] PHP Deprecated:  Function test is deprecated since version 1.0 with no alternative available. in \wp-includes\functions.php on line 6031
[11-Nov-2023 09:12:10 UTC] PHP Notice:  Function test was called incorrectly. test Please see <a>Debugging in WordPress</a> for more information. (This message was added in version 1.0.) in \wp-includes\functions.php on line 6031

Thanks! Regards.

crstauf commented 9 months ago

I don't believe that was intended, but you're right, it is a consequence.

Currently, the error is only prevented if the current user can view QM, which means that visits without QM visible will still be logged. Unless QM is visible to all users?