error handling cleanup.

framework/YiiBase.php

@@ -29,6 +29,11 @@ defined('YII_TRACE_LEVEL') or define('YII_TRACE_LEVEL', 0);
  * This constant defines the framework installation directory.
  */
 defined('YII_PATH') or define('YII_PATH', __DIR__);
+/**
+ * This constant defines whether error handling should be enabled. Defaults to true.
+ */
+defined('YII_ENABLE_ERROR_HANDLER') or define('YII_ENABLE_ERROR_HANDLER', true);
+
 
 /**
  * YiiBase is the core helper class for the Yii framework.

framework/base/Application.php

@@ -11,7 +11,6 @@ namespace yii\base;
 
 use Yii;
 use yii\util\FileHelper;
-use yii\base\InvalidCallException;
 
 /**
  * Application is the base class for all application classes.
@@ -76,11 +75,9 @@ class Application extends Module
 	 */
 	public $sourceLanguage = 'en_us';
 	/**
-	 * @var array IDs of application components that need to be loaded when the application starts.
+	 * @var array IDs of the components that need to be loaded when the application starts.
-	 * The default value is `array('errorHandler')`, which loads the [[errorHandler]] component
-	 * to ensure errors and exceptions can be handled nicely.
 	 */
-	public $preload = array('errorHandler');
+	public $preload = array();
 	/**
 	 * @var Controller the currently active controller instance
 	 */
@@ -110,8 +107,15 @@ class Application extends Module
 		Yii::$application = $this;
 		$this->id = $id;
 		$this->setBasePath($basePath);
+
+		if (YII_ENABLE_ERROR_HANDLER) {
+			set_exception_handler(array($this, 'handleException'));
+			set_error_handler(array($this, 'handleError'), error_reporting());
+		}
+
 		$this->registerDefaultAliases();
 		$this->registerCoreComponents();
+
 		Component::__construct($config);
 	}
 
@@ -390,4 +394,73 @@ class Application extends Module
 			),
 		));
 	}
+
+	/**
+	 * Handles PHP execution errors such as warnings, notices.
+	 *
+	 * This method is used as a PHP error handler. It will simply raise an `ErrorException`.
+	 *
+	 * @param integer $code the level of the error raised
+	 * @param string $message the error message
+	 * @param string $file the filename that the error was raised in
+	 * @param integer $line the line number the error was raised at
+	 * @throws \ErrorException the error exception
+	 */
+	public function handleError($code, $message, $file, $line)
+	{
+		if (error_reporting() !== 0) {
+			throw new \ErrorException($message, 0, $code, $file, $line);
+		}
+	}
+
+	/**
+	 * Handles uncaught PHP exceptions.
+	 *
+	 * This method is implemented as a PHP exception handler. It requires
+	 * that constant YII_ENABLE_ERROR_HANDLER be defined true.
+	 *
+	 * @param \Exception $exception exception that is not caught
+	 */
+	public function handleException($exception)
+	{
+		// disable error capturing to avoid recursive errors while handling exceptions
+		restore_error_handler();
+		restore_exception_handler();
+
+		try {
+			$this->logException($exception);
+
+			if (($handler = $this->getErrorHandler()) !== null) {
+				$handler->handle($exception);
+			} else {
+				$message = YII_DEBUG ? (string)$exception : 'Error: ' . $exception->getMessage() . "\n";
+				echo PHP_SAPI === 'cli' ? $message : '<pre>' . $message . '</pre>';
+			}
+
+			$this->end(1);
+
+		} catch(\Exception $e) {
+			// exception could be thrown in end() or ErrorHandler::handle()
+			$msg = (string)$e;
+			$msg .= "\nPrevious exception:\n";
+			$msg .= (string)$exception;
+			$msg .= "\n\$_SERVER = " . var_export($_SERVER, true);
+			error_log($msg);
+			exit(1);
+		}
+	}
+
+	// todo: to be polished
+	protected function logException($exception)
+	{
+		$category = get_class($exception);
+		if ($exception instanceof HttpException) {
+			/** @var $exception HttpException */
+			$category .= '\\' . $exception->statusCode;
+		} elseif ($exception instanceof \ErrorException) {
+			/** @var $exception \ErrorException */
+			$category .= '\\' . $exception->getSeverity();
+		}
+		Yii::error((string)$exception, $category);
+	}
 }

framework/base/ErrorHandler.php

