Make WordPress Core

Opened 9 years ago

Closed 8 years ago

#34074 closed defect (bug) (invalid)

wp_debug false still showing errors

Reported by: siebje's profile siebje Owned by:
Milestone: Priority: normal
Severity: normal Version: 4.3.1
Component: General Keywords: has-patch
Focuses: Cc:

Description

Where:

define('WP_DEBUG', false);

before PHP 5.4 did not show any errors, now after updating to PHP higher then 5.4, it does show errors:

http://wsinternetservices.nl/temp/errors.jpg

Now we have to add extra line of code to hide errors:

define('WP_DEBUG', false);
@ini_set('display_errors', 0);

Why is this ini_set part not included in the core wp_debug_mode function?

Because the above code now basicly is the same as:

define('WP_DEBUG', true);
define('WP_DEBUG_DISPLAY', false);

Because in the WP_DEBUG_DISPLAY the ini_set IS included in the core wp_debug_mode function:

if ( WP_DEBUG_DISPLAY )
ini_set( 'display_errors', 1 );
elseif ( null !== WP_DEBUG_DISPLAY )
ini_set( 'display_errors', 0 );

Attachments (1)

34074.patch (2.5 KB) - added by jrf 9 years ago.

Download all attachments as: .zip

Change History (6)

#1 @jrf
9 years ago

Hi @siebje,

Congratulations on your first ticket and thank you for pointing this out.

I've had a look at this and from what I can see, this behaviour is by design.

The WP_DEBUG_DISPLAY constant does not have any effect, unless WP_DEBUG is set to true.

See: https://codex.wordpress.org/Editing_wp-config.php#Configure_Error_Logging for the documentation stating this:

First off, remember that if WP_DEBUG is false, it and the other two WordPress DEBUG constants do not do anything. The PHP directives, whatever they are, will prevail.

In other words, if you do not set WP_DEBUG or set it to false, the server PHP configuration for display_errors will be used as defined in php.ini.

display_errors should always be off for production servers and is highly recommended to be *on* for development servers.
If the PHP version upgrade you mention in your report was on a production server, I would highly recommend that you switch to a more professional web-host.


Having said that, I agree with you that a case could be made for changing the behaviour, but considering that WP tries not to break backward compatibility if it can be avoided at all, I kind of doubt it would get in.

This was also discussed in ticket #11987 and decided against at that time.

Nevertheless, on the off-chance that someone would be willing to consider it, I will upload a potential patch for this.

The patch presumes that if WP_DEBUG was not set by the admin or if it was set to false, the displaying of errors should be turned off unless the admin has explicitly set WP_DEBUG_DISPLAY to null to force WP to follow the server settings.

The choice not to 'respect' WP_DEBUG_DISPLAY if set to true while WP_DEBUG is false was made because the default setting for WP_DEBUG_DISPLAY is true and changing the default seems even less likely to find a kind ear.

@jrf
9 years ago

#2 @jrf
9 years ago

  • Keywords has-patch added

#3 @nacin
9 years ago

This one is interesting.

WP_DEBUG is false by default. Allowing define( 'WP_DEBUG_DISPLAY', false ); to turn off display_errors when WP_DEBUG is *off* does not make a lot of sense to me. The WP_DEBUG_DISPLAY` constants is already pretty magical, as it's a boolean but actually has three states.

I think what is being proposed is this:

define( 'WP_DEBUG', false );
define( 'WP_DEBUG_DISPLAY', false );

Instead of this:

define( 'WP_DEBUG', false );
ini_set( 'display_errors', 0 );

Which is equivalent to this:

ini_set( 'display_errors', 0 );

I'd rather folks just stick to the PHP code here, rather than trying to mask it with some special WP constant.

Generally - display_errors should never be enabled in production, anyway, which is also why WP doesn't go anywhere near the setting. The exception is WP_DEBUG forcing it on, which is also why WP_DEBUG_DISPLAY exists. I'd rather not repurpose the constant to also work outside of debug mode.

This is a bit confusing -- I hope that explanation makes sense.

Version 0, edited 9 years ago by nacin (next)

#4 @jrf
9 years ago

  • Keywords close added

#5 @SergeyBiryukov
8 years ago

  • Keywords close removed
  • Milestone Awaiting Review deleted
  • Resolution set to invalid
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.