Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP
Browse files

FIX Nice errors and allows flush on module removal

  • Loading branch information...
commit a1ea905ca8f38e65f6a8d98f571b3adce1c641c7 1 parent 84011aa
@hafriedlander hafriedlander authored
View
39 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 @@ public function setErrored($error) {
public function setSuppression($suppression) {
$this->suppression = (bool)$suppression;
+ if ($this->handleFatalErrors) ini_set('display_errors', !$suppression);
}
/**
@@ -68,24 +69,14 @@ public function thenAlways($callback) {
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 @@ public function handleFatalError() {
}
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 @@ protected function step() {
$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 @@ protected function step() {
else {
// Now clean up
$this->handleFatalErrors = false;
- restore_error_handler();
+ ini_set('display_errors', $this->originalDisplayErrors);
}
}
}
View
245 tests/core/startup/ErrorControlChainTest.php
@@ -1,90 +1,241 @@
<?php
+/**
+ * An extension of ErrorControlChain that runs the chain in a subprocess.
+ *
+ * We need this because ErrorControlChain only suppresses uncaught fatal errors, and
+ * that would kill PHPUnit execution
+ */
+class ErrorControlChainTest_Chain extends ErrorControlChain {
+
+ function executeInSubprocess() {
+ // Get the path to the ErrorControlChain class
+ $classpath = SS_ClassLoader::instance()->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 setUp() {
+ // Check we can run PHP at all
+ $null = is_writeable('/dev/null') ? '/dev/null' : 'NUL';
+ exec("php -v 2> $null", $out, $rv);
+
+ if ($rv != 0) {
+ $this->markTestSkipped("Can't run PHP from the command line - is it in your path?");
+ $this->skipTest = true;
+ }
+
+ parent::setUp();
+ }
+
function testErrorSuppression() {
- $chain = new ErrorControlChain();
- $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
+ })
+ ->thenIfErrored(function(){
+ echo "Done";
})
- ->execute();
+ ->executeInSubprocess();
- $this->assertTrue($chain->hasErrored());
- }
+ $this->assertEquals('Done', $out);
+
+ // User error
- function testMultipleErrorSuppression() {
- $chain = new ErrorControlChain();
+ $chain = new ErrorControlChainTest_Chain();
- $chain
+ list($out, $code) = $chain
->then(function(){
- user_error('This error should be suppressed', E_USER_ERROR);
+ user_error('Error', E_USER_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);
- function testExceptionSuppression() {
- $chain = new ErrorControlChain();
+ // Recoverable error
- $chain
+ $chain = new ErrorControlChainTest_Chain();
+
+ list($out, $code) = $chain
->then(function(){
- throw new Exception('This exception should be suppressed');
+ $x = function(ErrorControlChain $foo){ };
+ $x(1); // Calling against type
})
- ->execute();
+ ->thenIfErrored(function(){
+ echo "Done";
+ })
+ ->executeInSubprocess();
- $this->assertTrue($chain->hasErrored());
+ $this->assertEquals('Done', $out);
}
- function testMultipleExceptionSuppression() {
- $chain = new ErrorControlChain();
+ function testExceptionSuppression() {
+ $chain = new ErrorControlChainTest_Chain();
- $chain
+ list($out, $code) = $chain
->then(function(){
throw new Exception('This exception should be suppressed');
})
- ->thenAlways(function(){
- throw new Exception('This exception should also be suppressed');
+ ->thenIfErrored(function(){
+ echo "Done";
})
- ->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'
+ "preThen,preThenAlways,postThenIfErrored,postThenAlways,",
+ $out
);
+ }
- $this->assertEquals(
- array('then' => false, 'thenIfErrored' => true, 'thenAlways' => true),
- $postError,
- 'thenIfErrored and thenAlways callbacks called after error, then callback not called'
- );
+ 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);
+ }
}
Please sign in to comment.
Something went wrong with that request. Please try again.