Exceptions - What could possibly go wrong?

diff --git a/CRM/Core/Exception.php b/CRM/Core/Exception.php
index b099c0a07ab3137aa19bd226ea5231f568b2a80b..5591885acc773445eac29a28ca026b731138bc50 100644 (file)
--- a/CRM/Core/Exception.php
+++ b/CRM/Core/Exception.php
@@ -13,27 +13,42 @@
  * Base class for exceptions generated by CiviCRM.
  * This Exception returns more information than the default one. We are using it from the
  * form layer to allow redirects to occur without having redirects in the BAO
- * @param string $message
- *   The human friendly error message.
- * @param string $error_code
- *   A computer friendly error code. By convention, no space (but underscore allowed).
- *   ex: mandatory_missing, duplicate, invalid_format
  * @param array $data
  *   Extra params to return. eg an extra array of ids. It is not mandatory, but can help the computer using the api. Keep in mind the api consumer isn't to be trusted. eg. the database password is NOT a good extra data.
  */
 class CRM_Core_Exception extends PEAR_Exception {
+
+  const UNAUTHORIZED = 'unauthorized';
+  const NOT_IMPLEMENTED = 'not-found';
+
   private $errorData = [];
 
   /**
    * Class constructor.
    *
    * @param string $message
-   * @param int $error_code
+   *   The human friendly error message.
+   * @param int|string $error_code
+   *   A computer friendly error code. By convention, no space (but underscore
+   *   allowed) (ex: mandatory_missing, duplicate, invalid_format).
    * @param array $errorData
-   * @param null $previous
+   *   Extra params to return. eg an extra array of ids. It is not mandatory,
+   *   but can help the computer using the api. Keep in mind the api consumer
+   *   isn't to be trusted. eg. the database password is NOT a good extra data.
+   * @param Throwable|NULL $previous
+   *   A previous exception which caused this new exception.
    */
   public function __construct($message, $error_code = 0, $errorData = [], $previous = NULL) {
-    parent::__construct($message);
+    // Using int for error code "old way") ?
+    if (is_numeric($error_code)) {
+      $code = $error_code;
+    }
+    else {
+      $code = 0;
+    }
+    // DIVERGENCE: CRM_Core_Exception used `$message`. API_Exception and CiviCRM_API3_Exception used `ts($message)`.
+    // IMHO, this seems like a terrible place to put `ts()` - civistrings won't detect, and it's hard dependency on l10n services.
+    parent::__construct($message, $code, $previous);
     $this->errorData = $errorData + ['error_code' => $error_code];
   }
 
@@ -49,7 +64,7 @@ class CRM_Core_Exception extends PEAR_Exception {
   /**
    * Get error code.
    *
-   * @return mixed
+   * @return int|string
    */
   public function getErrorCode() {
     return $this->errorData['error_code'];
@@ -74,4 +89,31 @@ class CRM_Core_Exception extends PEAR_Exception {
     return $this->errorData;
   }
 
+  /**
+   * Get extra parameters.
+   *
+   * @return array
+   * @deprecated Remove in v6.0. Compatibility with older API_Exception/CiviCRM_API3_Exception contracts.
+   */
+  public function getExtraParams() {
+    return $this->errorData;
+  }
+
+  /**
+   * Get error codes.
+   *
+   * DIVERGENCE: API_Exception defined a discoverable list of error-codes. CRM_Core_Exception and CiviCRM_API3_Exception did not.
+   *
+   * @return array
+   */
+  public function getErrorCodes() {
+    return [
+      2000 => '$params was not an array',
+      2001 => 'Invalid Value for Date field',
+      2100 => 'String value is longer than permitted length',
+      self::UNAUTHORIZED => 'Unauthorized',
+      self::NOT_IMPLEMENTED => 'Entity or method is not implemented',
+    ];
+  }
+
 }

diff --git a/api/Exception.php b/api/Exception.php
index b32a20a3e1c57c253425315128facda0a6404117..78498fb90170f7a51f9e2c2ba3ab33fcdd5ce355 100644 (file)
--- a/api/Exception.php
+++ b/api/Exception.php
@@ -8,134 +8,7 @@
  * @copyright CiviCRM LLC https://civicrm.org/licensing
  */
 
-/**
- * This api exception returns more information than the default one. The aim
- * it let the api consumer know better what is exactly the error without
- * having to parse the error message.
- *
- * If you consume an api that doesn't return an error_code or the extra data
- * you need, consider improving the api and contribute.
- */
-class API_Exception extends Exception {
-  const UNAUTHORIZED = 'unauthorized';
-  const NOT_IMPLEMENTED = 'not-found';
-
-  private $extraParams = [];
-
-  /**
-   * Class constructor.
-   *
-   * @param string $message
-   *   The human friendly error message.
-   * @param mixed $error_code
-   *   A computer friendly error code. By convention, no space (but underscore
-   *   allowed) (ex: mandatory_missing, duplicate, invalid_format).
-   * @param array $extraParams
-   *   Extra params to return. eg an extra array of ids. It is not mandatory,
-   *   but can help the computer using the api. Keep in mind the api consumer
-   *   isn't to be trusted. eg. the database password is NOT a good extra data.
-   * @param Exception|NULL $previous
-   *   A previous exception which caused this new exception.
-   */
-  public function __construct($message, $error_code = 0, $extraParams = [], Exception $previous = NULL) {
-    // Using int for error code "old way") ?
-    if (is_numeric($error_code)) {
-      $code = $error_code;
-    }
-    else {
-      $code = 0;
-    }
-    parent::__construct(ts($message), $code, $previous);
-    $this->extraParams = $extraParams + ['error_code' => $error_code];
-  }
-
-  /**
-   * Custom string representation of object.
-   *
-   * @return string
-   */
-  public function __toString() {
-    return __CLASS__ . ": [{$this->code}]: {$this->message}\n";
-  }
-
-  /**
-   * Get extra parameters.
-   *
-   * @return array
-   */
-  public function getExtraParams() {
-    return $this->extraParams;
-  }
-
-  /**
-   * Get error codes.
-   *
-   * @return array
-   */
-  public function getErrorCodes() {
-    return [
-      2000 => '$params was not an array',
-      2001 => 'Invalid Value for Date field',
-      2100 => 'String value is longer than permitted length',
-      self::UNAUTHORIZED => 'Unauthorized',
-      self::NOT_IMPLEMENTED => 'Entity or method is not implemented',
-    ];
-  }
-
-}
-
-/**
- * This api exception returns more information than the default one. We are using it rather than
- * API_Exception from the api wrapper as the namespace is more generic
- */
-class CiviCRM_API3_Exception extends Exception {
-  private $extraParams = [];
-
-  /**
-   * Class constructor.
-   *
-   * @param string $message
-   *   The human friendly error message.
-   * @param mixed $error_code
-   *   A computer friendly error code. By convention, no space (but underscore
-   *   allowed) (ex: mandatory_missing, duplicate, invalid_format).
-   * @param array $extraParams
-   *   Extra params to return. eg an extra array of ids. It is not mandatory,
-   *   but can help the computer using the api. Keep in mind the api consumer
-   *   isn't to be trusted. eg. the database password is NOT a good extra data.
-   * @param Exception|NULL $previous
-   *   A previous exception which caused this new exception.
-   */
-  public function __construct($message, $error_code = 0, $extraParams = [], Exception $previous = NULL) {
-    parent::__construct(ts($message));
-    $this->extraParams = $extraParams + ['error_code' => $error_code];
-  }
-
-  /**
-   * Custom string representation of object.
-   *
-   * @return string
-   */
-  public function __toString() {
-    return __CLASS__ . ": [{$this->extraParams['error_code']}: {$this->message}\n";
-  }
-
-  /**
-   * Get error code.
-   *
-   * @return mixed
-   */
-  public function getErrorCode() {
-    return $this->extraParams['error_code'];
-  }
-
-  /**
-   * Get extra parameters.
-   *
-   * @return array
-   */
-  public function getExtraParams() {
-    return $this->extraParams;
-  }
+// These two classes were basically equivalent
 
-}
+class_alias('CRM_Core_Exception', 'API_Exception');
+class_alias('CRM_Core_Exception', 'CiviCRM_API3_Exception');