WordPress.org

Make WordPress Core

Opened 5 years ago

Closed 5 years ago

#13185 closed defect (bug) (fixed)

advanced-cache.php is @included

Reported by: solarissmoke Owned by: ryan
Milestone: 3.0 Priority: low
Severity: minor Version: 3.0
Component: Cache API Keywords: has-patch
Focuses: Cc:

Description

I've been working using some caching plugins recently and am finding that when errors happen, tracking down the problem can be very tricky until you realise that advanced-cache.php is @included - and then you have to manually edit the core files to remove the error suppression.

The @include was introduced in #4293, to replace a require().

Any objections to removing the @ ?

Attachments (1)

13185.diff (552 bytes) - added by sivel 5 years ago.
Remove the suppression and check for existence of file

Download all attachments as: .zip

Change History (7)

comment:1 @nacin5 years ago

The issue is that then we'd probably want to introduce a file_exists. I assume the idea from #4293 was that silencing it is quicker than the file_exists check.

comment:2 @Denis-de-Bernardy5 years ago

Rather than a file exists check, I'd suggest directing users to the misconfiguration problem. something like:

if ( false === include(cache file) ) {
  echo your site has a cache configuration problem. delete the WP_CACHE define in your wp-config.php file.
}

alternatively, we could temporarily strip E_WARNING from error reporting.

comment:3 @Denis-de-Bernardy5 years ago

extra note: when in the admin area, and on the login screen, we shouldn't even be including that file. any unsilenced error in it prevents users from logging in because it forces php headers to be sent.

@sivel5 years ago

Remove the suppression and check for existence of file

comment:4 @sivel5 years ago

  • Keywords has-patch added

I think that trade off for a file_exists outweighs the potential problems of suppressing all errors that come from this file inclusion.

Patch removes suppression and adds file_exists.

comment:5 @nacin5 years ago

I don't like the idea of adding a file_exists check here. We do it in db.php and object-cache.php, but advanced-cache.php is included before both of them and may very well serve a static file and die, not loading any more of WP. It needs to be fast.

I'm sitting next to jjj and sivel, and we came up with this: Checking WP_DEBUG, and if true, don't suppress, otherwise, we do suppress errors. And no file_exists check. Thoughts?

comment:6 @nacin5 years ago

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

(In [14345]) Don't silence inclusion of advanced-cache.php for WP_DEBUG. fixes #13185.

Note: See TracTickets for help on using tickets.