Page MenuHomeIn-Portal Phabricator
Differential D3 Diff 307 core/kernel/utility/logger.php

Changeset View

Standalone View

View Options

core/kernel/utility/logger.php

<?php <?php
/** /**
* @version $Id$ * @version $Id$
* @package In-Portal * @package In-Portal
* @copyright Copyright (C) 1997 - 2012 Intechnic. All rights reserved. * @copyright Copyright (C) 1997 - 2012 Intechnic. All rights reserved.
* @license GNU/GPL * @license GNU/GPL
alexUnsubmitted
Not Done
Reply

This doesn't cover following cases:

  1. non-catchable fatal error (e.g. calling method on a non-object variable, calling non-existing method on an object) described in http://qa.in-portal.org/D3?vs=19&id=27#188
  2. CLI usage

More info:

  1. when non-catchable fatal error happens then $this->Application will most likely reference a non-object (need to debug to be sure)
  2. in CLI (PHP_SAPI == 'cli') the debug mode is turned off (because there is no IP to match debug mode permission by) and it's a Front-End (e.g. /tools/run_event.php or /tools/cron.php)

So I actually recommend checking conditions for old error display way (via DIV) first and none of them matches, then do a redirect.

alex: This doesn't cover following cases: # non-catchable fatal error (e.g. calling method on a non…
* In-Portal is Open Source software. * In-Portal is Open Source software.
* This means that this software may have been modified pursuant * This means that this software may have been modified pursuant
* the GNU General Public License, and as distributed it includes * the GNU General Public License, and as distributed it includes
* or is derivative of works licensed under the GNU General Public License * or is derivative of works licensed under the GNU General Public License
* or other free or open source software licenses. * or other free or open source software licenses.
* See http://www.in-portal.org/license for copyright notices and details. * See http://www.in-portal.org/license for copyright notices and details.
*/ */
defined('FULL_PATH') or die('restricted access!'); defined('FULL_PATH') or die('restricted access!');
/** /**
* Class for logging system activity * Class for logging system activity
*/ */
class kLogger extends kBase { class kLogger extends kBase {
/** /**
* Prefix of all database related errors * Prefix of all database related errors
*/ */
const DB_ERROR_PREFIX = 'SQL Error:'; const DB_ERROR_PREFIX = 'SQL Error:';
/** /**
alexUnsubmitted
Not Done
Reply

Inline variable because now it won't create any extra long lines.

alex: Inline variable because now it won't create any extra long lines.
* Logger state: logging of errors and user-defined messages * Logger state: logging of errors and user-defined messages
*/ */
alexUnsubmitted
Not Done
Reply

The ternary operator must be bracketed because otherwise the 'margin: ' . $this->Application->isAdmin will become it's condition.

alex: The ternary operator must be bracketed because otherwise the `'margin: ' . $this->Application…
const STATE_ENABLED = 1; const STATE_ENABLED = 1;
/** /**
* Logger state: logging of user-defined messages only * Logger state: logging of user-defined messages only
*/ */
const STATE_USER_ONLY = 2; const STATE_USER_ONLY = 2;
/** /**
▲ Show 20 Lines • Show All 163 Lines • ▼ Show 20 Line(s)
* Ignores backtrace record where following files are mentioned * Ignores backtrace record where following files are mentioned
* *
* @var Array * @var Array
* @access protected * @access protected
*/ */
protected $_ignoreInTrace = Array ('logger.php', 'db_connection.php', 'db_load_balancer.php'); protected $_ignoreInTrace = Array ('logger.php', 'db_connection.php', 'db_load_balancer.php');
/** /**
* True when application is going to shutdown.
*
* @var boolean
*/
public $applicationShutdown = false;
/**
* Id of last log record.
*
* @var integer
*/
alexUnsubmitted
Not Done
Reply

Can't apply patch due conflict with current file version. Please sort of re-base.

alex: Can't apply patch due conflict with current file version. Please sort of re-base.
public $lastLogId = null;
/**
* Create event log * Create event log
* *
* @param Array $methods_to_call List of invokable kLogger class method with their parameters (if any) * @param Array $methods_to_call List of invokable kLogger class method with their parameters (if any)
* @access public * @access public
*/ */
public function __construct($methods_to_call = Array ()) public function __construct($methods_to_call = Array ())
{ {
parent::__construct(); parent::__construct();
▲ Show 20 Lines • Show All 515 Lines • ▼ Show 20 Line(s)
$this->_logRecord['LogDate'] = date('Y-m-d H:i:s'); $this->_logRecord['LogDate'] = date('Y-m-d H:i:s');
if ( $storage_medium == self::LS_AUTOMATIC ) { if ( $storage_medium == self::LS_AUTOMATIC ) {
$storage_medium = $this->Conn->connectionOpened() ? self::LS_DATABASE : self::LS_DISK; $storage_medium = $this->Conn->connectionOpened() ? self::LS_DATABASE : self::LS_DISK;
} }
if ( $storage_medium == self::LS_DATABASE ) { if ( $storage_medium == self::LS_DATABASE ) {
$result = $this->Conn->doInsert($this->_logRecord, TABLE_PREFIX . 'SystemLog'); $result = $this->Conn->doInsert($this->_logRecord, TABLE_PREFIX . 'SystemLog');
$this->lastLogId = $this->Conn->getInsertID();
} }
elseif ( $storage_medium == self::LS_DISK ) { elseif ( $storage_medium == self::LS_DISK ) {
$result = $this->_saveToFile(RESTRICTED . '/system.log'); $result = $this->_saveToFile(RESTRICTED . '/system.log');
} }
else { else {
throw new InvalidArgumentException('Unknown storage medium "' . $storage_medium . '"'); throw new InvalidArgumentException('Unknown storage medium "' . $storage_medium . '"');
} }
Show All 11 Lines
/** /**
* Catches last error happened before script ended * Catches last error happened before script ended
* *
* @return void * @return void
* @access public * @access public
*/ */
public function catchLastError() public function catchLastError()
{ {
$this->applicationShutdown = true;
$this->write(); $this->write();
$last_error = error_get_last(); $last_error = error_get_last();
if ( !is_null($last_error) && isset($this->_handlers[self::LL_ERROR]) ) { if ( !is_null($last_error) && isset($this->_handlers[self::LL_ERROR]) ) {
$handler = $this->_handlers[self::LL_ERROR]; $handler = $this->_handlers[self::LL_ERROR];
/* @var $handler kErrorHandlerStack */ /* @var $handler kErrorHandlerStack */
$handler->handle($last_error['type'], $last_error['message'], $last_error['file'], $last_error['line']); $handler->handle($last_error['type'], $last_error['message'], $last_error['file'], $last_error['line']);
▲ Show 20 Lines • Show All 352 Lines • ▼ Show 20 Line(s)
return null; return null;
} }
/** /**
* Determines if given error is a fatal * Determines if given error is a fatal
* *
* @abstract * @abstract
* @param Exception|int $errno * @param Exception|int $errno
alexUnsubmitted
Not Done
Reply

Can't we just call StoreVar method off the Application itself?

alex: Can't we just call `StoreVar` method off the `Application` itself?
* @return bool * @return bool
alexUnsubmitted
Not Done
Reply

The Redirect method you're calling below is already saving session content, so this line isn't needed.

alex: The `Redirect` method you're calling below is already saving session content, so this line…
*/ */
abstract protected function _isFatalError($errno); abstract protected function _isFatalError($errno);
alexUnsubmitted
Not Done
Reply

Here is what needs to be done:

  1. add applicationShutdown property to kLogger class, which default to false
  2. from kLogger::catchLastError method set kLogger::applicationShutdown to true
  3. replace the !$this->Application check with $this->applicationShutdown check

Here is how I was able to the kLogger::catchLastError method:

  1. disable debugger
  2. add $class = new MissingClass(); on top of kMainTagProcessor::Phrase method
  3. go to http://your-in-portal-install.com/directory.html
alex: Here is what needs to be done: # add `applicationShutdown` property to `kLogger` class, which…
alexUnsubmitted
Not Done
Reply

I think we can benefit from using error_fatal.tpl in Admin Console as well, when debug mode is turned off. To make this happen please replace

  • this: $this->Application->isDebugMode() || $this->Application->isAdmin
  • with this: $this->Application->isDebugMode(false)
alex: I think we can benefit from using `error_fatal.tpl` in Admin Console as well, when debug mode…
/** /**
* Displays div with given error message * Displays div with given error message
* *
* @param string $errno * @param string $errno
* @return void * @return void
* @access protected * @access protected
*/ */
alexUnsubmitted
Not Done
Reply

This code will be used when it's Front-End and Debug Mode is Turned Off. In all other cases please use original error reporting code.

alex: This code will be used when `it's Front-End` and `Debug Mode is Turned Off`. In all other cases…
alexUnsubmitted
Not Done
Reply

When we're in admin then use error_fatal as error template and don't read it from system setting to allow Front-End having different template without affecting Admin Console where template name is always the same.

alex: When we're in admin then use `error_fatal` as error template and don't read it from system…
protected function _displayFatalError($errno) protected function _displayFatalError($errno)
alexUnsubmitted
Not Done
Reply

Please remove the isDebugMode part (let's use nice template in debug mode too).

alex: Please remove the `isDebugMode` part (let's use nice template in debug mode too).
alexUnsubmitted
Not Done
Reply

Actually don't change that line, because we're only getting to _displayFatalError method in debug mode, when debug report is invisible and this happens during ajax request only in which case I want to see failed ajax request instead of redirect.

alex: Actually don't change that line, because we're only getting to `_displayFatalError` method in…
{ {
$errno = $this->_getFatalErrorTitle($errno); $errno = $this->_getFatalErrorTitle($errno);
$message = $this->_logger->toString($errno);
alexUnsubmitted
Not Done
Reply

This concatenation style doesn't look right. Please to do this instead:

  1. create $css_properties variable, which will be an array of all selectors (e.g. background-color: #FEFFBF, margin: ' . $margin) with no trailing ;
  2. replace $style variable with implode('; ', $css_properties) . '; construct
alex: This concatenation style doesn't look right. Please to do this instead: # create…
$margin = $this->Application->isAdmin ? '8px' : 'auto'; if ( PHP_SAPI == 'cli' || $this->_logger->applicationShutdown || $this->Application->isDebugMode(false) ) {
echo '<div style="background-color: #FEFFBF; margin: ' . $margin . '; padding: 10px; border: 2px solid red; text-align: center">' . $this->_logger->toString($errno) . '</div>'; $css_properties = array(
'background-color: #FEFFBF',
'margin: ' . ($this->Application->isAdmin ? '8px' : 'auto'),
'padding: 10px',
'border: 2px solid red',
'text-align: center',
);
echo '<div style="' . implode('; ', $css_properties) . ';">' . $message . '</div>';
alexUnsubmitted
Not Done
Reply

When in CLI:

  1. don't append HTML/CSS to the error message
  2. also show stack trace by using debug_print_backtrace function

Idea is to have same error display in CLI as PHP builds it, when it encounters unhandled exception:

PHP Fatal error:  Uncaught exception 'Exception' with message 'IntuitAnywhere login failed' in /path/to/project/modules/custom/units/helpers/quickbooks/QBConnectorHelper.php:91
Stack trace:
#0 /path/to/project/core/kernel/utility/factory.php(118): QBConnectorHelper->Init('QBConnectorHelp...', '', Array)
#1 /path/to/project/core/kernel/application.php(2492): kFactory->getObject('QBConnectorHelp...', NULL, Array)
#2 /path/to/project/modules/custom/units/helpers/quick_books_helper.php(296): kApplication->recallObject('QBConnectorHelp...')
#3 /path/to/project/modules/custom/units/organizations/organization_eh.php(1328): IntechnicQuickBooksHelper->synchronizeVendor(Array)
#4 /path/to/project/modules/custom/units/organizations/organization_eh.php(201): OrganizationEventHandler->synchronizeSuppliers(1447405965)
#5 /path/to/project/core/kernel/event_handler.php(116): OrganizationEventHandler->OnQuickBooksSynchronize(Object(kEvent))
#6 /mn in /path/to/project/modules/custom/units/helpers/quickbooks/QBConnectorHelper.php on line 91
alex: When in CLI: # don't append HTML/CSS to the error message # also show stack trace by using…
alexUnsubmitted
Not Done
Reply

Please don't do it, because we have separate discussion about this at http://community.in-portal.org/x/ZAD3.

alex: Please don't do it, because we have separate discussion about this at http://community.in…
exit; exit;
} }
alexUnsubmitted
Not Done
Reply

Item 5 of the plan in the Jira issue clearly says that new error display system only takes effect when Debug Mode is turned off. Looks like you entirely replaced old system and that's incorrect.

alex: Item 5 of the plan in the Jira issue clearly says that new error display system only takes…
else {
alexUnsubmitted
Not Done
Reply

Try rearranging code like so:

  1. move StoreVar statement on top of outer else
  2. do ->Redirect call in place (if/else) instead of using helper $error_template variable
alex: Try rearranging code like so: # move `StoreVar` statement on top of outer `else` # do `…
$this->Application->StoreVar('FatalErrorMessage', $message);
$redirect_params = array();
alexUnsubmitted
Not Done
Reply

Please also do this (for debug mode only):

  1. add the system-log_id parameter to redirect params, that would contain ID of just created log record
  2. on the error template use <inp2:system-log_PrintBacktrace/> tag to display expandable stack trace of the error
alex: Please also do this (for debug mode only): # add the `system-log_id` parameter to redirect…
if ( $this->Application->isDebugMode() && $this->_logger->lastLogId ) {
$redirect_params['system-log_id'] = $this->_logger->lastLogId;
}
if ( $this->Application->isAdmin ) {
$this->Application->Redirect('error_fatal', $redirect_params);
}
else {
$this->Application->Redirect($this->Application->ConfigValue('FatalErrorTemplate'), $redirect_params);
}
}
}
/** /**
* Returns title to show for a fatal * Returns title to show for a fatal
* *
* @abstract * @abstract
* @param Exception|int $errno * @param Exception|int $errno
* @return string * @return string
*/ */
▲ Show 20 Lines • Show All 186 Lines • Show Last 20 Lines
In-Portal CMS · In-Portal Community Pages · In-Portal Issue Tracker · In-Portal Repository Browser · Quality Assurance · In-Portal CI Server