From 6fcac324104499b5f570df358a4065997538997d Mon Sep 17 00:00:00 2001 From: Qiang Xue Date: Thu, 31 Jan 2013 23:50:53 -0500 Subject: [PATCH] error handling cleanup. --- framework/YiiBase.php | 5 + framework/base/Application.php | 193 ++++++++++++++------- framework/base/ErrorHandler.php | 65 +------ framework/console/Application.php | 8 +- framework/console/Controller.php | 2 +- framework/console/controllers/HelpController.php | 4 +- .../console/controllers/MigrateController.php | 139 +++++++++------ tests/unit/bootstrap.php | 1 - 8 files changed, 231 insertions(+), 186 deletions(-) diff --git a/framework/YiiBase.php b/framework/YiiBase.php index 14e0f44..d01648c 100644 --- a/framework/YiiBase.php +++ b/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. diff --git a/framework/base/Application.php b/framework/base/Application.php index 6203d11..e797ab7 100644 --- a/framework/base/Application.php +++ b/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. - * The default value is `array('errorHandler')`, which loads the [[errorHandler]] component - * to ensure errors and exceptions can be handled nicely. + * @var array IDs of the components that need to be loaded when the application starts. */ - 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); } @@ -253,61 +257,61 @@ class Application extends Module date_default_timezone_set($value); } -// /** -// * Returns the security manager component. -// * @return SecurityManager the security manager application component. -// */ -// public function getSecurityManager() -// { -// return $this->getComponent('securityManager'); -// } -// -// /** -// * Returns the locale instance. -// * @param string $localeID the locale ID (e.g. en_US). If null, the {@link getLanguage application language ID} will be used. -// * @return CLocale the locale instance -// */ -// public function getLocale($localeID = null) -// { -// return CLocale::getInstance($localeID === null ? $this->getLanguage() : $localeID); -// } -// -// /** -// * @return CNumberFormatter the locale-dependent number formatter. -// * The current {@link getLocale application locale} will be used. -// */ -// public function getNumberFormatter() -// { -// return $this->getLocale()->getNumberFormatter(); -// } -// -// /** -// * Returns the locale-dependent date formatter. -// * @return CDateFormatter the locale-dependent date formatter. -// * The current {@link getLocale application locale} will be used. -// */ -// public function getDateFormatter() -// { -// return $this->getLocale()->getDateFormatter(); -// } -// -// /** -// * Returns the core message translations component. -// * @return \yii\i18n\MessageSource the core message translations -// */ -// public function getCoreMessages() -// { -// return $this->getComponent('coreMessages'); -// } -// -// /** -// * Returns the application message translations component. -// * @return \yii\i18n\MessageSource the application message translations -// */ -// public function getMessages() -// { -// return $this->getComponent('messages'); -// } + // /** + // * Returns the security manager component. + // * @return SecurityManager the security manager application component. + // */ + // public function getSecurityManager() + // { + // return $this->getComponent('securityManager'); + // } + // + // /** + // * Returns the locale instance. + // * @param string $localeID the locale ID (e.g. en_US). If null, the {@link getLanguage application language ID} will be used. + // * @return CLocale the locale instance + // */ + // public function getLocale($localeID = null) + // { + // return CLocale::getInstance($localeID === null ? $this->getLanguage() : $localeID); + // } + // + // /** + // * @return CNumberFormatter the locale-dependent number formatter. + // * The current {@link getLocale application locale} will be used. + // */ + // public function getNumberFormatter() + // { + // return $this->getLocale()->getNumberFormatter(); + // } + // + // /** + // * Returns the locale-dependent date formatter. + // * @return CDateFormatter the locale-dependent date formatter. + // * The current {@link getLocale application locale} will be used. + // */ + // public function getDateFormatter() + // { + // return $this->getLocale()->getDateFormatter(); + // } + // + // /** + // * Returns the core message translations component. + // * @return \yii\i18n\MessageSource the core message translations + // */ + // public function getCoreMessages() + // { + // return $this->getComponent('coreMessages'); + // } + // + // /** + // * Returns the application message translations component. + // * @return \yii\i18n\MessageSource the application message translations + // */ + // public function getMessages() + // { + // return $this->getComponent('messages'); + // } /** * Returns the database connection component. @@ -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 : '
' . $message . '
'; + } + + $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); + } } diff --git a/framework/base/ErrorHandler.php b/framework/base/ErrorHandler.php index 5b48fbf..e544a49 100644 --- a/framework/base/ErrorHandler.php +++ b/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); - } + $this->render($exception); } 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 diff --git a/framework/console/Application.php b/framework/console/Application.php index 6ad40cc..64c17e5 100644 --- a/framework/console/Application.php +++ b/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) + { + + } } diff --git a/framework/console/Controller.php b/framework/console/Controller.php index ce4c233..16968f2 100644 --- a/framework/console/Controller.php +++ b/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); diff --git a/framework/console/controllers/HelpController.php b/framework/console/controllers/HelpController.php index c663b2b..6e4b397 100644 --- a/framework/console/controllers/HelpController.php +++ b/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) { diff --git a/framework/console/controllers/MigrateController.php b/framework/console/controllers/MigrateController.php index b0804c3..e104856 100644 --- a/framework/console/controllers/MigrateController.php +++ b/framework/console/controllers/MigrateController.php @@ -99,17 +99,19 @@ class MigrateController extends Controller public function beforeAction($action) { - $path = Yii::getAlias($this->migrationPath); - if ($path === false || !is_dir($path)) { - echo 'Error: the migration directory does not exist "' . $this->migrationPath . "\"\n"; + 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(); + echo "\nYii Migration Tool v2.0 (based on Yii v{$version})\n\n"; + return true; + } else { return false; } - $this->migrationPath = $path; - - $yiiVersion = Yii::getVersion(); - echo "\nYii Migration Tool v2.0 (based on Yii v{$yiiVersion})\n\n"; - - return parent::beforeAction($action); } /** @@ -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 - { - echo "Total $n out of $total new " . ($total === 1 ? 'migration' : 'migrations') . " to be applied:\n"; - } + } 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,16 +321,18 @@ 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"; + } } } @@ -326,30 +340,34 @@ class MigrateController extends Controller { $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,12 +439,15 @@ class MigrateController extends Controller protected function getDb() { - if ($this->_db !== null) - return $this->_db; - else if (($this->_db = Yii::$application->getComponent($this->connectionID)) instanceof CDbConnection) + if ($this->_db !== null) { return $this->_db; - else - die("Error: CMigrationCommand.connectionID '{$this->connectionID}' is invalid. Please make sure it refers to the ID of a CDbConnection application component.\n"); + } else { + if (($this->_db = Yii::$application->getComponent($this->connectionID)) instanceof CDbConnection) { + return $this->_db; + } 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,17 +482,20 @@ 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); @@ -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 <<