@@ -54,66 +54,18 @@ class ErrorHandler extends Component
 	public $exception;
 
 
-	public function init()
-	{
-		set_exception_handler(array($this, 'handleException'));
-		set_error_handler(array($this, 'handleError'), error_reporting());
-	}
-
-	/**
-	 * Handles PHP execution errors such as warnings, notices.
-	 *
-	 * This method is used as a PHP error handler. It will simply raise an `ErrorException`.
-	 *
-	 * @param integer $code the level of the error raised
-	 * @param string $message the error message
-	 * @param string $file the filename that the error was raised in
-	 * @param integer $line the line number the error was raised at
-	 * @throws \ErrorException the error exception
-	 */
-	public function handleError($code, $message, $file, $line)
-	{
-		if(error_reporting()!==0) {
-			throw new \ErrorException($message, 0, $code, $file, $line);
-		}
-	}
-
 	/**
 	 * @param \Exception $exception
 	 */
-	public function handleException($exception)
+	public function handle($exception)
 	{
-		// disable error capturing to avoid recursive errors while handling exceptions
-		restore_error_handler();
-		restore_exception_handler();
-
 		$this->exception = $exception;
-		$this->logException($exception);
 
 		if ($this->discardExistingOutput) {
 			$this->clearOutput();
 		}
 
-		try {
 		$this->render($exception);
-		} catch (\Exception $e) {
-			// use the most primitive way to display exception thrown in the error view
-			$this->renderAsText($e);
-		}
-
-
-		try {
-			\Yii::$application->end(1);
-		} catch (Exception $e2) {
-			// use the most primitive way to log error occurred in end()
-			$msg = get_class($e2) . ': ' . $e2->getMessage() . ' (' . $e2->getFile() . ':' . $e2->getLine() . ")\n";
-			$msg .= $e2->getTraceAsString() . "\n";
-			$msg .= "Previous error:\n";
-			$msg .= $e2->getTraceAsString() . "\n";
-			$msg .= '$_SERVER=' . var_export($_SERVER, true);
-			error_log($msg);
-			exit(1);
-		}
 	}
 
 	protected function render($exception)
@@ -282,19 +234,6 @@ class ErrorHandler extends Component
 		return htmlspecialchars($text, ENT_QUOTES, \Yii::$application->charset);
 	}
 
-	public function logException($exception)
-	{
-		$category = get_class($exception);
-		if ($exception instanceof HttpException) {
-			/** @var $exception HttpException */
-			$category .= '\\' . $exception->statusCode;
-		} elseif ($exception instanceof \ErrorException) {
-			/** @var $exception \ErrorException */
-			$category .= '\\' . $exception->getSeverity();
-		}
-		\Yii::error((string)$exception, $category);
-	}
-
 	public function clearOutput()
 	{
 		// the following manual level counting is to deal with zlib.output_compression set to On

framework/console/Application.php

@@ -85,7 +85,6 @@ class Application extends \yii\base\Application
 	 * Processes the request.
 	 * The request is represented in terms of a controller route and action parameters.
 	 * @return integer the exit status of the controller action (0 means normal, non-zero values mean abnormal)
-	 * @throws Exception if the route cannot be resolved into a controller
 	 */
 	public function processRequest()
 	{
@@ -99,7 +98,6 @@ class Application extends \yii\base\Application
 		}
 	}
 
-
 	/**
 	 * Runs a controller action specified by a route.
 	 * This method parses the specified route and creates the corresponding child module(s), controller and action
@@ -108,7 +106,6 @@ class Application extends \yii\base\Application
 	 * @param string $route the route that specifies the action.
 	 * @param array $params the parameters to be passed to the action
 	 * @return integer the status code returned by the action execution. 0 means normal, and other values mean abnormal.
-	 * @throws InvalidRouteException if the requested route cannot be resolved into an action successfully
 	 */
 	public function runAction($route, $params = array())
 	{
@@ -151,4 +148,9 @@ class Application extends \yii\base\Application
 			),
 		));
 	}
+
+	public function usageError($message)
+	{
+
+	}
 }

framework/console/Controller.php

@@ -103,7 +103,7 @@ class Controller extends \yii\base\Controller
 		}
 	}
 
