From d634744020fc6e3080669d45c63c5b3be36f47dc Mon Sep 17 00:00:00 2001 From: Seamus Lee Date: Wed, 6 Jan 2021 08:22:37 +1100 Subject: [PATCH] REF Update CiviCRM default PEAR Error handling to be exception rather than just PEAR_Error object and remove useException overirdes as no longer needed and patch necessary places to handle with Exceptions coming back --- CRM/Activity/BAO/Activity.php | 3 --- CRM/Activity/BAO/ActivityContact.php | 1 - CRM/Admin/Form/Setting/Smtp.php | 1 - CRM/Case/Info.php | 16 +++++++++------ CRM/Contact/Selector.php | 2 -- CRM/Core/Config.php | 2 +- CRM/Queue/ErrorPolicy.php | 1 - CRM/Queue/Runner.php | 1 - CRM/Utils/File.php | 20 +++++++++++-------- CRM/Utils/Mail/FilteredPearMailer.php | 9 +++++++++ CRM/Utils/SQL/Delete.php | 1 - CRM/Utils/SQL/Select.php | 1 - CRM/Utils/System/Backdrop.php | 8 +++++--- CRM/Utils/System/Drupal.php | 8 +++++--- CRM/Utils/System/Drupal6.php | 8 +++++--- Civi/API/Kernel.php | 1 - Civi/Test/CiviTestListener.php | 5 ----- Civi/Test/CiviTestListenerPHPUnit7.php | 6 ------ Civi/Test/Legacy/CiviTestListener.php | 5 ----- sql/GenerateData.php | 1 - .../phpunit/CRM/Core/BAO/CustomFieldTest.php | 1 - .../Core/BAO/CustomValueTableSetGetTest.php | 2 -- tests/phpunit/CRM/Dedupe/DedupeFinderTest.php | 1 - tests/phpunit/CiviTest/CiviUnitTestCase.php | 2 -- 24 files changed, 47 insertions(+), 59 deletions(-) diff --git a/CRM/Activity/BAO/Activity.php b/CRM/Activity/BAO/Activity.php index 22c460d35c..aabb79f369 100644 --- a/CRM/Activity/BAO/Activity.php +++ b/CRM/Activity/BAO/Activity.php @@ -1398,9 +1398,6 @@ class CRM_Activity_BAO_Activity extends CRM_Activity_DAO_Activity { $providerObj = CRM_SMS_Provider::singleton(['provider_id' => $smsProviderParams['provider_id']]); $sendResult = $providerObj->send($recipient, $smsProviderParams, $tokenText, NULL, $sourceContactID); - if (PEAR::isError($sendResult)) { - throw new CRM_Core_Exception($sendResult->getMessage()); - } // add activity target record for every sms that is sent $targetID = CRM_Core_PseudoConstant::getKey('CRM_Activity_BAO_ActivityContact', 'record_type_id', 'Activity Targets'); diff --git a/CRM/Activity/BAO/ActivityContact.php b/CRM/Activity/BAO/ActivityContact.php index 6efa32ab9d..4ac6652f7c 100644 --- a/CRM/Activity/BAO/ActivityContact.php +++ b/CRM/Activity/BAO/ActivityContact.php @@ -37,7 +37,6 @@ class CRM_Activity_BAO_ActivityContact extends CRM_Activity_DAO_ActivityContact * activity_contact object */ public static function create($params) { - $errorScope = CRM_Core_TemporaryErrorScope::useException(); $activityContact = new CRM_Activity_DAO_ActivityContact(); $activityContact->copyValues($params); try { diff --git a/CRM/Admin/Form/Setting/Smtp.php b/CRM/Admin/Form/Setting/Smtp.php index 83ec40083a..eaefe4c2fb 100644 --- a/CRM/Admin/Form/Setting/Smtp.php +++ b/CRM/Admin/Form/Setting/Smtp.php @@ -172,7 +172,6 @@ class CRM_Admin_Form_Setting_Smtp extends CRM_Admin_Form_Setting { $mailer = CRM_Utils_Mail::_createMailer($mailerName, $params); try { - $errorScope = CRM_Core_TemporaryErrorScope::useException(); $mailer->send($toEmail, $headers, $message); if (defined('CIVICRM_MAIL_LOG') && defined('CIVICRM_MAIL_LOG_AND_SEND')) { $testMailStatusMsg .= '
' . ts('You have defined CIVICRM_MAIL_LOG_AND_SEND - mail will be logged.') . '

'; diff --git a/CRM/Case/Info.php b/CRM/Case/Info.php index ef3bcb41df..d3f6061028 100644 --- a/CRM/Case/Info.php +++ b/CRM/Case/Info.php @@ -275,9 +275,11 @@ class CRM_Case_Info extends CRM_Core_Component_Info { foreach ($queries as $query) { $query = trim($query); if (!empty($query)) { - $res = &$db->query($query); - if (PEAR::isError($res)) { - die("Cannot execute $query: " . $res->getMessage()); + try { + $res = &$db->query($query); + } + catch (Exception $e) { + die("Cannot execute $query: " . $e->getMessage()); } } } @@ -290,9 +292,11 @@ class CRM_Case_Info extends CRM_Core_Component_Info { $string = trim($string); if (!empty($string)) { - $res = &$db->query($string); - if (PEAR::isError($res)) { - die("Cannot execute $string: " . $res->getMessage()); + try { + $res = &$db->query($string); + } + catch (Exception $e) { + die("Cannot execute $string: " . $e->getMessage()); } } } diff --git a/CRM/Contact/Selector.php b/CRM/Contact/Selector.php index 72cf3039db..17a1155d0f 100644 --- a/CRM/Contact/Selector.php +++ b/CRM/Contact/Selector.php @@ -1016,8 +1016,6 @@ class CRM_Contact_Selector extends CRM_Core_Selector_Base implements CRM_Core_Se */ public function fillupPrevNextCache($sort, $cacheKey, $start = 0, $end = self::CACHE_SIZE) { $coreSearch = TRUE; - // This ensures exceptions are caught in the try-catch. - $handling = CRM_Core_TemporaryErrorScope::useException(); // For custom searches, use the contactIDs method if (is_a($this, 'CRM_Contact_Selector_Custom')) { $sql = $this->_search->contactIDs($start, $end, $sort, TRUE); diff --git a/CRM/Core/Config.php b/CRM/Core/Config.php index 62fa439755..7f4bdd0268 100644 --- a/CRM/Core/Config.php +++ b/CRM/Core/Config.php @@ -86,7 +86,7 @@ class CRM_Core_Config extends CRM_Core_Config_MagicMerge { */ public static function &singleton($loadFromDB = TRUE, $force = FALSE) { if (self::$_singleton === NULL || $force) { - $GLOBALS['civicrm_default_error_scope'] = CRM_Core_TemporaryErrorScope::create(['CRM_Core_Error', 'handle']); + $GLOBALS['civicrm_default_error_scope'] = CRM_Core_TemporaryErrorScope::create(['CRM_Core_Error', 'exceptionHandler'], 1); $errorScope = CRM_Core_TemporaryErrorScope::create(['CRM_Core_Error', 'simpleHandler']); if (defined('E_DEPRECATED')) { diff --git a/CRM/Queue/ErrorPolicy.php b/CRM/Queue/ErrorPolicy.php index f7bc774add..178591bbd2 100644 --- a/CRM/Queue/ErrorPolicy.php +++ b/CRM/Queue/ErrorPolicy.php @@ -56,7 +56,6 @@ class CRM_Queue_ErrorPolicy { } set_error_handler([$this, 'onError'], $this->level); // FIXME make this temporary/reversible - $this->errorScope = CRM_Core_TemporaryErrorScope::useException(); } /** diff --git a/CRM/Queue/Runner.php b/CRM/Queue/Runner.php index bac1f8e449..0652629c15 100644 --- a/CRM/Queue/Runner.php +++ b/CRM/Queue/Runner.php @@ -152,7 +152,6 @@ class CRM_Queue_Runner { // setting -- it should be more of a contextual/stack-based setting. // This should be appropriate because queue-runners are not used with // basic web pages -- they're used with CLI/REST/AJAX. - $errorScope = CRM_Core_TemporaryErrorScope::useException(); $taskResult = $this->runNext(); $errorScope = NULL; } diff --git a/CRM/Utils/File.php b/CRM/Utils/File.php index e4fb43b9cc..5c4ae3c548 100644 --- a/CRM/Utils/File.php +++ b/CRM/Utils/File.php @@ -326,12 +326,14 @@ class CRM_Utils_File { else { require_once 'DB.php'; $dsn = CRM_Utils_SQL::autoSwitchDSN($dsn); - $db = DB::connect($dsn); + try { + $db = DB::connect($dsn); + } + catch (Exception $e) { + die("Cannot open $dsn: " . $e->getMessage()); + } } - if (PEAR::isError($db)) { - die("Cannot open $dsn: " . $db->getMessage()); - } $db->query('SET NAMES utf8mb4'); $transactionId = CRM_Utils_Type::escape(CRM_Utils_Request::id(), 'String'); $db->query('SET @uniqueID = ' . "'$transactionId'"); @@ -345,13 +347,15 @@ class CRM_Utils_File { $query = trim($query); if (!empty($query)) { CRM_Core_Error::debug_query($query); - $res = &$db->query($query); - if (PEAR::isError($res)) { + try { + $res = &$db->query($query); + } + catch (Exception $e) { if ($dieOnErrors) { - die("Cannot execute $query: " . $res->getMessage()); + die("Cannot execute $query: " . $e->getMessage()); } else { - echo "Cannot execute $query: " . $res->getMessage() . "

"; + echo "Cannot execute $query: " . $e->getMessage() . "

"; } } } diff --git a/CRM/Utils/Mail/FilteredPearMailer.php b/CRM/Utils/Mail/FilteredPearMailer.php index 5ede19aa46..bf80e19a1f 100644 --- a/CRM/Utils/Mail/FilteredPearMailer.php +++ b/CRM/Utils/Mail/FilteredPearMailer.php @@ -56,6 +56,15 @@ class CRM_Utils_Mail_FilteredPearMailer extends Mail { $this->_delegate = $mailer; } + public function __destruct() { + try { + unset($this->_delegate); + } + catch (Exception $e) { + Civi::log()->error($e->getMessage()); + } + } + public function send($recipients, $headers, $body) { $filterArgs = [$this, &$recipients, &$headers, &$body]; foreach ($this->_filters as $filter) { diff --git a/CRM/Utils/SQL/Delete.php b/CRM/Utils/SQL/Delete.php index fcb5443030..b31a7162eb 100644 --- a/CRM/Utils/SQL/Delete.php +++ b/CRM/Utils/SQL/Delete.php @@ -241,7 +241,6 @@ class CRM_Utils_SQL_Delete extends CRM_Utils_SQL_BaseParamQuery { // Don't pass through $abort, $trapException. Just use straight-up exceptions. $abort = TRUE; $trapException = FALSE; - $errorScope = CRM_Core_TemporaryErrorScope::useException(); // Don't pass through freeDAO. You can do it yourself. $freeDAO = FALSE; diff --git a/CRM/Utils/SQL/Select.php b/CRM/Utils/SQL/Select.php index 642d98ff2b..fa21fbde6f 100644 --- a/CRM/Utils/SQL/Select.php +++ b/CRM/Utils/SQL/Select.php @@ -557,7 +557,6 @@ class CRM_Utils_SQL_Select extends CRM_Utils_SQL_BaseParamQuery { // Don't pass through $abort, $trapException. Just use straight-up exceptions. $abort = TRUE; $trapException = FALSE; - $errorScope = CRM_Core_TemporaryErrorScope::useException(); // Don't pass through freeDAO. You can do it yourself. $freeDAO = FALSE; diff --git a/CRM/Utils/System/Backdrop.php b/CRM/Utils/System/Backdrop.php index ded63e2164..1284b59563 100644 --- a/CRM/Utils/System/Backdrop.php +++ b/CRM/Utils/System/Backdrop.php @@ -292,9 +292,11 @@ class CRM_Utils_System_Backdrop extends CRM_Utils_System_DrupalBase { $config = CRM_Core_Config::singleton(); $ufDSN = CRM_Utils_SQL::autoSwitchDSN($config->userFrameworkDSN); - $dbBackdrop = DB::connect($ufDSN); - if (DB::isError($dbBackdrop)) { - throw new CRM_Core_Exception("Cannot connect to Backdrop database via $ufDSN, " . $dbBackdrop->getMessage()); + try { + $dbBackdrop = DB::connect($ufDSN); + } + catch (Exception $e) { + throw new CRM_Core_Exception("Cannot connect to Backdrop database via $ufDSN, " . $e->getMessage()); } $account = $userUid = $userMail = NULL; diff --git a/CRM/Utils/System/Drupal.php b/CRM/Utils/System/Drupal.php index 710e178355..3d7efe9ad2 100644 --- a/CRM/Utils/System/Drupal.php +++ b/CRM/Utils/System/Drupal.php @@ -318,9 +318,11 @@ class CRM_Utils_System_Drupal extends CRM_Utils_System_DrupalBase { $config = CRM_Core_Config::singleton(); $ufDSN = CRM_Utils_SQL::autoSwitchDSN($config->userFrameworkDSN); - $dbDrupal = DB::connect($ufDSN); - if (DB::isError($dbDrupal)) { - throw new CRM_Core_Exception("Cannot connect to drupal db via $ufDSN, " . $dbDrupal->getMessage()); + try { + $dbDrupal = DB::connect($ufDSN); + } + catch (Exception $e) { + throw new CRM_Core_Exception("Cannot connect to drupal db via $ufDSN, " . $e->getMessage()); } $account = $userUid = $userMail = NULL; diff --git a/CRM/Utils/System/Drupal6.php b/CRM/Utils/System/Drupal6.php index 97249fd9bd..5db5f33d3c 100644 --- a/CRM/Utils/System/Drupal6.php +++ b/CRM/Utils/System/Drupal6.php @@ -301,9 +301,11 @@ class CRM_Utils_System_Drupal6 extends CRM_Utils_System_DrupalBase { $config = CRM_Core_Config::singleton(); $ufDSN = CRM_Utils_SQL::autoSwitchDSN($config->userFrameworkDSN); - $dbDrupal = DB::connect($ufDSN); - if (DB::isError($dbDrupal)) { - throw new CRM_Core_Exception("Cannot connect to drupal db via $ufDSN, " . $dbDrupal->getMessage()); + try { + $dbDrupal = DB::connect($ufDSN); + } + catch (Exception $e) { + throw new CRM_Core_Exception("Cannot connect to drupal db via $ufDSN, " . $e->getMessage()); } $strtolower = function_exists('mb_strtolower') ? 'mb_strtolower' : 'strtolower'; diff --git a/Civi/API/Kernel.php b/Civi/API/Kernel.php index feb8b662e4..91544231ae 100644 --- a/Civi/API/Kernel.php +++ b/Civi/API/Kernel.php @@ -142,7 +142,6 @@ class Kernel { */ public function runRequest($apiRequest) { $this->boot($apiRequest); - $errorScope = \CRM_Core_TemporaryErrorScope::useException(); list($apiProvider, $apiRequest) = $this->resolve($apiRequest); $this->authorize($apiProvider, $apiRequest); diff --git a/Civi/Test/CiviTestListener.php b/Civi/Test/CiviTestListener.php index 2851bda3d8..9c83015057 100644 --- a/Civi/Test/CiviTestListener.php +++ b/Civi/Test/CiviTestListener.php @@ -25,10 +25,6 @@ else { * @see HookInterface */ class CiviTestListener extends \PHPUnit\Framework\BaseTestListener { - /** - * @var \CRM_Core_TemporaryErrorScope - */ - private $errorScope; /** * @var array @@ -55,7 +51,6 @@ else { public function startTest(\PHPUnit\Framework\Test $test) { if ($this->isCiviTest($test)) { error_reporting(E_ALL); - $this->errorScope = \CRM_Core_TemporaryErrorScope::useException(); } if ($test instanceof HeadlessInterface) { diff --git a/Civi/Test/CiviTestListenerPHPUnit7.php b/Civi/Test/CiviTestListenerPHPUnit7.php index ee8173d2b8..70238826e1 100644 --- a/Civi/Test/CiviTestListenerPHPUnit7.php +++ b/Civi/Test/CiviTestListenerPHPUnit7.php @@ -18,11 +18,6 @@ class CiviTestListenerPHPUnit7 implements \PHPUnit\Framework\TestListener { use \PHPUnit\Framework\TestListenerDefaultImplementation; - /** - * @var \CRM_Core_TemporaryErrorScope - */ - private $errorScope; - /** * @var array * Ex: $cache['Some_Test_Class']['civicrm_foobar'] = 'hook_civicrm_foobar'; @@ -48,7 +43,6 @@ class CiviTestListenerPHPUnit7 implements \PHPUnit\Framework\TestListener { public function startTest(\PHPUnit\Framework\Test $test): void { if ($this->isCiviTest($test)) { error_reporting(E_ALL); - $this->errorScope = \CRM_Core_TemporaryErrorScope::useException(); } if ($test instanceof HeadlessInterface) { diff --git a/Civi/Test/Legacy/CiviTestListener.php b/Civi/Test/Legacy/CiviTestListener.php index eb85e8906e..f1e9cad2e0 100644 --- a/Civi/Test/Legacy/CiviTestListener.php +++ b/Civi/Test/Legacy/CiviTestListener.php @@ -15,10 +15,6 @@ namespace Civi\Test\Legacy; * @see HookInterface */ class CiviTestListener extends \PHPUnit_Framework_BaseTestListener { - /** - * @var \CRM_Core_TemporaryErrorScope - */ - private $errorScope; /** * @var array @@ -45,7 +41,6 @@ class CiviTestListener extends \PHPUnit_Framework_BaseTestListener { public function startTest(\PHPUnit_Framework_Test $test) { if ($this->isCiviTest($test)) { error_reporting(E_ALL); - $this->errorScope = \CRM_Core_TemporaryErrorScope::useException(); } if ($test instanceof \Civi\Test\HeadlessInterface) { diff --git a/sql/GenerateData.php b/sql/GenerateData.php index 6928bf57af..63c2b30b23 100644 --- a/sql/GenerateData.php +++ b/sql/GenerateData.php @@ -75,7 +75,6 @@ CRM_Core_Config::singleton(); echo ("Starting data generation on " . date("F dS h:i:s A") . "\n"); try { - $scope = CRM_Core_TemporaryErrorScope::useException(); // Generate reproducible data-set // $gcd = new CRM_Core_CodeGen_GenerateData('1234', strtotime(date('Y') . '-01-01 02:03:04')); // Generate unique data-set diff --git a/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php b/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php index 8ea4dad0cd..f31fac6213 100644 --- a/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php +++ b/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php @@ -345,7 +345,6 @@ class CRM_Core_BAO_CustomFieldTest extends CiviUnitTestCase { CRM_Core_BAO_CustomField::moveField($fields['countryB']['id'], $groupB['id']); // Group[A] no longer has fields[countryB] - $errorScope = CRM_Core_TemporaryErrorScope::useException(); try { $this->assertDBQuery(1, "SELECT {$fields['countryB']['column_name']} FROM " . $groupA['table_name']); $this->fail('Expected exception when querying column on wrong table'); diff --git a/tests/phpunit/CRM/Core/BAO/CustomValueTableSetGetTest.php b/tests/phpunit/CRM/Core/BAO/CustomValueTableSetGetTest.php index 1e9d7fa84b..8d2b52519d 100644 --- a/tests/phpunit/CRM/Core/BAO/CustomValueTableSetGetTest.php +++ b/tests/phpunit/CRM/Core/BAO/CustomValueTableSetGetTest.php @@ -66,7 +66,6 @@ class CRM_Core_BAO_CustomValueTableSetGetTest extends CiviUnitTestCase { 'custom_' . $fieldID => $badDate, ]; - CRM_Core_TemporaryErrorScope::useException(); $message = NULL; try { CRM_Core_BAO_CustomValueTable::setValues($params); @@ -165,7 +164,6 @@ class CRM_Core_BAO_CustomValueTableSetGetTest extends CiviUnitTestCase { 'custom_' . $fieldID => $badYesNo, ]; - CRM_Core_TemporaryErrorScope::useException(); $message = NULL; try { CRM_Core_BAO_CustomValueTable::setValues($params); diff --git a/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php b/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php index df954e592c..82c6e9c198 100644 --- a/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php +++ b/tests/phpunit/CRM/Dedupe/DedupeFinderTest.php @@ -321,7 +321,6 @@ class CRM_Dedupe_DedupeFinderTest extends CiviUnitTestCase { 'email' => 'hood@example.com', 'street_address' => 'Ambachtstraat 23', ]; - CRM_Core_TemporaryErrorScope::useException(); $ids = CRM_Contact_BAO_Contact::getDuplicateContacts($fields, 'Individual', 'General', [], TRUE, NULL, ['event_id' => 1]); // Check with default Individual-General rule diff --git a/tests/phpunit/CiviTest/CiviUnitTestCase.php b/tests/phpunit/CiviTest/CiviUnitTestCase.php index 54fb3746d9..40e5577ee9 100644 --- a/tests/phpunit/CiviTest/CiviUnitTestCase.php +++ b/tests/phpunit/CiviTest/CiviUnitTestCase.php @@ -323,8 +323,6 @@ class CiviUnitTestCase extends PHPUnit\Framework\TestCase { $this->_apiversion = 3; - // REVERT - $this->errorScope = CRM_Core_TemporaryErrorScope::useException(); // Use a temporary file for STDIN $GLOBALS['stdin'] = tmpfile(); if ($GLOBALS['stdin'] === FALSE) { -- 2.25.1