Make WordPress Core

Opened 14 years ago

Closed 14 years ago

#13185 closed defect (bug) (fixed)

advanced-cache.php is @included

Reported by: solarissmoke's profile solarissmoke Owned by: ryan's profile 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 14 years ago.
Remove the suppression and check for existence of file

Download all attachments as: .zip

Change History (7)

#1 @nacin
14 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.

#2 @Denis-de-Bernardy
14 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.

#3 @Denis-de-Bernardy
14 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.

@sivel
14 years ago

Remove the suppression and check for existence of file

#4 @sivel
14 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.

#5 @nacin
14 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?

#6 @nacin
14 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.