Don't cache pages when there are PHP/MySQL errors
EvanDonovan - February 27, 2009 - 16:40
| Project: | Boost |
| Version: | 6.x-1.x-dev |
| Component: | Caching logic |
| Category: | bug report |
| Priority: | normal |
| Assigned: | Unassigned |
| Status: | closed |
Description
I have noticed that Boost will cache pages even when there is no working connection to the database. This causes anonymous users to be served pages with error messages stating "MySQL server has gone away" even after MySQL has been restarted. The only way I know to solve this currently is to clear the entire Boost cache.
It would be great if there was a way for Boost to test for a MySQL connection before caching pages.
#1
Maybe this should actually be a feature request, but I think it would be great if Boost didn't cache pages when either the MySQL connection was down or when there were fatal PHP errors.
I know the Authcache module handles the latter case as follows (in the _authcache_shutdown_save_page() function of authcache.helpers.inc, lines 116-119):
<?php
// Don't cache pages with PHP errors (Drupal can't catch fatal errors)
if(count(error_get_last())) {
return;
}
?>
Since I think a broken MySQL connection causes PHP errors that might actually solve both cases at once. I'm not completely sure.
#2
#3
I'm currently in Los Angeles (not home) until the 20th, but this will be a top priority once I get back. Fix is easy, if the above code you gave is correct.
Before
<?php/**
* Implementation of hook_init(). Performs page setup tasks if page not cached.
*/
function boost_init() {
// Stop right here unless we're being called for an ordinary page request
if (strpos($_SERVER['SCRIPT_FILENAME'], 'index.php') === FALSE || variable_get('site_offline', 0))
return;
?>
After
<?php/**
* Implementation of hook_init(). Performs page setup tasks if page not cached.
*/
function boost_init() {
// Stop right here unless we're being called for an ordinary, error free page request
if (strpos($_SERVER['SCRIPT_FILENAME'], 'index.php') === FALSE || variable_get('site_offline', 0) || count(error_get_last()))
return;
?>
While here we should also check for drupal messages; those shouldn't be cached.
http://api.drupal.org/api/function/drupal_set_message
http://api.drupal.org/api/function/page_get_cache
#4
Sounds great. They check for those in Authcache as well; I can find the specific code if you need it.
#5
Something that may be a problem, though, is error reporting level. When installing Authcache a few days ago, I found pages were not being saved to the cache due to an E_NOTICE-level error. Since those could happen quite frequently on some Drupal sites, and don't generally show to the screen, then this check would stop more pages from being cached than necessary.
Therefore, the checking code might need to be more elaborate, similar to that in comment #2 of http://php.market.in.th/manual/sl/function.set-error-handler.php.
<?phpif ($error = error_get_last()){
switch($error['type']){
case E_ERROR:
// other cases that should not be cached
return;
default:
//do nothing, continue as normal
}
}
?>
#6
Here's a list of errors
http://www.php.net/errorfunc.constants
http://www.php.net/error_get_last
If you could write the logic so it only halts on critical & writing to the output, errors that would be quite helpful.
#7
I think these are the 4 errors in which boost should ignore and cache the page
<?phpif ($error = error_get_last()){
switch($error['type']){
case E_NOTICE: //Ignore run-time notices
case E_USER_NOTICE: //Ignore user-generated notice message
case E_DEPRECATED: //Ignore run-time notices
case E_USER_DEPRECATED: //Ignore user-generated notice message
break;
default: //Do not cache page on all other errors
return;
}
}
?>
So this is what the new code block would look like
<?php/**
* Implementation of hook_init(). Performs page setup tasks if page not cached.
*/
function boost_init() {
// Stop right here unless we're being called for an ordinary, error free page request
if (strpos($_SERVER['SCRIPT_FILENAME'], 'index.php') === FALSE || variable_get('site_offline', 0) || $user->uid || $_SERVER['REQUEST_METHOD'] != 'GET' || count(drupal_get_messages(NULL, FALSE)) || $_SERVER['SERVER_SOFTWARE'] === 'PHP CLI')
return;
if ($error = error_get_last()){
switch($error['type']){
case E_NOTICE: //Ignore run-time notices
case E_USER_NOTICE: //Ignore user-generated notice message
case E_DEPRECATED: //Ignore run-time notices
case E_USER_DEPRECATED: //Ignore user-generated notice message
break;
default: //Do not cache page on all other errors
return;
}
}
?>
#8
Sounds good. But what if there were an error message that was generated on the page request and printed to the screen after the boost module had already passed the init hook?
#9
Good point, I'll add the error checks into the _boost_ob_handler() function, so
count(drupal_get_messages(NULL, FALSE))anderror_get_last()would go in there.#10
That's good - that's exactly how Authcache handles it.
#11
Decided to clean up boost_init while we are here. This really hasn't been tested since I have to run; will test later.
Here's how it looks
<?php
/**
* Implementation of hook_init(). Performs page setup tasks if page not cached.
*/
function boost_init() {
// Make the proper filename for our query
$fname = '';
foreach ($_GET as $key => $val) {
if ($key != 'q') {
$fname .= (empty($fname) ? '' : '&') . $key . '=' . $val;
}
}
//set variables
$GLOBALS['_boost_query'] = (empty($fname) ? '' : '_' . $fname);
$GLOBALS['_boost_path'] = (empty($_REQUEST['q'])) ? 'index' : $_REQUEST['q'];
global $user;
// Make sure the page is/should be cached according to our current configuration
if ( strpos($_SERVER['SCRIPT_FILENAME'], 'index.php') === FALSE
|| variable_get('site_offline', 0)
|| $_SERVER['REQUEST_METHOD'] != 'GET'
|| $_SERVER['SERVER_SOFTWARE'] === 'PHP CLI'
|| variable_get('cache', CACHE_DISABLED)
|| !BOOST_ENABLED
|| isset($_GET['nocache'])
|| !boost_is_cacheable($GLOBALS['_boost_path'])
) {
return;
}
// For authenticated users, set a special cookie to prevent them
// inadvertently getting served pages from the static page cache.
if (!empty($user->uid)) {
boost_set_cookie($user);
}
// We only generate cached pages for anonymous visitors.
else {
ob_start('_boost_ob_handler');
}
}
?>
#12
Tested above code and it works; meaning the hook_init rewrite is good. Inside the patch is the do not cache errors code as well; this is a little bit harder to test so any error testing is greatly appreciated.
#13
committed
#14
Adding code to boost block so admin knows the page can not be cached due to php/drupal errors/messages. Also used hook_help as a hack to get the correct values from drupal_get_messages().
#15
Changing above patch do to this issue #480266: PHP < 5.2.0 - No files created since update to 6.x-1.0-beta1 due to missing error_get_last(). Also did some code cleanup of the patch.
#16
incorporating #480266: PHP < 5.2.0 - No files created since update to 6.x-1.0-beta1 due to missing error_get_last() into this patch since it's related. Formatted the block better as well.
#17
Added a setting to control if boost will cache on errors.
#18
#19
committed
#20
if i uncheck the new checkbox and save the settings, the checkbox is checked again
#21
I can not confirm this, it works for me. Try unchecking it one more time; it makes no since. In the mean time I've done more cosmetic changes to boost's watchdog errors, and added more as well. Also changed the default to false, because php throws errors too easily.
This patch requires the one above to be applied first.
#22
#23
committed above changes so next dev will have the default set to false
#24
Ok it was my fault, somehow the previous patch messed up my module file. I re applied Patch 3 and then Patch 4, now everything is working as expected.
Thanks for all your support and this great module.
#25
#26
Automatically closed -- issue fixed for 2 weeks with no activity.