Skip to content

Commit

Permalink
Support exceptions as a way to trigger errors
Browse files Browse the repository at this point in the history
- 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.
  • Loading branch information
vboctor committed Dec 30, 2017
1 parent f9d45aa commit 295526c
Show file tree
Hide file tree
Showing 7 changed files with 145 additions and 58 deletions.
12 changes: 11 additions & 1 deletion core.php
Expand Up @@ -160,16 +160,26 @@ 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 );
return;
}
}

# 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;

Expand Down
44 changes: 0 additions & 44 deletions core/commands/CommandException.php

This file was deleted.

15 changes: 6 additions & 9 deletions core/commands/MonitorCommand.php
Expand Up @@ -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' );
Expand All @@ -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() );
Expand All @@ -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 ) {
Expand All @@ -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;
Expand Down
81 changes: 77 additions & 4 deletions core/error_api.php
Expand Up @@ -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;
Expand All @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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 ) {
Expand Down Expand Up @@ -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 '<div class="table-responsive">';
echo '<table class="table table-bordered table-striped table-condensed">';
echo '<tr><th>Filename</th><th>Line</th><th></th><th></th><th>Function</th><th>Args</th></tr>';

$t_stack = debug_backtrace();

array_shift( $t_stack );

# remove the call to this function from the stack trace
Expand Down
19 changes: 19 additions & 0 deletions core/exceptions/ClientException.php
@@ -0,0 +1,19 @@
<?php
/**
* An exception that is triggered where the error is caused by
* client input.
*/
class ClientException extends MantisException {
/**
* Constructor
*
* @param string $p_message The internal non-localized error message.
* @param integer $p_code The Mantis error code.
* @param array $p_params Localized error message parameters.
* @param Throwable $p_previous The inner exception.
* @return void
*/
function __construct( $p_message, $p_code, $p_params = array(), Throwable $p_previous = null ) {
parent::__construct( $p_message, $p_code, $p_params, $p_previous );
}
}
14 changes: 14 additions & 0 deletions core/exceptions/MantisException.php
@@ -0,0 +1,14 @@
<?php

class MantisException extends Exception {
protected $params;

function __construct( $p_message, $p_code, $p_params = array(), Throwable $p_previous = null ) {
parent::__construct( $p_message, $p_code, $p_previous );
$this->params = $p_params;
}

function getParams() {
return $this->params;
}
}
18 changes: 18 additions & 0 deletions core/exceptions/ServiceException.php
@@ -0,0 +1,18 @@
<?php
/**
* An exception that is triggered due to a Mantis error.
*/
class ServiceException extends MantisException {
/**
* Constructor
*
* @param string $p_message The internal non-localized error message.
* @param integer $p_code The Mantis error code.
* @param array $p_params Localized error message parameters.
* @param Throwable $p_previous The inner exception.
* @return void
*/
function __construct( $p_message, $p_code, $p_params = array(), Throwable $p_previous = null ) {
parent::__construct( $p_message, $p_code, $p_params, $p_previous );
}
}

0 comments on commit 295526c

Please sign in to comment.