Make WordPress Core

Opened 2 years ago

Last modified 2 years ago

#56035 new enhancement

Error logging improvements

Reported by: galbaras's profile galbaras Owned by:
Milestone: Awaiting Review Priority: normal
Severity: normal Version:
Component: Site Health Keywords:
Focuses: Cc:

Description

The current error logging isn't always as helpful as it could be. Here are some things that can be improved:

  1. Consolidation of all messages to one log file. I sometimes see the same message in error_log and wp-admin/error_log. Sometimes, I see admin messages only in error_log, and it's often a hassle cross-referencing issues that may occur on the front-end, but are an effect of some admin issue.

When factoring in things like WooCommerce logs and other special cases, it gets even worse.

Therefore, I suggest having one file where all messages are written, whether they are written somewhere else as well or not.

Since error_log() is a PHP function, this can be done via a wrapper function and/or communication with plugin and theme developers, unless someone has a better idea.

  1. All database query failures should be logged along with the context in which they occurred (the call stack).
  1. Site Health recovery messages should be written to the log file, not just sent by email.

When changing code, it's important to fix errors quickly, and email can take a few minutes to arrive in some cases, e.g. when the email goes via some external spam filtering service.

  1. Ideally, the default log file should be named error.log or error_log.txt to make it easier to open it on computers that associate text editors with file name extensions.

Change History (7)

#1 @ironprogrammer
2 years ago

  • Type changed from defect (bug) to enhancement

#2 @galbaras
2 years ago

As an example, I get many repetitions of this message:

WordPress database error: Processing the value for the following field failed: url. The supplied value may be too long or contains invalid data.

There's nothing before or after that can help shed light on the bit of code where this happens, so I'm flying blind.

At other times, all I get is Failed to insert log entry, which is really unhelpful.

This is related to #32315.

#3 @SergeyBiryukov
2 years ago

Hi there, thanks for the ticket!

Replying to galbaras:

Consolidation of all messages to one log file. I sometimes see the same message in error_log and wp-admin/error_log. Sometimes, I see admin messages only in error_log, and it's often a hassle cross-referencing issues that may occur on the front-end, but are an effect of some admin issue.
...
Therefore, I suggest having one file where all messages are written, whether they are written somewhere else as well or not.

I think it depends on how error logging is set up on a particular web host: per directory, per site, etc.

I believe this can already be addressed without any core changes by defining the WP_DEBUG_LOG constant, which logs to wp-content/debug.log by default, but also allows a custom path to be specified.

#4 @SergeyBiryukov
2 years ago

Replying to galbaras:

As an example, I get many repetitions of this message:

WordPress database error: Processing the value for the following field failed: url. The supplied value may be too long or contains invalid data.

There's nothing before or after that can help shed light on the bit of code where this happens, so I'm flying blind.

I agree, some kind of context or call stack would be helpful here.

At other times, all I get is Failed to insert log entry, which is really unhelpful.

I could not find that string in core, it appears to come from the iThemes Security plugin.

#5 follow-up: @galbaras
2 years ago

Thank you, @SergeyBiryukov .

According to the documentation:

WP_DEBUG_LOG is a companion to WP_DEBUG that causes all errors to also be saved to a debug.log log file

This means it's not a way to override the logging destination, but to write additional copies of the messages in another file.

Looking at wp-includes/load.php, this isn't the case. When WP_DEBUG_LOG is set, it becomes the value of the PHP setting error_log. However, it required WP_DEBUG to be true, which we normally don't want/need.

On my sites, the error_log setting is set to "error_log", which means a file with that name in the directory where the code is being executed. This seems to be the source of having multiple files. Why not do this, then:

if ( ini_get( 'error_log' ) === 'error_log' ) {
   ini_set( 'error_log', ABSPATH . 'error_log' );
}

The best place for this code is at the bottom of wp-config.php, after ABSPATH is defined, or the top of wp-settings.php, ahead of anything that can fail :)

Alternatively, provide a way to change the log file even then WP_DEBUG is false.

I could not find that string in core, it appears to come from the iThemes Security plugin.

Embarrassingly, I knew that and opened a forum thread for this under iThemes Security, which is now closed. I've opened a new one with the updated message, which will hopefully be addresses by the respective team.

Having said that, a stack trace of this could really help, but we don't have one :(

#6 in reply to: ↑ 5 ; follow-up: @SergeyBiryukov
2 years ago

Replying to galbaras:

According to the documentation:

WP_DEBUG_LOG is a companion to WP_DEBUG that causes all errors to also be saved to a debug.log log file

This means it's not a way to override the logging destination, but to write additional copies of the messages in another file.

Hmm, my interpretation of that is a bit different, I understand that as errors being saved in addition to being displayed, not saved to multiple files. Perhaps that sentence could be clarified a bit.

Looking at wp-includes/load.php, this isn't the case. When WP_DEBUG_LOG is set, it becomes the value of the PHP setting error_log. However, it required WP_DEBUG to be true, which we normally don't want/need.

Indeed, thanks for pointing that out! I missed that WP_DEBUG_LOG only works if WP_DEBUG is enabled.

Alternatively, provide a way to change the log file even then WP_DEBUG is false.

I wonder if we could make WP_DEBUG_DISPLAY and WP_DEBUG_LOG independent of WP_DEBUG:

  • WP_DEBUG_DISPLAY and WP_DEBUG_LOG would always control error displaying and log file location, if set.
  • WP_DEBUG would then only control the error reporting level.

#7 in reply to: ↑ 6 @galbaras
2 years ago

I wonder if we could make WP_DEBUG_DISPLAY and WP_DEBUG_LOG independent of WP_DEBUG:

  • WP_DEBUG_DISPLAY and WP_DEBUG_LOG would always control error displaying and log file location, if set.
  • WP_DEBUG would then only control the error reporting level.

That'll work for me :)

I suppose setting my own log file will take care of points 1 and 4 of this ticket. You've agreed to point 2, which should be resolved by triggering a stack trace (using debug_backtrace()?).

What about writing Site Health recovery notices to the log file besides emailing them?

Note: See TracTickets for help on using tickets.