Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

Merge pull request #552 from tPl0ch/2.1-error-handler-bootstrap

Move inclusion of APP bootstrap after initialization of ErrorHandler
  • Loading branch information...
commit f26d24b445830f5ebe12051c459480675314a9b8 2 parents 13b748a + cc9b445
@markstory markstory authored
View
6 lib/Cake/Core/Configure.php
@@ -75,9 +75,6 @@ public static function bootstrap($boot = true) {
App::$bootstrapping = false;
App::init();
App::build();
- if (!include APP . 'Config' . DS . 'bootstrap.php') {
- trigger_error(__d('cake_dev', "Can't find application bootstrap file. Please create %sbootstrap.php, and make sure it is readable by PHP.", APP . 'Config' . DS), E_USER_ERROR);
- }
$level = -1;
if (isset(self::$_values['Error']['level'])) {
error_reporting(self::$_values['Error']['level']);
@@ -89,6 +86,9 @@ public static function bootstrap($boot = true) {
if (!empty(self::$_values['Exception']['handler'])) {
set_exception_handler(self::$_values['Exception']['handler']);
}
+ if (!include APP . 'Config' . DS . 'bootstrap.php') {
+ trigger_error(__d('cake_dev', "Can't find application bootstrap file. Please create %sbootstrap.php, and make sure it is readable by PHP.", APP . 'Config' . DS), E_USER_ERROR);
+ }
}
}
View
10 lib/Cake/View/Elements/exception_stack_trace.ctp
@@ -41,10 +41,12 @@ App::uses('Debugger', 'Utility');
echo ' → ';
if ($stack['function']):
$args = array();
- foreach ($stack['args'] as $arg):
- $args[] = Debugger::getType($arg);
- $params[] = Debugger::exportVar($arg, 2);
- endforeach;
+ if (!empty($stack['args'])):
+ foreach ((array)$stack['args'] as $arg):
+ $args[] = Debugger::getType($arg);
+ $params[] = Debugger::exportVar($arg, 2);
+ endforeach;
+ endif;
$called = isset($stack['class']) ? $stack['class'] . $stack['type'] . $stack['function'] : $stack['function'];

7 comments on commit f26d24b

@dereuromark
Collaborator

It seems this breaks current ErrorHandling in apps if they use Plugin stuff!

App::uses('MyErrorHandler', 'Tools.Error');
Configure::write('Error', array(
    'handler' => 'MyErrorHandler::handleError',
    'level' => E_ALL & ~E_DEPRECATED,
    'trace' => true
));

now results in:

Warning: set_error_handler() expects the argument (MyErrorHandler::handleError) to be a valid callback in E:\...\lib\Cake\Core\Configure.php on line 84
Fatal error: Uncaught exception 'MissingPluginException' with message 'Plugin Tools could not be found.' 
@ADmad
Collaborator

Perhaps the error/exception handler setup code should be extracted in a separate function and that function should be for app's bootstrap.php. Then the developer can load required plugin before calling the setup function.

@tPl0ch

@ADmad @markstory Probably reverting this change in 2.1 and use a BC compatible implementation in 2.2 is a better approach since existing apps are affected.

@markstory
Owner

Or set the exception/error handler to cakephp's before the bootstrap, and then reset them with the app settings after. This would at least gives nice errors in the situation of the original pull request, and fixes the issues this change introduced.

@ADmad
Collaborator

@markstory Yes that's an option I had thought too, just wanted to avoid setting the handlers twice. But I guess its no biggie and a better safeguard against potential problems.

@tPl0ch

@markstory Quick and dirty: tPl0ch@6ba225d

@markstory
Owner

@tPl0ch I think this has the same issue as before, as the plugin/app error handlers won't have been loaded yet. If anything we'd need to set the default handlers to the core ones before bootstrap.php and re-bind afterwards. This will also probably require a change in the test runners, as they restore exception/error handlers so PHPUnit's is the active one.

I guess we could just revert the change on moving the error handlers for now too as an alternative option.

Please sign in to comment.
Something went wrong with that request. Please try again.