Some api calls no longer give an error when e.g. a database error happens

I've marked it regression but it's not a usual type of regression. It has more to do with error handling and that some types of calls won't themselves generate errors in unit tests anymore. Or for example instead of an error on the screen, a dropdown list would just be empty.

It turns out there's two things going on here. The complicated one first:

In https://github.com/civicrm/civicrm-core/pull/19323 the way that pear, in particular pear::DB, generates errors was changed to use exceptions instead of PEAR_Error, which overall is a good thing.

However the API catches PEAR_Exceptions (not Errors) in Civi\API\Kernel:

    catch (\Exception $e) {
      if ($apiRequest) {
        $this->dispatcher->dispatch('civi.api.exception', new ExceptionEvent($e, NULL, $apiRequest, $this));
      }

      if ($e instanceof \PEAR_Exception) {
        $err = $this->formatPearException($e, $apiRequest);

Sometimes this is ok and you won't notice any difference in what happens later, but it depends on how the caller handles $result['is_error'].

For example, calls to getoptions or getfields go through metadata something

  $options = civicrm_api($apiRequest['entity'], 'getoptions', ['version' => 3, 'field' => $fieldname, 'context' => $context]);
  if (isset($options['values']) && is_array($options['values'])) {
    $metadata[$fieldname]['options'] = $options['values'];
  }

If a db error occurs during that api call, it used to bubble up and you'd see it, especially during unit tests. Now it still logs it (at the above formatPearException in Kernel), but then silently continues on.

So at first glance you might say that metadata-something should just check for is_error. That does "fix" that particular spot but now let's look at the second part. I haven't looked at every type of api call, but for example a civicrm_api3(contact.get) isn't affected because it will end up hitting api.php and so check is_error and throw an exception as expected:

if (is_array($result) && !empty($result['is_error'])) {
  throw new CiviCRM_API3_Exception($result['error_message'], CRM_Utils_Array::value('error_code', $result, 'undefined'), $result);
}

But note that there is a difference between calling civicrm_api with param version=3, as what happens in the metadata-something, and calling civicrm_api3. If I call contact.get using just civicrm_api, it does NOT check the is_error result and you get the silence. I don't know if this is on purpose because of differences with api4, but if so, then it could check $params['version'] and if it's 3 then check is_error. It looks a bit messy, but it maybe depends what the original reason is for not checking is_error.

diff --git a/api/api.php b/api/api.php
index be1946db37..66d5ccc82d 100644
--- a/api/api.php
+++ b/api/api.php
@@ -19,6 +19,13 @@
  * @return array|int
  */
 function civicrm_api(string $entity, string $action, array $params) {
+  if ($params['version'] == 3) {
+    $result = \Civi::service('civi_api_kernel')->runSafe($entity, $action, $params);
+    if (is_array($result) && !empty($result['is_error'])) {
+      throw new CiviCRM_API3_Exception($result['error_message'], CRM_Utils_Array::value('error_code', $result, 'undefined'), $result);
+    }
+    return $result;
+  }
   return \Civi::service('civi_api_kernel')->runSafe($entity, $action, $params);
 }