Move error/exception handler higher up in the bootstrap process
killes@www.drop.org - October 23, 2008 - 14:43
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | base system |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | needs work |
Description
It is currently only set during the full bootstrap which is a bit late if e.g. some errors occur earlier.
#1
Agreed, that may require some bootstrap tweaking. Assigning myself.
#2
This should be set as early as humanly possible. Like, immediately after including bootstrap.inc. There's no too-early for this to be set. :-) (Well, except "before the function has been defined".)
#3
What's the correct way to do it, i tried the following?
* move set_error_hander and set_exception_handler to the beginning of _drupal_bootstrap
* move dependant functions from common.inc to bootstrap.inc
* change drupal_log_error to print plain text error message before DRUPAL_BOOTSTRAP_FULL
#4
Haven't looked at the patch, just setting the status, so that bot can say that this doesn't apply anymore ;)
#5
The last submitted patch failed testing.
#6
You don't need me as a bottleneck on this one.
#7
Uncaught exceptions can contain sensitive data (site or database login info), so this is critical for security. (See #332413: [PDO] [Security] Database server failure makes Drupal emit Database user, password)
#8
subscribing because my 7.x system is totally down - makes it hard to convert my modules.
#9
From looking at the patch, this seems straightforward as anything:
1. Move alll error-handling code from common.inc to bootstrap.inc
2. Move the initialization from FULL to CONFIGURATION bootstrap stage.
I've rerolled these two changes. Also added a PHPdoc group for these functions ("Error handling"), since this should make the API page a bit more organized.
Now waiting for Testbot to explode on it.
#10
The last submitted patch failed testing.
#11
I had some rubbish in my text editor, apparently.
#12
The last submitted patch failed testing.
#13
It worked on my computer. Try again?
#14
The last submitted patch failed testing.
#15
In other news, I'm a dunce. :P
Here's the diff including bootstrap.inc.
#16
The last submitted patch failed testing.
#17
#18
Anyone working on this? It is a real problem for drush, among many other things.
#19
re-roll of patch from #15...
#20
The last submitted patch failed testing.
#21
A few test failures. Anyone up for reroll?
#22
#19 was calling a function that does not actually exist. Attached patch calls a function that does exist. No other changes.
#23
Per prodding from Moshe, make _drupal_log_error() safe to use before t() is available.
#24
Remove more t(). (A nod to the coffee drinkers...)
#25
my issue with the error / exception handler is that it falls prey to drupal4hu.com/node/222
<?php: in (line of ). [29.59 sec] [error]
: in (line of ). [29.591 sec] [error]
An error occurred at function : [error]
?>
and
<?php
Fatal error: Exception thrown without a stack frame in Unknown on line 0
Call Stack:
30.0299 1. _drupal_exception_handler() /Users/adrian/Projects/hosting/platforms/HEAD/includes/bootstrap.inc:0
30.0301 2. _drupal_log_error() /Users/adrian/Projects/hosting/platforms/HEAD/includes/bootstrap.inc:2110
?>
what it comes down to is that drupal_log_error should not be called from the error handler, and if it is it should not be throwing exceptions itself, which is what watchdog() is doing.
I want to suggest we put the following into drupal_log_error, because maintenance mode is basically the textbook definition of 'we can't rely on the database to be there to do the logging'
<?phpif (defined('maintenance_mode')) {
return;
}
?>
#26
sorry, correction :
what it comes down to is that drupal_log_error should not be called from the exception handler, and if it is it should not be throwing exceptions itself, which is what watchdog() is doing.
#27
The last submitted patch failed testing.