diff --git a/application/utils.inc.php b/application/utils.inc.php index 5d6754b8e..ec5b43ab5 100644 --- a/application/utils.inc.php +++ b/application/utils.inc.php @@ -2315,4 +2315,26 @@ class utils { return str_replace(' ', '', ucwords(strtr($sInput, '_-', ' '))); } + + /** + * @param \cmdbAbstractObject $oCmdbAbstract + * @param \Exception $oException + * + * @throws \Exception + * @since 2.7.2/ 2.8.0 + */ + public static function EnrichRaisedException($oCmdbAbstract, $oException) + { + if (is_null($oCmdbAbstract) || + ! is_a($oCmdbAbstract, \cmdbAbstractObject::class)) + { + throw $oException; + } + + $sCmdbAbstractInfo = str_replace("\n", '', "" . $oCmdbAbstract); + $sMessage = $oException->getMessage() . " (" . $sCmdbAbstractInfo . ")"; + + $e = new CoreException($sMessage, null, '', $oException); + throw $e; + } } diff --git a/core/coreexception.class.inc.php b/core/coreexception.class.inc.php index 81ba9efef..7188446c7 100644 --- a/core/coreexception.class.inc.php +++ b/core/coreexception.class.inc.php @@ -28,7 +28,7 @@ class CoreException extends Exception { - public function __construct($sIssue, $aContextData = null, $sImpact = '') + public function __construct($sIssue, $aContextData = null, $sImpact = '', $oPrevious = null) { $this->m_sIssue = $sIssue; $this->m_sImpact = $sImpact; @@ -66,7 +66,7 @@ class CoreException extends Exception } $sMessage .= implode(', ', $aContextItems); } - parent::__construct($sMessage, 0); + parent::__construct($sMessage, 0, $oPrevious); } /** @@ -81,6 +81,16 @@ class CoreException extends Exception return $this->getMessage(); } + /** + * getTraceAsString() cannot be overrided and it is limited as only current exception stack is returned. + * we need stack of all previous exceptions + * @uses __tostring() already does the work. + * @since 2.7.2/ 2.8.0 + */ + public function getFullStackTraceAsString(){ + return "" . $this; + } + public function getTraceAsHtml() { $aBackTrace = $this->getTrace(); diff --git a/core/dbobject.class.php b/core/dbobject.class.php index fae884d1e..aba3b5a25 100644 --- a/core/dbobject.class.php +++ b/core/dbobject.class.php @@ -2805,7 +2805,14 @@ abstract class DBObject implements iDisplay while ($oTrigger = $oSet->Fetch()) { /** @var \Trigger $oTrigger */ - $oTrigger->DoActivate($this->ToArgs('this')); + try + { + $oTrigger->DoActivate($this->ToArgs('this')); + } + catch(Exception $e) + { + utils::EnrichRaisedException($oTrigger, $e); + } } $this->RecordObjCreation(); @@ -3117,7 +3124,14 @@ abstract class DBObject implements iDisplay while ($oTrigger = $oSet->Fetch()) { /** @var \Trigger $oTrigger */ - $oTrigger->DoActivate($this->ToArgs('this')); + try + { + $oTrigger->DoActivate($this->ToArgs('this')); + } + catch(Exception $e) + { + utils::EnrichRaisedException($oTrigger, $e); + } } $bHasANewExternalKeyValue = false; @@ -3433,7 +3447,14 @@ abstract class DBObject implements iDisplay while ($oTrigger = $oSet->Fetch()) { /** @var \Trigger $oTrigger */ - $oTrigger->DoActivate($this->ToArgs('this')); + try + { + $oTrigger->DoActivate($this->ToArgs('this')); + } + catch(Exception $e) + { + utils::EnrichRaisedException($oTrigger, $e); + } } $this->RecordObjDeletion($this->m_iKey); // May cause a reload for storing history information @@ -3837,14 +3858,27 @@ abstract class DBObject implements iDisplay while ($oTrigger = $oSet->Fetch()) { /** @var \Trigger $oTrigger */ - $oTrigger->DoActivate($this->ToArgs('this')); + try + { + $oTrigger->DoActivate($this->ToArgs('this')); + } + catch(Exception $e) + { + utils::EnrichRaisedException($oTrigger, $e); + } } $oSet = new DBObjectSet(DBObjectSearch::FromOQL("SELECT TriggerOnStateEnter AS t WHERE t.target_class IN (:class_list) AND t.state=:new_state"), array(), $aParams); while ($oTrigger = $oSet->Fetch()) { /** @var \Trigger $oTrigger */ - $oTrigger->DoActivate($this->ToArgs('this')); + try{ + $oTrigger->DoActivate($this->ToArgs('this')); + } + catch(Exception $e) + { + utils::EnrichRaisedException($oTrigger, $e); + } } } else diff --git a/core/ormstopwatch.class.inc.php b/core/ormstopwatch.class.inc.php index 3af604e2c..1913a36c4 100644 --- a/core/ormstopwatch.class.inc.php +++ b/core/ormstopwatch.class.inc.php @@ -614,7 +614,14 @@ class CheckStopWatchThresholds implements iBackgroundProcess ); while ($oTrigger = $oTriggerSet->Fetch()) { - $oTrigger->DoActivate($oObj->ToArgs('this')); + try + { + $oTrigger->DoActivate($oObj->ToArgs('this')); + } + catch(Exception $e) + { + utils::EnrichRaisedException($oTrigger, $e); + } } } } diff --git a/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php b/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php index 835099838..11c9f3995 100644 --- a/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php +++ b/datamodels/2.x/itop-portal-base/portal/src/Form/ObjectFormManager.php @@ -1139,7 +1139,14 @@ class ObjectFormManager extends FormManager /** @var \Trigger $oTrigger */ while ($oTrigger = $oTriggerSet->Fetch()) { - $oTrigger->DoActivate($this->oObject->ToArgs('this')); + try + { + $oTrigger->DoActivate($this->oObject->ToArgs('this')); + } + catch(Exception $e) + { + utils::EnrichRaisedException($oTrigger, $e); + } } } } diff --git a/datamodels/2.x/itop-tickets/module.itop-tickets.php b/datamodels/2.x/itop-tickets/module.itop-tickets.php index 4be3a3d22..0830d1917 100755 --- a/datamodels/2.x/itop-tickets/module.itop-tickets.php +++ b/datamodels/2.x/itop-tickets/module.itop-tickets.php @@ -55,9 +55,16 @@ class TicketsInstaller extends ModuleInstallerAPI $oSet = new DBObjectSet($oSearch); while($oTrigger = $oSet->Fetch()) { - if (!MetaModel::IsValidClass($oTrigger->Get('target_class'))) + try { - $oTrigger->DBDelete(); + if (!MetaModel::IsValidClass($oTrigger->Get('target_class'))) + { + $oTrigger->DBDelete(); + } + } + catch(Exception $e) + { + utils::EnrichRaisedException($oTrigger, $e); } } } diff --git a/pages/UI.php b/pages/UI.php index 3c908983c..f931e9121 100644 --- a/pages/UI.php +++ b/pages/UI.php @@ -2004,7 +2004,7 @@ catch(CoreException $e) $oLog->Set('userinfo', ''); $oLog->Set('issue', $e->GetIssue()); $oLog->Set('impact', 'Page could not be displayed'); - $oLog->Set('callstack', $e->getTrace()); + $oLog->Set('callstack', $e->getFullStackTraceAsString()); $oLog->Set('data', $e->getContextData()); $oLog->DBInsertNoReload(); } @@ -2014,7 +2014,7 @@ catch(CoreException $e) } } - IssueLog::Error('UI.php operation='.$operation.', error='.$e->getMessage()."\n".$e->getTraceAsString()); + IssueLog::Error('UI.php operation='.$operation.', error='.$e->getMessage()."\n".$e->getFullStackTraceAsString()); } // For debugging only diff --git a/test/core/TriggerTest.php b/test/core/TriggerTest.php index e9c703cec..b2547b0e9 100644 --- a/test/core/TriggerTest.php +++ b/test/core/TriggerTest.php @@ -5,6 +5,7 @@ namespace Combodo\iTop\Test\UnitTest\Core; use Combodo\iTop\Test\UnitTest\ItopDataTestCase; use ContextTag; use MetaModel; +use PHPUnit\Exception; use TriggerOnObjectCreate; /** @@ -14,10 +15,21 @@ use TriggerOnObjectCreate; * * @runTestsInSeparateProcesses */ + +//define('APPROOT', dirname(__FILE__).'/../../'); +//define('APPCONF', APPROOT.'conf/'); + class TriggerTest extends ItopDataTestCase { const USE_TRANSACTION = false; + + protected function setUp() + { + //@include_once APPROOT . 'approot.inc.php'; + parent::setUp(); + } + public function testIsContextValid() { /** @var TriggerOnObjectCreate $oTrigger */ @@ -29,4 +41,65 @@ class TriggerTest extends ItopDataTestCase ContextTag::AddContext(ContextTag::TAG_CRON); $this->assertTrue($oTrigger->IsContextValid()); } + + public function testEnrichRaisedException_Trigger() + { + $oTrigger = MetaModel::NewObject('TriggerOnObjectCreate'); + $sStackTrace = ""; + try + { + try + { + MetaModel::NewObject('Toto'); + } + catch (\Exception $e) + { + $sStackTrace = $e->getTraceAsString(); + \utils::EnrichRaisedException($oTrigger, $e); + } + $this->assertTrue(false, "An exception should have been thrown"); + } + catch(\CoreException $e1) + { + $this->assertEquals('CoreException', get_class($e1)); + $this->assertEquals('Unknown class \'Toto\' (TriggerOnObjectCreate::-1 ()
)', $e1->getMessage()); + + $fullStackTraceAsString = $e1->getFullStackTraceAsString(); + $this->assertContains("MetaModel::NewObject", $fullStackTraceAsString,"new enriched exception should contain root cause method: " . $fullStackTraceAsString); + } + } + + public function NoEnrichmentProvider() + { + return [ + [ null ], + [ new \PHPUnit\Runner\Exception() ], + ] ; + } + + /** + * @param $oCmdbAbstract + * @dataProvider NoEnrichmentProvider + */ + public function testEnrichRaisedException_NoEnrichment($oCmdbAbstract) + { + $sStackTrace = ""; + try + { + try + { + MetaModel::NewObject('CoreException'); + } + catch (\Exception $e) + { + $sStackTrace = $e->getTraceAsString(); + \utils::EnrichRaisedException($oCmdbAbstract, $e); + } + $this->assertTrue(false, "An exception should have been thrown"); + } + catch(\Exception $e1) + { + $this->assertEquals($e, $e1); + } + } }