diff --git a/core/startup/ErrorControlChain.php b/core/startup/ErrorControlChain.php index 1861f4b88..dc32257e7 100644 --- a/core/startup/ErrorControlChain.php +++ b/core/startup/ErrorControlChain.php @@ -3,16 +3,13 @@ /** * Class ErrorControlChain * - * Runs a set of steps, optionally suppressing (but recording) any errors (even fatal ones) that occur in each step. - * If an error does occur, subsequent steps are normally skipped, but can optionally be run anyway - * - * Normal errors are suppressed even past the end of the chain. Fatal errors are only suppressed until the end - * of the chain - the request will then die silently. + * Runs a set of steps, optionally suppressing uncaught errors or exceptions which would otherwise be fatal that + * occur in each step. If an error does occur, subsequent steps are normally skipped, but can optionally be run anyway. * * Usage: * * $chain = new ErrorControlChain(); - * $chain->then($callback1)->then($callback2)->then(true, $callback3)->execute(); + * $chain->then($callback1)->then($callback2)->thenIfErrored($callback3)->execute(); * * WARNING: This class is experimental and designed specifically for use pre-startup in main.php * It will likely be heavily refactored before the release of 3.2 @@ -28,6 +25,9 @@ class ErrorControlChain { /** We can't unregister_shutdown_function, so this acts as a flag to enable handling */ protected $handleFatalErrors = false; + /** We overload display_errors to hide errors during execution, so we need to remember the original to restore to */ + protected $originalDisplayErrors = null; + public function hasErrored() { return $this->error; } @@ -38,6 +38,7 @@ class ErrorControlChain { public function setSuppression($suppression) { $this->suppression = (bool)$suppression; + if ($this->handleFatalErrors) ini_set('display_errors', !$suppression); } /** @@ -68,24 +69,14 @@ class ErrorControlChain { return $this->then($callback, null); } - public function handleError($errno, $errstr) { - if ((error_reporting() & self::$fatal_errors & $errno) != 0 && $this->suppression) { - throw new Exception('Generic Error'); - } - else { - return false; - } - } - protected function lastErrorWasFatal() { $error = error_get_last(); - return $error && $error['type'] == 1; + return $error && ($error['type'] & self::$fatal_errors) != 0; } public function handleFatalError() { if ($this->handleFatalErrors && $this->suppression) { if ($this->lastErrorWasFatal()) { - ob_clean(); $this->error = true; $this->step(); } @@ -93,10 +84,12 @@ class ErrorControlChain { } public function execute() { - set_error_handler(array($this, 'handleError')); register_shutdown_function(array($this, 'handleFatalError')); $this->handleFatalErrors = true; + $this->originalDisplayErrors = ini_get('display_errors'); + ini_set('display_errors', !$this->suppression); + $this->step(); } @@ -105,13 +98,7 @@ class ErrorControlChain { $step = array_shift($this->steps); if ($step['onErrorState'] === null || $step['onErrorState'] === $this->error) { - try { - call_user_func($step['callback'], $this); - } - catch (Exception $e) { - if ($this->suppression) $this->error = true; - else throw $e; - } + call_user_func($step['callback'], $this); } $this->step(); @@ -119,7 +106,7 @@ class ErrorControlChain { else { // Now clean up $this->handleFatalErrors = false; - restore_error_handler(); + ini_set('display_errors', $this->originalDisplayErrors); } } } diff --git a/tests/core/startup/ErrorControlChainTest.php b/tests/core/startup/ErrorControlChainTest.php index e9c2c8435..8b5a1d622 100644 --- a/tests/core/startup/ErrorControlChainTest.php +++ b/tests/core/startup/ErrorControlChainTest.php @@ -1,90 +1,241 @@ getItemPath('ErrorControlChain'); + $suppression = $this->suppression ? 'true' : 'false'; + + // Start building a PHP file that will execute the chain + $src = '<'."?php +require_once '$classpath'; + +\$chain = new ErrorControlChain(); + +\$chain->setSuppression($suppression); + +\$chain +"; + + // For each step, use reflection to pull out the call, stick in the the PHP source we're building + foreach ($this->steps as $step) { + $func = new ReflectionFunction($step['callback']); + $source = file($func->getFileName()); + + $start_line = $func->getStartLine() - 1; + $end_line = $func->getEndLine(); + $length = $end_line - $start_line; + + $src .= implode("", array_slice($source, $start_line, $length)) . "\n"; + } + + // Finally add a line to execute the chain + $src .= "->execute();"; + + // Now stick it in a temporary file & run it + $codepath = TEMP_FOLDER.'/ErrorControlChainTest_'.sha1($src).'.php'; + + $null = is_writeable('/dev/null') ? '/dev/null' : 'NUL'; + + file_put_contents($codepath, $src); + exec("php $codepath 2> $null", $stdout, $errcode); + unlink($codepath); + + return array(implode("\n", $stdout), $errcode); + } +} + class ErrorControlChainTest extends SapphireTest { - function testErrorSuppression() { - $chain = new ErrorControlChain(); + function setUp() { + // Check we can run PHP at all + $null = is_writeable('/dev/null') ? '/dev/null' : 'NUL'; + exec("php -v 2> $null", $out, $rv); - $chain - ->then(function(){ - user_error('This error should be suppressed', E_USER_ERROR); - }) - ->execute(); + if ($rv != 0) { + $this->markTestSkipped("Can't run PHP from the command line - is it in your path?"); + $this->skipTest = true; + } - $this->assertTrue($chain->hasErrored()); + parent::setUp(); } - function testMultipleErrorSuppression() { - $chain = new ErrorControlChain(); + function testErrorSuppression() { - $chain + // Fatal error + + $chain = new ErrorControlChainTest_Chain(); + + list($out, $code) = $chain ->then(function(){ - user_error('This error should be suppressed', E_USER_ERROR); + Foo::bar(); // Non-existant class causes fatal error }) - ->thenAlways(function(){ - user_error('This error should also be suppressed', E_USER_ERROR); + ->thenIfErrored(function(){ + echo "Done"; }) - ->execute(); + ->executeInSubprocess(); - $this->assertTrue($chain->hasErrored()); + $this->assertEquals('Done', $out); + + // User error + + $chain = new ErrorControlChainTest_Chain(); + + list($out, $code) = $chain + ->then(function(){ + user_error('Error', E_USER_ERROR); + }) + ->thenIfErrored(function(){ + echo "Done"; + }) + ->executeInSubprocess(); + + $this->assertEquals('Done', $out); + + // Recoverable error + + $chain = new ErrorControlChainTest_Chain(); + + list($out, $code) = $chain + ->then(function(){ + $x = function(ErrorControlChain $foo){ }; + $x(1); // Calling against type + }) + ->thenIfErrored(function(){ + echo "Done"; + }) + ->executeInSubprocess(); + + $this->assertEquals('Done', $out); } function testExceptionSuppression() { - $chain = new ErrorControlChain(); + $chain = new ErrorControlChainTest_Chain(); - $chain + list($out, $code) = $chain ->then(function(){ throw new Exception('This exception should be suppressed'); }) - ->execute(); - - $this->assertTrue($chain->hasErrored()); - } - - function testMultipleExceptionSuppression() { - $chain = new ErrorControlChain(); - - $chain - ->then(function(){ - throw new Exception('This exception should be suppressed'); + ->thenIfErrored(function(){ + echo "Done"; }) - ->thenAlways(function(){ - throw new Exception('This exception should also be suppressed'); - }) - ->execute(); + ->executeInSubprocess(); - $this->assertTrue($chain->hasErrored()); + $this->assertEquals('Done', $out); } function testErrorControl() { - $preError = $postError = array('then' => false, 'thenIfErrored' => false, 'thenAlways' => false); + $chain = new ErrorControlChainTest_Chain(); - $chain = new ErrorControlChain(); - - $chain - ->then(function() use (&$preError) { $preError['then'] = true; }) - ->thenIfErrored(function() use (&$preError) { $preError['thenIfErrored'] = true; }) - ->thenAlways(function() use (&$preError) { $preError['thenAlways'] = true; }) + list($out, $code) = $chain + ->then(function() { echo 'preThen,'; }) + ->thenIfErrored(function() { echo 'preThenIfErrored,'; }) + ->thenAlways(function() { echo 'preThenAlways,'; }) ->then(function(){ user_error('An error', E_USER_ERROR); }) - ->then(function() use (&$postError) { $postError['then'] = true; }) - ->thenIfErrored(function() use (&$postError) { $postError['thenIfErrored'] = true; }) - ->thenAlways(function() use (&$postError) { $postError['thenAlways'] = true; }) + ->then(function() { echo 'postThen,'; }) + ->thenIfErrored(function() { echo 'postThenIfErrored,'; }) + ->thenAlways(function() { echo 'postThenAlways,'; }) - ->execute(); + ->executeInSubprocess(); $this->assertEquals( - array('then' => true, 'thenIfErrored' => false, 'thenAlways' => true), - $preError, - 'Then and thenAlways callbacks called before error, thenIfErrored callback not called' - ); - - $this->assertEquals( - array('then' => false, 'thenIfErrored' => true, 'thenAlways' => true), - $postError, - 'thenIfErrored and thenAlways callbacks called after error, then callback not called' + "preThen,preThenAlways,postThenIfErrored,postThenAlways,", + $out ); } + function testSuppressionControl() { + // Turning off suppression before execution + + $chain = new ErrorControlChainTest_Chain(); + $chain->setSuppression(false); + + list($out, $code) = $chain + ->then(function($chain){ + Foo::bar(); // Non-existant class causes fatal error + }) + ->executeInSubprocess(); + + $this->assertContains("Fatal error: Class 'Foo' not found", $out); + + // Turning off suppression during execution + + $chain = new ErrorControlChainTest_Chain(); + + list($out, $code) = $chain + ->then(function($chain){ + $chain->setSuppression(false); + Foo::bar(); // Non-existent class causes fatal error + }) + ->executeInSubprocess(); + + $this->assertContains("Fatal error: Class 'Foo' not found", $out); + } + + function testDoesntAffectNonFatalErrors() { + $chain = new ErrorControlChainTest_Chain(); + + list($out, $code) = $chain + ->then(function(){ + $array = null; + if (@$array['key'] !== null) user_error('Error', E_USER_ERROR); + }) + ->then(function(){ + echo "Good"; + }) + ->thenIfErrored(function(){ + echo "Bad"; + }) + ->executeInSubprocess(); + + $this->assertContains("Good", $out); + } + + function testDoesntAffectCaughtExceptions() { + $chain = new ErrorControlChainTest_Chain(); + + list($out, $code) = $chain + ->then(function(){ + try { + throw new Exception('Error'); + } + catch (Exception $e) { + echo "Good"; + } + }) + ->thenIfErrored(function(){ + echo "Bad"; + }) + ->executeInSubprocess(); + + $this->assertContains("Good", $out); + } + + function testDoesntAffectHandledErrors() { + $chain = new ErrorControlChainTest_Chain(); + + list($out, $code) = $chain + ->then(function(){ + set_error_handler(function(){ /* NOP */ }); + user_error('Error', E_USER_ERROR); + }) + ->then(function(){ + echo "Good"; + }) + ->thenIfErrored(function(){ + echo "Bad"; + }) + ->executeInSubprocess(); + + $this->assertContains("Good", $out); + } } \ No newline at end of file