WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#10202 closed enhancement (fixed)

Enable display_errors if WP_DEBUG is true

Reported by: sivel Owned by:
Milestone: 2.9 Priority: normal
Severity: normal Version: 2.8
Component: Warnings/Notices Keywords: has-patch commit
Focuses: Cc:

Description

Often enough when users enable WP_DEBUG they still do not get any good information because most hosts have display_errors set to Off and some times do not have access to the error logs.

Enabling display_errors when WP_DEBUG is true would make troubleshooting much easier.

We can add a constant for disabling display_errors for those people that would rather just have the errors sent to the globally defined error log file.

In addition we will add another constant to enable logging to a log file called debug.log.

New contants:

  • WP_DEBUG_DISPLAY - If not defined or defined as true it will set display_errors to On. If set to false it will set display_errors to Off. This sets the default behavior with WP_DEBUG to display errors.
  • WP_DEBUG_LOG - If defined and set to true this will log to a file in ABSPATH called debug.log. Set to false or do not define to disable logging to this file.

The above constants will only take effect if WP_DEBUG is defined and set to true.

Attachments (3)

10202.diff (821 bytes) - added by sivel 5 years ago.
10202.1.diff (864 bytes) - added by sivel 5 years ago.
Following the suggestions from Denis-de-Bernardy
10202.2.diff (1.0 KB) - added by sivel 5 years ago.
Reverting some of the logic and keep WP_CONTENT_DIR as the location of the debug.log

Download all attachments as: .zip

Change History (17)

comment:1 sivel5 years ago

  • Keywords has-patch commit added

sivel5 years ago

comment:2 Denis-de-Bernardy5 years ago

  • Keywords commit removed

you'r introducing a potential E_NOTICE or E_WARNING:

if ( ! defined('WP_DEBUG_DISPLAY') || WP_DEBUG_DISPLAY == true ) 

should be:

if ( ! defined('WP_DEBUG_DISPLAY') || defined('WP_DEBUG_DISPLAY') && WP_DEBUG_DISPLAY == true )


comment:3 Denis-de-Bernardy5 years ago

also:

        if ( defined('WP_DEBUG_LOG') && WP_DEBUG_LOG == true ) { 
 	209	                ini_set('log_errors', 1); 
 	210	                ini_set('error_log', ABSPATH . 'debug.log'); 
 	211	        } 

should probably go into WP_CONTENT_DIR (which usually is wrtiable as opposed to ABSPATH)

sivel5 years ago

Following the suggestions from Denis-de-Bernardy

comment:4 dd325 years ago

if ( ! defined('WP_DEBUG_DISPLAY')
( defined('WP_DEBUG_DISPLAY') && WP_DEBUG_DISPLAY == true ) )

That can be simplified to this in line with the rest of core:

if ( ! defined('WP_DEBUG_DISPLAY') || WP_DEBUG_DISPLAY == true ) 

The == true isnt really needed either IMO.. but 'eh :)

+1 for WP_CONTENT_DIR instead of ABSPATH. Even though i think XMLRPC uses ABSPATH/.../rpc.log or something :)

comment:5 dd325 years ago

actually, the uploads directory would be a better location..

comment:6 Denis-de-Bernardy5 years ago

how about we make WP_DEBUG_LOG contain the log file's location?

sivel5 years ago

Reverting some of the logic and keep WP_CONTENT_DIR as the location of the debug.log

comment:7 sivel5 years ago

  • Keywords commit added

DD32 and I agree that the location of the debug.log should be static so that it is located in the same place for everyone. We will not use the uploads dir, as at that point in the load we do not have access to that uploads dir location.

comment:8 follow-up: azaozz5 years ago

Wouldn't it be better to do:

if ( defined('WP_DEBUG_LOG') && WP_DEBUG_LOG ) { 
    ini_set('log_errors', 1); 
    ini_set('error_log', WP_CONTENT_DIR . '/debug.log'); 
} else {
    ini_set('display_errors', 1);
}

When debugging you'll want either one or the other.

comment:9 in reply to: ↑ 8 sivel5 years ago

Replying to azaozz:

Wouldn't it be better to do:

if ( defined('WP_DEBUG_LOG') && WP_DEBUG_LOG ) { 
    ini_set('log_errors', 1); 
    ini_set('error_log', WP_CONTENT_DIR . '/debug.log'); 
} else {
    ini_set('display_errors', 1);
}

When debugging you'll want either one or the other.

Well...what we are trying to achieve is to enable display_errors unless the user wants to disable them. And we only want to set the error_log if the user so specifies, that way logging can continue to go to the globally defined error_log and not force php to log to the debug.log in wp-content.

comment:10 follow-up: jacobsantos5 years ago

Yeah, you should look at BackPress and the Logger that exists in it.

comment:11 jacobsantos5 years ago

You guys are going about it the wrong way. Instead of trying to extend a hack, why not improve it to something actually worth while.

comment:12 in reply to: ↑ 10 westi5 years ago

Replying to jacobsantos:

Yeah, you should look at BackPress and the Logger that exists in it.

The BackPress logger is for logging your own info.

This is about logging php errors either to the page or to a file.

comment:13 automattor5 years ago

  • Resolution set to fixed
  • Status changed from new to closed

(In [11702]) Add extra site debugging support to enable conditionally enable display_errors or a special error_log. Fixes #10202 props sivel.

comment:14 westi5 years ago

  • Milestone changed from 2.8.2 to 2.9
Note: See TracTickets for help on using tickets.