Fixing incorrect exit codes on console exceptions.

Uncaught exceptions on the console would result in exit code 0. This is not helpful in *nix scripting. - Update the ConsoleErrorHandler to not inherit from the web error handler. - Make ErrorHandler::_mapErrorCode() public as it isn't overly private. - Make ConsoleErrorHandler methods instance methods so mocking can be done. - Add a _stop method to correctly exit. - Update tests.
cakephp · Oct 4, 2011 · aa7448e · aa7448e
1 parent bab131b
commit aa7448e
Show file tree

Hide file tree

Showing 4 changed files with 43 additions and 14 deletions.
diff --git a/lib/Cake/Console/ConsoleErrorHandler.php b/lib/Cake/Console/ConsoleErrorHandler.php
@@ -25,7 +25,7 @@
  *
  * @package       Cake.Console
  */
-class ConsoleErrorHandler extends ErrorHandler {
+class ConsoleErrorHandler {
 
 /**
  * Standard error stream.
@@ -52,12 +52,13 @@ public static function getStderr() {
  * @param Exception $exception The exception to handle
  * @return void
  */
-	public static function handleException(Exception $exception) {
+	public function handleException(Exception $exception) {
 		$stderr = self::getStderr();
 		$stderr->write(__d('cake_console', "<error>Error:</error> %s\n%s",
 			$exception->getMessage(),
 			$exception->getTraceAsString()
 		));
+		$this->_stop($exception->getCode() ? $exception->getCode() : 1);
 	}
 
 /**
@@ -71,12 +72,12 @@ public static function handleException(Exception $exception) {
  * @param array $context The backtrace of the error.
  * @return void
  */
-	public static function handleError($code, $description, $file = null, $line = null, $context = null) {
+	public function handleError($code, $description, $file = null, $line = null, $context = null) {
 		if (error_reporting() === 0) {
 			return;
 		}
 		$stderr = self::getStderr();
-		list($name, $log) = self::_mapErrorCode($code);
+		list($name, $log) = ErrorHandler::mapErrorCode($code);
 		$message = __d('cake_console', '%s in [%s, line %s]', $description, $file, $line);
 		$stderr->write(__d('cake_console', "<error>%s Error:</error> %s\n", $name, $message));
 
@@ -85,4 +86,13 @@ public static function handleError($code, $description, $file = null, $line = nu
 		}
 	}
 
+/**
+ * Wrapper for exit(), used for testing.
+ *
+ * @param $code int The exit code.
+ */
+	protected function _stop($code = 0) {
+		exit($code);
+	}
+
 }
diff --git a/lib/Cake/Console/ShellDispatcher.php b/lib/Cake/Console/ShellDispatcher.php
@@ -135,8 +135,9 @@ protected function _bootstrap() {
 			App::build();
 		}
 		require_once CAKE . 'Console' . DS . 'ConsoleErrorHandler.php';
-		set_exception_handler(array('ConsoleErrorHandler', 'handleException'));
-		set_error_handler(array('ConsoleErrorHandler', 'handleError'), Configure::read('Error.level'));
+		$ErrorHandler = new ConsoleErrorHandler();
+		set_exception_handler(array($ErrorHandler, 'handleException'));
+		set_error_handler(array($ErrorHandler, 'handleError'), Configure::read('Error.level'));
 
 		if (!defined('FULL_BASE_URL')) {
 			define('FULL_BASE_URL', 'http://localhost');

diff --git a/lib/Cake/Error/ErrorHandler.php b/lib/Cake/Error/ErrorHandler.php
@@ -157,7 +157,7 @@ public static function handleError($code, $description, $file = null, $line = nu
 			return false;
 		}
 		$errorConfig = Configure::read('Error');
-		list($error, $log) = self::_mapErrorCode($code);
+		list($error, $log) = self::mapErrorCode($code);
 
 		$debug = Configure::read('debug');
 		if ($debug) {
@@ -189,7 +189,7 @@ public static function handleError($code, $description, $file = null, $line = nu
  * @param integer $code Error code to map
  * @return array Array of error word, and log location.
  */
-	protected static function _mapErrorCode($code) {
+	public static function mapErrorCode($code) {
 		$error = $log = null;
 		switch ($code) {
 			case E_PARSE:

diff --git a/lib/Cake/Test/Case/Console/ConsoleErrorHandlerTest.php b/lib/Cake/Test/Case/Console/ConsoleErrorHandlerTest.php
@@ -33,6 +33,7 @@ class ConsoleErrorHandlerTest extends CakeTestCase {
  */
 	public function setUp() {
 		parent::setUp();
+		$this->Error = $this->getMock('ConsoleErrorHandler', array('_stop'));
 		ConsoleErrorHandler::$stderr = $this->getMock('ConsoleOutput', array(), array(), '', false);
 	}
 
@@ -42,6 +43,7 @@ public function setUp() {
  * @return void
  */
 	public function tearDown() {
+		unset($this->Error);
 		parent::tearDown();
 	}
 
@@ -55,7 +57,7 @@ public function testHandleError() {
 		ConsoleErrorHandler::$stderr->expects($this->once())->method('write')
 			->with($content);
 
-		ConsoleErrorHandler::handleError(E_NOTICE, 'This is a notice error', '/some/file', 275);
+		$this->Error->handleError(E_NOTICE, 'This is a notice error', '/some/file', 275);
 	}
 
 /**
@@ -68,7 +70,11 @@ public function testCakeErrors() {
 		ConsoleErrorHandler::$stderr->expects($this->once())->method('write')
 			->with($this->stringContains('Missing action'));
 
-		ConsoleErrorHandler::handleException($exception);
+		$this->Error->expects($this->once())
+			->method('_stop')
+			->with(404);
+
+		$this->Error->handleException($exception);
 	}
 
 /**
@@ -82,7 +88,11 @@ public function testNonCakeExceptions() {
 		ConsoleErrorHandler::$stderr->expects($this->once())->method('write')
 			->with($this->stringContains('Too many parameters.'));
 
-		ConsoleErrorHandler::handleException($exception);
+		$this->Error->expects($this->once())
+			->method('_stop')
+			->with(1);
+
+		$this->Error->handleException($exception);
 	}
 
 /**
@@ -96,7 +106,11 @@ public function testError404Exception() {
 		ConsoleErrorHandler::$stderr->expects($this->once())->method('write')
 			->with($this->stringContains('dont use me in cli.'));
 
-		ConsoleErrorHandler::handleException($exception);
+		$this->Error->expects($this->once())
+			->method('_stop')
+			->with(404);
+
+		$this->Error->handleException($exception);
 	}
 
 /**
@@ -110,7 +124,11 @@ public function testError500Exception() {
 		ConsoleErrorHandler::$stderr->expects($this->once())->method('write')
 			->with($this->stringContains('dont use me in cli.'));
 
-		ConsoleErrorHandler::handleException($exception);
+		$this->Error->expects($this->once())
+			->method('_stop')
+			->with(500);
+
+		$this->Error->handleException($exception);
 	}
 
-}
+}