Skip to content

REST API: Log doing_it_wrong notices to debug.log #10537

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.

Sign up for GitHub

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

Open
ilclaudio wants to merge 9 commits into
base: trunk
Choose a base branch
from
+20 −2
Open

REST API: Log doing_it_wrong notices to debug.log #10537

Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0a06f41
Fixed rest_handle_doing_it_wrong to manage debug messages properly - …
ilclaudio Mar 25, 2026
6c3e353
Fixed rest_handle_doing_it_wrong: added headers check
ilclaudio Mar 25, 2026
610ac74
REST API: Use with 'PHP Notice:' prefix in rest_handle_doing_it_wron…
ilclaudio Mar 26, 2026
6c7c2d4
Merge remote-tracking branch 'upstream/trunk' into fix/64260-rest-doi…
ilclaudio Apr 6, 2026
8bb3815
REST API: Add caller location to rest_handle_doing_it_wrong() error log
ilclaudio Apr 6, 2026
40c4f98
Merge branch 'trunk' into fix/64260-rest-doing-it-wrong-log
westonruter May 14, 2026
0a98d18
Update src/wp-includes/rest-api.php
ilclaudio May 15, 2026
922b735
Update src/wp-includes/rest-api.php
ilclaudio May 15, 2026
043f126
Merge branch 'trunk' into fix/64260-rest-doing-it-wrong-log
ilclaudio Jun 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 20 additions & 2 deletions src/wp-includes/rest-api.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,7 +760,7 @@ function rest_handle_deprecated_argument( $function_name, $message, $version ) {
* @param string|null $version The version of WordPress where the message was added.
*/
function rest_handle_doing_it_wrong( $function_name, $message, $version ) {
if ( ! WP_DEBUG || headers_sent() ) {
if ( ! WP_DEBUG ) {
return;
}

Expand All @@ -774,7 +774,25 @@ function rest_handle_doing_it_wrong( $function_name, $message, $version ) {
$string = sprintf( $string, $function_name, $message );
}

header( sprintf( 'X-WP-DoingItWrong: %s', $string ) );
if ( ! headers_sent() ) {
header( sprintf( 'X-WP-DoingItWrong: %s', $string ) );
}

if ( WP_DEBUG_LOG ) {
Copy link
Copy Markdown
Member

@westonruter westonruter Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude Code: Incomplete/inconsistent fix. Only rest_handle_doing_it_wrong() gains debug-log output. The sibling handlers rest_handle_deprecated_function() and rest_handle_deprecated_argument() are registered identically in rest_api_default_filters() (lines 243–246) with the same __return_false suppression, but still drop their notices from debug.log entirely.

With WP_DEBUG_LOG on, a deprecated function/argument used during a REST request logs nothing, while a _doing_it_wrong() call in the same request now logs — an asymmetry that will confuse debugging and invites copy-pasting this same backtrace/format block into two more places.

Copy link
Copy Markdown
Member

@westonruter westonruter Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude Code (altitude): Consider whether this is the right depth for the fix. The reason these notices never reach the log during REST is line 248's add_filter( 'doing_it_wrong_trigger_error', '__return_false' ), which short-circuits the native wp_trigger_error() logging path. Re-implementing logging here bypasses the canonical message format, sanitization, and hooks of _doing_it_wrong().

Note that display_errors is already forced to 0 for REST requests in wp_debug_mode() (load.php ~642–647), so the suppression filter may be relaxable instead of bolting on a parallel logger. As written, future changes to how core formats/logs _doing_it_wrong won't apply to REST.

Copy link
Copy Markdown
Member

@westonruter westonruter Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude Code: This logging runs on the doing_it_wrong_run action and does not consult the doing_it_wrong_trigger_error filter, so it can't be silenced and may double-log. A site that adds its own doing_it_wrong_trigger_error filter to quiet a known-noisy third-party notice will still get it written to the log on every REST request. Conversely, if any code restores triggering during a REST request, the same incident is recorded twice (native + this entry).

Copy link
Copy Markdown
Member

@westonruter westonruter Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should ensure the notices only get logged out if such logs are desired.

Suggested change
if ( WP_DEBUG_LOG ) {
if ( WP_DEBUG_LOG && ( error_reporting() & E_USER_NOTICE ) ) {

$backtrace = debug_backtrace( DEBUG_BACKTRACE_IGNORE_ARGS );
Copy link
Copy Markdown
Member

@westonruter westonruter Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude Code (reuse): The bespoke debug_backtrace() + "first frame under WP_CONTENT_DIR" loop reinvents caller-location logic that core already provides via wp_debug_backtrace_summary(). Reusing the existing helper (or at least factoring this out) avoids a one-off heuristic that no other part of core relies on and must be re-audited whenever PHP backtrace semantics or path constants change.

$caller = '';
// Find the first caller outside of WordPress core (plugin or theme).
foreach ( $backtrace as $frame ) {
if ( isset( $frame['file'] ) && str_starts_with( $frame['file'], WP_CONTENT_DIR ) ) {
Copy link
Copy Markdown
Member

@westonruter westonruter Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude Code: str_starts_with( $frame['file'], WP_CONTENT_DIR ) silently drops the caller location in two real cases:

  1. Windows — backtrace file paths often use backslashes (C:\\site\\wp-content\\plugins\\foo.php) while WP_CONTENT_DIR may be forward-slash normalized, so the match fails and $caller stays empty.
  2. Custom layouts — plugins/themes outside wp-content (custom WP_PLUGIN_DIR, symlinks) won't match either.

In both cases the log omits the offending file/line, which is exactly the information this feature exists to surface. Also note there's no path-separator boundary on the prefix match, so a sibling dir like /srv/site/wp-content-backup/... would be misattributed as a plugin/theme caller.

$caller = ' in ' . $frame['file'];
if ( isset( $frame['line'] ) ) {
$caller .= ' on line ' . $frame['line'];
}
break;
}
}
error_log( 'PHP Notice: ' . wp_strip_all_tags( $string ) . $caller );
Copy link
Copy Markdown
Member

@westonruter westonruter Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need wp_strip_all_tags() here. This is going straight to the error log, so it is not being displayed anywhere. I don't feel strongly about this, however.

Suggested change
error_log( 'PHP Notice: ' . wp_strip_all_tags( $string ) . $caller );
error_log( 'PHP Notice: ' . $string . $caller );

Copy link
Copy Markdown
Member

@westonruter westonruter Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude Code: error_log() with a single argument writes to PHP's configured error_log ini setting. That setting is only applied by wp_debug_mode() (load.php ~630–633) when debug-mode checks run. If a mu-plugin returns false on the enable_wp_debug_mode_checks filter, wp_debug_mode() returns early (lines 609–611) before ini_set( 'error_log', $log_path ), yet WP_DEBUG_LOG is still truthy here — so the notice goes to the SAPI default error log (e.g. stderr / the web-server log), not the admin's configured debug.log.

(For what it's worth, I verified the 'PHP Notice: ' prefix is correct: wp_trigger_error() defaults to E_USER_NOTICE, so this matches native severity.)

Copy link
Copy Markdown
Member

@westonruter westonruter Jun 2, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤖 Claude Code: error_log() is not filtered by error_reporting() — it writes unconditionally. The native logging path (_doing_it_wrong() → wp_trigger_error() → trigger_error( $message, E_USER_NOTICE )) does respect error_reporting(), so a site that masks out notices (e.g. error_reporting( E_ALL & ~E_USER_NOTICE ), or a php.ini that excludes notices combined with an enable_wp_debug_mode_checks filter that stops WP from forcing E_ALL) would normally see nothing logged.

With this direct error_log() call, the notice is written to the log even when the operator has explicitly configured error reporting to exclude notices — i.e. logging output that wasn't requested. If the logging stays hand-rolled, it should gate on error_reporting() & E_USER_NOTICE (and arguably on ! headers_sent()-independent state) to match the behavior callers expect; routing through wp_trigger_error() instead would get this for free.

}
}

/**
Expand Down
Loading