From 295526c27b3978363530060f399c153e36f4758e Mon Sep 17 00:00:00 2001 From: Victor Boctor Date: Thu, 7 Dec 2017 11:35:03 -0800 Subject: [PATCH] Support exceptions as a way to trigger errors MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add MantisException, ServiceException, and ClientException. - Add unhandled exception handler that forward to trigger error on error mode is display. - Force termination on case of E_USER_ERROR (aliased to ERROR in Mantis). This allows replacing trigger_error( …, ERROR) with throwing the appropriate exception while maintaining backward compatibility for UI code. --- core.php | 12 ++++- core/commands/CommandException.php | 44 --------------- core/commands/MonitorCommand.php | 15 +++--- core/error_api.php | 81 ++++++++++++++++++++++++++-- core/exceptions/ClientException.php | 19 +++++++ core/exceptions/MantisException.php | 14 +++++ core/exceptions/ServiceException.php | 18 +++++++ 7 files changed, 145 insertions(+), 58 deletions(-) delete mode 100644 core/commands/CommandException.php create mode 100644 core/exceptions/ClientException.php create mode 100644 core/exceptions/MantisException.php create mode 100644 core/exceptions/ServiceException.php diff --git a/core.php b/core.php index 5aa7f77042..b84aeab73d 100644 --- a/core.php +++ b/core.php @@ -160,9 +160,10 @@ function http_is_protocol_https() { * @return void */ function __autoload( $p_class ) { + global $g_core_path; + # Commands if( substr( $p_class, -7 ) === 'Command' ) { - global $g_core_path; $t_require_path = $g_core_path . 'commands/' . $p_class . '.php'; if( file_exists( $t_require_path ) ) { require_once( $t_require_path ); @@ -170,6 +171,15 @@ function __autoload( $p_class ) { } } + # Exceptions + if( substr( $p_class, -9 ) === 'Exception' ) { + $t_require_path = $g_core_path . 'exceptions/' . $p_class . '.php'; + if( file_exists( $t_require_path ) ) { + require_once( $t_require_path ); + return; + } + } + global $g_class_path; global $g_library_path; diff --git a/core/commands/CommandException.php b/core/commands/CommandException.php deleted file mode 100644 index 3933e206c8..0000000000 --- a/core/commands/CommandException.php +++ /dev/null @@ -1,44 +0,0 @@ -error_code = $p_error_code; - - # TODO: construct error message similar to trigger errors. - $this->error_message = 'error messag for ' . $p_error_code; - } - - /** - * Error code used to fetch localized error message and used for application logic to know what happened if needed. - * - * @var integer The error code as defined in constants and lang files. - */ - public $error_code; - - /** - * Localized error messaged displayed in UI. - * - * @var string The localized error message. - */ - public $error_message; - - function getHttpErrorCode() { - return parent::getCode(); - } - - function getHttpErrorMessage() { - return parent::getMessage(); - } - - function getMantisErrorCode() { - return $this->error_code; - } - - function getUIErrorMessage() { - return $this->error_message; - } -} diff --git a/core/commands/MonitorCommand.php b/core/commands/MonitorCommand.php index 0b078a7f49..983bd2b9df 100644 --- a/core/commands/MonitorCommand.php +++ b/core/commands/MonitorCommand.php @@ -27,17 +27,17 @@ function __construct( array $p_data ) { function validate() { # Validate issue id if( !isset( $this->data['issue_id'] ) ) { - throw new CommandException( HTTP_STATUS_BAD_REQUEST, 'issue_id missing', ERROR_GPC_VAR_NOT_FOUND ); + throw new ClientException( 'issue_id missing', ERROR_GPC_VAR_NOT_FOUND ); } if( !is_numeric( $this->data['issue_id'] ) ) { - throw new CommandException( HTTP_STATUS_BAD_REQUEST, 'issue_id must be a valid issue id', ERROR_GPC_NOT_NUMBER ); + throw new ClientException( 'issue_id must be numeric', ERROR_GPC_VAR_NOT_FOUND ); } $t_issue_id = (int)$this->data['issue_id']; if( !bug_exists( $t_issue_id ) ) { - throw new CommandException( HTTP_STATUS_NOT_FOUND, "Issue id {$t_issue_id} not found", ERROR_BUG_NOT_FOUND ); + throw new ClientException( "Issue id {$t_issue_id} not found", ERROR_BUG_NOT_FOUND, ERROR_GPC_VAR_NOT_FOUND ); } $this->projectId = bug_get_field( $t_issue_id, 'project_id' ); @@ -46,7 +46,7 @@ function validate() { # Validate user id (if specified), otherwise set from context if( !isset( $this->data['users'] ) ) { if( !auth_is_user_authenticated() ) { - throw new CommandException( HTTP_STATUS_BAD_REQUEST, 'user_id missing', ERROR_GPC_VAR_NOT_FOUND ); + throw new ClientException( 'user_id missing', ERROR_GPC_VAR_NOT_FOUND ); } $this->data['users'] = array( 'id' => auth_get_current_user_id() ); @@ -66,9 +66,7 @@ function validate() { $this->userIdsToAdd = array(); foreach( $t_user_ids as $t_user_id ) { if( user_is_anonymous( $t_user_id ) ) { - # TODO: trigger exception - # trigger_error( ERROR_PROTECTED_ACCOUNT, E_USER_ERROR ); - continue; + throw new ClientException( "anonymous account can't monitor issues", ERROR_PROTECTED_ACCOUNT ); } if( $t_logged_in_user == $t_user_id ) { @@ -84,8 +82,7 @@ function validate() { $this->projectId ); if( !access_has_bug_level( $t_access_level, $t_issue_id ) ) { - # TODO: trigger error - continue; + throw new ClientException( 'access denied', ERROR_ACCESS_DENIED ); } $this->userIdsToAdd[] = $t_user_id; diff --git a/core/error_api.php b/core/error_api.php index 280fe72962..db5489fdc3 100644 --- a/core/error_api.php +++ b/core/error_api.php @@ -38,6 +38,8 @@ require_api( 'html_api.php' ); require_api( 'lang_api.php' ); +require_once( dirname( __FILE__ ) . '/exceptions/MantisException.php' ); + $g_error_parameters = array(); $g_errors_delayed = array(); $g_error_handled = false; @@ -49,6 +51,55 @@ error_reporting( error_reporting() | E_USER_ERROR | E_USER_WARNING | E_USER_NOTICE | E_USER_DEPRECATED ); set_error_handler( 'error_handler' ); +set_exception_handler( 'error_exception_handler' ); + +$g_exception = null; + +/** + * Unhandled exception handler + * + * @param Exception|Error $p_exception The exception to handle + * @return void + */ +function error_exception_handler( $p_exception ) { + global $g_exception; + + # As per PHP documentation, null may be received to reset handler to default state. + if( $p_exception === null ) { + $g_exception = null; + return; + } + + $g_exception = $p_exception; + + if( is_subclass_of( $p_exception, 'MantisException' ) ) { + $t_params = $p_exception->getParams(); + if( !empty( $t_params ) ) { + call_user_func_array( 'error_parameters', $t_params ); + } + + trigger_error( $p_exception->getCode(), ERROR ); + + # It is not expected to get here, but just in case! + return; + } + + # trigger a generic error + # TODO: we may want to log such errors + trigger_error( ERROR_GENERIC, E_ERROR ); +} + +/** + * Get error stack based on last exception + * @return array The stack trace as an array + */ +function error_stack_trace() { + global $g_exception; + + return ($g_exception === null ) ? + debug_backtrace() : + $g_exception->getTrace(); +} /** * Default error handler @@ -151,7 +202,7 @@ function error_handler( $p_type, $p_error, $p_file, $p_line, array $p_context ) case E_USER_DEPRECATED: # Get the parent of the call that triggered the error to facilitate # debugging with a more useful filename and line number - $t_stack = debug_backtrace(); + $t_stack = error_stack_trace(); if( isset( $t_stack[2] ) ) { $t_caller = $t_stack[2]; } else { @@ -187,7 +238,7 @@ function error_handler( $p_type, $p_error, $p_file, $p_line, array $p_context ) if( ON == config_get_global( 'show_detailed_errors' ) ) { echo "\n"; - debug_print_backtrace(); + error_print_stack_trace(); } } if( DISPLAY_ERROR_HALT == $t_method ) { @@ -417,12 +468,34 @@ function error_print_context( array $p_context ) { * Print out a stack trace */ function error_print_stack_trace() { + $t_stack = error_stack_trace(); + + if( php_sapi_name() == 'cli' ) { + foreach( $t_stack as $t_frame ) { + echo ( isset( $t_frame['file'] ) ? $t_frame['file'] : '-' ), ': ' , + ( isset( $t_frame['line'] ) ? $t_frame['line'] : '-' ), ': ', + ( isset( $t_frame['class'] ) ? $t_frame['class'] : '-' ), ' - ', + ( isset( $t_frame['type'] ) ? $t_frame['type'] : '-' ), ' - ', + ( isset( $t_frame['function'] ) ? $t_frame['function'] : '-' ); + + $t_args = array(); + if( isset( $t_frame['args'] ) && !empty( $t_frame['args'] ) ) { + foreach( $t_frame['args'] as $t_value ) { + $t_args[] = error_build_parameter_string( $t_value ); + } + echo '(', implode( $t_args, ', ' ), ' )', "\n"; + } else { + echo "()\n"; + } + } + + return; + } + echo '
'; echo ''; echo ''; - $t_stack = debug_backtrace(); - array_shift( $t_stack ); # remove the call to this function from the stack trace diff --git a/core/exceptions/ClientException.php b/core/exceptions/ClientException.php new file mode 100644 index 0000000000..1472451a86 --- /dev/null +++ b/core/exceptions/ClientException.php @@ -0,0 +1,19 @@ +params = $p_params; + } + + function getParams() { + return $this->params; + } +} \ No newline at end of file diff --git a/core/exceptions/ServiceException.php b/core/exceptions/ServiceException.php new file mode 100644 index 0000000000..dd74bf1202 --- /dev/null +++ b/core/exceptions/ServiceException.php @@ -0,0 +1,18 @@ +
FilenameLineFunctionArgs