Managed - Log errors instead of throwing exceptions unless in debug mode

diff --git a/CRM/Core/ManagedEntities.php b/CRM/Core/ManagedEntities.php
index d923998692d93e31d5fd78812be0ad37d9c52352..f53431fe3f493a9432f54c19e13e157c4175d710 100644 (file)
--- a/CRM/Core/ManagedEntities.php
+++ b/CRM/Core/ManagedEntities.php
@@ -89,7 +89,7 @@ class CRM_Core_ManagedEntities {
         $result = civicrm_api3($dao->entity_type, 'getsingle', $params);
       }
       catch (Exception $e) {
-        $this->onApiError($dao->entity_type, 'getsingle', $params, $result);
+        $this->onApiError($dao->name, 'getsingle', $result['error_message']);
       }
       return $result;
     }
@@ -182,14 +182,21 @@ class CRM_Core_ManagedEntities {
       // Use "save" instead of "create" action to accommodate a "match" param
       $params['records'] = [$params['values']];
       unset($params['values']);
-      $result = civicrm_api4($item['entity_type'], 'save', $params);
+      try {
+        $result = civicrm_api4($item['entity_type'], 'save', $params);
+      }
+      catch (CRM_Core_Exception $e) {
+        $this->onApiError($item['name'], 'save', $e->getMessage());
+        return;
+      }
       $id = $result->first()['id'];
     }
     // APIv3
     else {
       $result = civicrm_api($item['entity_type'], 'create', $params);
       if (!empty($result['is_error'])) {
-        $this->onApiError($item['entity_type'], 'create', $params, $result);
+        $this->onApiError($item['name'], 'create', $result['error_message']);
+        return;
       }
       $id = $result['id'];
     }
@@ -247,7 +254,8 @@ class CRM_Core_ManagedEntities {
 
       $result = civicrm_api($item['entity_type'], 'create', $params);
       if ($result['is_error']) {
-        $this->onApiError($item['entity_type'], 'create', $params, $result);
+        $this->onApiError($item['name'], 'create', $result['error_message']);
+        return;
       }
     }
     elseif ($doUpdate && $item['params']['version'] == 4) {
@@ -255,7 +263,13 @@ class CRM_Core_ManagedEntities {
       $params['values']['id'] = $item['entity_id'];
       // 'match' param doesn't apply to "update" action
       unset($params['match']);
-      civicrm_api4($item['entity_type'], 'update', $params);
+      try {
+        civicrm_api4($item['entity_type'], 'update', $params);
+      }
+      catch (CRM_Core_Exception $e) {
+        $this->onApiError($item['name'], 'update', $e->getMessage());
+        return;
+      }
     }
 
     if (isset($item['cleanup']) || $doUpdate) {
@@ -287,7 +301,8 @@ class CRM_Core_ManagedEntities {
       ];
       $result = civicrm_api($item['entity_type'], 'create', $params);
       if ($result['is_error']) {
-        $this->onApiError($item['entity_type'], 'create', $params, $result);
+        $this->onApiError($item['name'], 'create', $result['error_message']);
+        return;
       }
       // Reset the `entity_modified_date` timestamp to indicate that the entity has not been modified by the user.
       $dao = new CRM_Core_DAO_Managed();
@@ -339,30 +354,25 @@ class CRM_Core_ManagedEntities {
 
     // Delete the entity and the managed record
     if ($doDelete) {
-      // APIv4 delete
-      if (CRM_Core_BAO_Managed::isApi4ManagedType($item['entity_type'])) {
-        civicrm_api4($item['entity_type'], 'delete', [
-          'checkPermissions' => FALSE,
-          'where' => [['id', '=', $item['entity_id']]],
-        ]);
-      }
-      // APIv3 delete
-      else {
-        $params = [
-          'version' => 3,
-          'id' => $item['entity_id'],
-        ];
-        $check = civicrm_api3($item['entity_type'], 'get', $params);
-        if ($check['count']) {
-          $result = civicrm_api($item['entity_type'], 'delete', $params);
-          if ($result['is_error']) {
-            if (isset($item['name'])) {
-              $params['name'] = $item['name'];
-            }
-            $this->onApiError($item['entity_type'], 'delete', $params, $result);
+      try {
+        // APIv4 delete
+        if (CRM_Core_BAO_Managed::isApi4ManagedType($item['entity_type'])) {
+          civicrm_api4($item['entity_type'], 'delete', [
+            'checkPermissions' => FALSE,
+            'where' => [['id', '=', $item['entity_id']]],
+          ]);
+        }
+        // APIv3 delete
+        else {
+          $check = civicrm_api3($item['entity_type'], 'get', ['id' => $item['entity_id']]);
+          if ($check['count']) {
+            civicrm_api3($item['entity_type'], 'delete', ['id' => $item['entity_id']]);
           }
         }
       }
+      catch (CRM_Core_Exception $e) {
+        $this->onApiError($item['name'], 'delete', $e->getMessage());
+      }
       // Ensure managed record is deleted.
       // Note: in many cases CRM_Core_BAO_Managed::on_hook_civicrm_post() will take care of
       // deleting it, but there may be edge cases, such as the source record no longer existing,
@@ -465,23 +475,23 @@ class CRM_Core_ManagedEntities {
   }
 
   /**
-   * @param string $entity
-   * @param string $action
-   * @param array $params
-   * @param array $result
+   * @param string $managedEntityName
+   * @param string $actionName
+   * @param string $errorMessage
    *
-   * @throws Exception
+   * @throws CRM_Core_Exception
    */
-  protected function onApiError($entity, $action, $params, $result) {
-    CRM_Core_Error::debug_var('ManagedEntities_failed', [
-      'entity' => $entity,
-      'action' => $action,
-      'params' => $params,
-      'result' => $result,
-    ]);
-    throw new Exception('API error: ' . $result['error_message'] . ' on ' . $entity . '.' . $action
-      . (!empty($params['name']) ? '( entity name ' . $params['name'] . ')' : '')
-    );
+  protected function onApiError(string $managedEntityName, string $actionName, string $errorMessage): void {
+    $message = "Unable to $actionName managed entity '$managedEntityName'. $errorMessage";
+    // During upgrade  this problem might be due to an about-to-be-installed extension
+    // So only log the error if it persists outside of upgrade mode
+    if (!CRM_Core_Config::isUpgradeMode()) {
+      Civi::log()->error($message);
+    }
+    // Errors should be very loud when debugging
+    if (Civi::settings()->get('debug_enabled')) {
+      throw new CRM_Core_Exception($message);
+    }
   }
 
   /**

diff --git a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php
index 87088dd8ae5cc1a2e375cd81867f7a28a5866c79..686fcf8bde1d40538379ac4e8a4a7a2ad35297ad 100644 (file)
--- a/tests/phpunit/api/v4/Entity/ManagedEntityTest.php
+++ b/tests/phpunit/api/v4/Entity/ManagedEntityTest.php
@@ -49,9 +49,16 @@ class ManagedEntityTest extends TestCase implements HeadlessInterface, Transacti
 
   public function setUp(): void {
     $this->_managedEntities = [];
+    // Ensure exceptions get thrown
+    \Civi::settings()->set('debug_enabled', TRUE);
     parent::setUp();
   }
 
+  public function tearDown(): void {
+    \Civi::settings()->revert('debug_enabled');
+    parent::tearDown();
+  }
+
   public function setUpHeadless(): CiviEnvBuilder {
     return Test::headless()->apply();
   }