-	public function error($message)
+	public function usageError($message)
 	{
 		echo "\nError: $message\n";
 		Yii::$application->end(1);

framework/console/controllers/HelpController.php

@@ -55,7 +55,7 @@ class HelpController extends Controller
 		} else {
 			$result = \Yii::$application->createController($args[0]);
 			if ($result === false) {
-				echo "\nError: no help for unknown command \"{$args[0]}\".\n";
+				echo "Error: no help for unknown command \"{$args[0]}\".\n";
 				return 1;
 			}
 
@@ -213,7 +213,7 @@ class HelpController extends Controller
 	{
 		$action = $controller->createAction($actionID);
 		if ($action === null) {
-			echo "Unknown sub-command: " . $controller->getUniqueId() . "/$actionID\n";
+			echo 'Error: no help for unknown sub-command "' . $controller->getUniqueId() . "/$actionID\".\n";
 			return 1;
 		}
 		if ($action instanceof InlineAction) {

framework/console/controllers/MigrateController.php

@@ -99,17 +99,19 @@ class MigrateController extends Controller
 
 	public function beforeAction($action)
 	{
+		if (parent::beforeAction($action)) {
 			$path = Yii::getAlias($this->migrationPath);
 			if ($path === false || !is_dir($path)) {
 				echo 'Error: the migration directory does not exist "' . $this->migrationPath . "\"\n";
 				return false;
 			}
 			$this->migrationPath = $path;
-
+			$version = Yii::getVersion();
-		$yiiVersion = Yii::getVersion();
+			echo "\nYii Migration Tool v2.0 (based on Yii v{$version})\n\n";
-		echo "\nYii Migration Tool v2.0 (based on Yii v{$yiiVersion})\n\n";
+			return true;
-
+		} else {
-		return parent::beforeAction($action);
+			return false;
+		}
 	}
 
 	/**
@@ -129,15 +131,15 @@ class MigrateController extends Controller
 		}
 
 		$n = count($migrations);
-		if ($n === $total)
+		if ($n === $total) {
 			echo "Total $n new " . ($n === 1 ? 'migration' : 'migrations') . " to be applied:\n";
-		else
+		} else {
-				{
 			echo "Total $n out of $total new " . ($total === 1 ? 'migration' : 'migrations') . " to be applied:\n";
 		}
 
-		foreach ($migrations as $migration)
+		foreach ($migrations as $migration) {
 			echo "    $migration\n";
+		}
 		echo "\n";
 
 		if ($this->confirm('Apply the above ' . ($n === 1 ? 'migration' : 'migrations') . "?")) {
@@ -154,8 +156,9 @@ class MigrateController extends Controller
 	public function actionDown($args)
 	{
 		$step = isset($args[0]) ? (int)$args[0] : 1;
-		if ($step < 1)
+		if ($step < 1) {
 			die("Error: The step parameter must be greater than 0.\n");
+		}
 
 		if (($migrations = $this->getMigrationHistory($step)) === array()) {
 			echo "No migration has been done before.\n";
@@ -165,8 +168,9 @@ class MigrateController extends Controller
 
 		$n = count($migrations);
 		echo "Total $n " . ($n === 1 ? 'migration' : 'migrations') . " to be reverted:\n";
-		foreach ($migrations as $migration)
+		foreach ($migrations as $migration) {
 			echo "    $migration\n";
+		}
 		echo "\n";
 
 		if ($this->confirm('Revert the above ' . ($n === 1 ? 'migration' : 'migrations') . "?")) {
@@ -183,8 +187,9 @@ class MigrateController extends Controller
 	public function actionRedo($args)
 	{
 		$step = isset($args[0]) ? (int)$args[0] : 1;
-		if ($step < 1)
+		if ($step < 1) {
 			die("Error: The step parameter must be greater than 0.\n");
+		}
 
 		if (($migrations = $this->getMigrationHistory($step)) === array()) {
 			echo "No migration has been done before.\n";
@@ -194,8 +199,9 @@ class MigrateController extends Controller
 
 		$n = count($migrations);
 		echo "Total $n " . ($n === 1 ? 'migration' : 'migrations') . " to be redone:\n";
-		foreach ($migrations as $migration)
+		foreach ($migrations as $migration) {
 			echo "    $migration\n";
+		}
 		echo "\n";
 
 		if ($this->confirm('Redo the above ' . ($n === 1 ? 'migration' : 'migrations') . "?")) {
@@ -217,16 +223,18 @@ class MigrateController extends Controller
 
 	public function actionTo($args)
 	{
-		if (isset($args[0]))
+		if (isset($args[0])) {
 			$version = $args[0];
-		else
+		} else {
 			$this->usageError('Please specify which version to migrate to.');
+		}
 
 		$originalVersion = $version;
-		if (preg_match('/^m?(\d{6}_\d{6})(_.*?)?$/', $version, $matches))
+		if (preg_match('/^m?(\d{6}_\d{6})(_.*?)?$/', $version, $matches)) {
 			$version = 'm' . $matches[1];
-		else
+		} else {
 			die("Error: The version option must be either a timestamp (e.g. 101129_185401)\nor the full name of a migration (e.g. m101129_185401_create_user_table).\n");
+		}
 
 		// try migrate up
 		$migrations = $this->getNewMigrations();
@@ -241,10 +249,11 @@ class MigrateController extends Controller
 		$migrations = array_keys($this->getMigrationHistory(-1));
 		foreach ($migrations as $i => $migration) {
 			if (strpos($migration, $version . '_') === 0) {
-				if ($i === 0)
+				if ($i === 0) {
 					echo "Already at '$originalVersion'. Nothing needs to be done.\n";
-				else
+				} else {
 					$this->actionDown(array($i));
+				}
 				return;
 			}
 		}
@@ -254,15 +263,17 @@ class MigrateController extends Controller
 
 	public function actionMark($args)
 	{
-		if (isset($args[0]))
+		if (isset($args[0])) {
 			$version = $args[0];
-		else
+		} else {
 			$this->usageError('Please specify which version to mark to.');
+		}
 		$originalVersion = $version;
-		if (preg_match('/^m?(\d{6}_\d{6})(_.*?)?$/', $version, $matches))
+		if (preg_match('/^m?(\d{6}_\d{6})(_.*?)?$/', $version, $matches)) {
 			$version = 'm' . $matches[1];
-		else
+		} else {
 			die("Error: The version option must be either a timestamp (e.g. 101129_185401)\nor the full name of a migration (e.g. m101129_185401_create_user_table).\n");
+		}
 
 		$db = $this->getDb();
 
@@ -288,13 +299,14 @@ class MigrateController extends Controller
 		$migrations = array_keys($this->getMigrationHistory(-1));
 		foreach ($migrations as $i => $migration) {
 			if (strpos($migration, $version . '_') === 0) {
-				if ($i === 0)
+				if ($i === 0) {
 					echo "Already at '$originalVersion'. Nothing needs to be done.\n";
-				else {
+				} else {
 					if ($this->confirm("Set migration history at $originalVersion?")) {
 						$command = $db->createCommand();
-						for ($j = 0; $j < $i; ++$j)
+						for ($j = 0; $j < $i; ++$j) {
 							$command->delete($this->migrationTable, $db->quoteColumnName('version') . '=:version', array(':version' => $migrations[$j]));
+						}
 						echo "The migration history is set at $originalVersion.\nNo actual migration was performed.\n";
 					}
 				}
@@ -309,47 +321,53 @@ class MigrateController extends Controller
 	{
 		$limit = isset($args[0]) ? (int)$args[0] : -1;
 		$migrations = $this->getMigrationHistory($limit);
-		if ($migrations === array())
+		if ($migrations === array()) {
 			echo "No migration has been done before.\n";
-		else {
+		} else {
 			$n = count($migrations);
-			if ($limit > 0)
+			if ($limit > 0) {
 				echo "Showing the last $n applied " . ($n === 1 ? 'migration' : 'migrations') . ":\n";
-			else
+			} else {
 				echo "Total $n " . ($n === 1 ? 'migration has' : 'migrations have') . " been applied before:\n";
-			foreach ($migrations as $version => $time)
+			}
+			foreach ($migrations as $version => $time) {
 				echo "    (" . date('Y-m-d H:i:s', $time) . ') ' . $version . "\n";
 			}
 		}
+	}
 
 	public function actionNew($args)
 	{
 		$limit = isset($args[0]) ? (int)$args[0] : -1;
 		$migrations = $this->getNewMigrations();
-		if ($migrations === array())
+		if ($migrations === array()) {
 			echo "No new migrations found. Your system is up-to-date.\n";
-		else {
+		} else {
 			$n = count($migrations);
 			if ($limit > 0 && $n > $limit) {
 				$migrations = array_slice($migrations, 0, $limit);
 				echo "Showing $limit out of $n new " . ($n === 1 ? 'migration' : 'migrations') . ":\n";
-			} else
+			} else {
 				echo "Found $n new " . ($n === 1 ? 'migration' : 'migrations') . ":\n";
+			}
 
-			foreach ($migrations as $migration)
+			foreach ($migrations as $migration) {
 				echo "    " . $migration . "\n";
 			}
 		}
+	}
 
 	public function actionCreate($args)
 	{
-		if (isset($args[0]))
+		if (isset($args[0])) {
 			$name = $args[0];
-		else
+		} else {
 			$this->usageError('Please provide the name of the new migration.');
+		}
 
-		if (!preg_match('/^\w+$/', $name))
+		if (!preg_match('/^\w+$/', $name)) {
 			die("Error: The name of the migration must contain letters, digits and/or underscore characters only.\n");
+		}
 
 		$name = 'm' . gmdate('ymd_His') . '_' . $name;
 		$content = strtr($this->getTemplate(), array('{ClassName}' => $name));
@@ -363,8 +381,9 @@ class MigrateController extends Controller
 
 	protected function migrateUp($class)
 	{
-		if ($class === self::BASE_MIGRATION)
+		if ($class === self::BASE_MIGRATION) {
 			return;
+		}
 
 		echo "*** applying $class\n";
 		$start = microtime(true);
@@ -385,8 +404,9 @@ class MigrateController extends Controller
 
 	protected function migrateDown($class)
 	{
-		if ($class === self::BASE_MIGRATION)
+		if ($class === self::BASE_MIGRATION) {
 			return;
+		}
 
 		echo "*** reverting $class\n";
 		$start = microtime(true);
@@ -419,13 +439,16 @@ class MigrateController extends Controller
 
 	protected function getDb()
 	{
-		if ($this->_db !== null)
+		if ($this->_db !== null) {
 			return $this->_db;
-		else if (($this->_db = Yii::$application->getComponent($this->connectionID)) instanceof CDbConnection)
+		} else {
+			if (($this->_db = Yii::$application->getComponent($this->connectionID)) instanceof CDbConnection) {
 				return $this->_db;
-		else
+			} else {
 				die("Error: CMigrationCommand.connectionID '{$this->connectionID}' is invalid. Please make sure it refers to the ID of a CDbConnection application component.\n");
 			}
+		}
+	}
 
 	protected function getMigrationHistory($limit)
 	{
@@ -459,18 +482,21 @@ class MigrateController extends Controller
 	protected function getNewMigrations()
 	{
 		$applied = array();
-		foreach ($this->getMigrationHistory(-1) as $version => $time)
+		foreach ($this->getMigrationHistory(-1) as $version => $time) {
 			$applied[substr($version, 1, 13)] = true;
+		}
 
 		$migrations = array();
 		$handle = opendir($this->migrationPath);
 		while (($file = readdir($handle)) !== false) {
-			if ($file === '.' || $file === '..')
+			if ($file === '.' || $file === '..') {
 				continue;
+			}
 			$path = $this->migrationPath . DIRECTORY_SEPARATOR . $file;
-			if (preg_match('/^(m(\d{6}_\d{6})_.*?)\.php$/', $file, $matches) && is_file($path) && !isset($applied[$matches[2]]))
+			if (preg_match('/^(m(\d{6}_\d{6})_.*?)\.php$/', $file, $matches) && is_file($path) && !isset($applied[$matches[2]])) {
 				$migrations[] = $matches[1];
 			}
+		}
 		closedir($handle);
 		sort($migrations);
 		return $migrations;
@@ -478,9 +504,9 @@ class MigrateController extends Controller
 
 	protected function getTemplate()
 	{
-		if ($this->templateFile !== null)
+		if ($this->templateFile !== null) {
 			return file_get_contents(Yii::getPathOfAlias($this->templateFile) . '.php');
-		else
+		} else {
 			return <<<EOD
 <?php
 
@@ -510,3 +536,4 @@ class {ClassName} extends CDbMigration
 EOD;
 		}
 	}
+}

tests/unit/bootstrap.php

@@ -1,6 +1,5 @@
 <?php
 
-define('YII_ENABLE_EXCEPTION_HANDLER', false);
 define('YII_ENABLE_ERROR_HANDLER', false);
 define('YII_DEBUG', true);
 $_SERVER['SCRIPT_NAME'] = '/' . __DIR__;