From 94ce42cccf1e5f6a4aba9702899872c0908fb220 Mon Sep 17 00:00:00 2001 From: eileen Date: Wed, 11 Dec 2019 00:11:41 +1300 Subject: [PATCH] Update Paypal invokeApi to only throw exceptions. Currently invokeAPI will return an error for a curl function and throw a processorException for an outcome other than success. In just about every instance where it is called the Core_Error is converted into an exception anyway - this can all be cleaned up as a follow up. The exceptions are doQuery - called by doPayment doDirectPayment - this is called by doPayment which converts it into an exception. Calling doDirectPayment directly was deprecated in 4.6 createRecurringPayments - called by doExpressCheckout which in already throws exceptions in other scenarios, updateSubscriptionBillingInfo - we can update the one place that calls this in core as a follow up like https://github.com/civicrm/civicrm-core/pull/15676 However, note that an exception was already thrown in all these places in the more common error scenario of the action not succeeding as opposed to the rare curl error scenario Follow ons to consider 1) introduce more nuance into PaymentProcessorExceptions - extend the exception class with PaymentProcessorConfigException, PaymentProcessingException or similar to differentiate 2) remove all the handling for invokeApi to have returned an error object. 3) fix the code that calls updateSubscriptionBillingInfo 4) ensure docs reflect that doPayment should throw an exception. AFAIK we have not yet documented other 'do' functions & I'd prefer to stdise them first --- CRM/Core/Payment/PayPalImpl.php | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/CRM/Core/Payment/PayPalImpl.php b/CRM/Core/Payment/PayPalImpl.php index 21f9fd37b2d1..ad49b89b0212 100644 --- a/CRM/Core/Payment/PayPalImpl.php +++ b/CRM/Core/Payment/PayPalImpl.php @@ -1016,13 +1016,13 @@ public function doTransferCheckout(&$params, $component = 'contribute') { * @param null $url * * @return array|object - * @throws \Exception + * @throws \Civi\Payment\Exception\PaymentProcessorException */ public function invokeAPI($args, $url = NULL) { if ($url === NULL) { if (empty($this->_paymentProcessor['url_api'])) { - CRM_Core_Error::fatal(ts('Please set the API URL. Please refer to the documentation for more details')); + throw new PaymentProcessorException(ts('Please set the API URL. Please refer to the documentation for more details')); } $url = $this->_paymentProcessor['url_api'] . 'nvp'; @@ -1062,16 +1062,9 @@ public function invokeAPI($args, $url = NULL) { $result = self::deformat($response); if (curl_errno($ch)) { - $e = CRM_Core_Error::singleton(); - $e->push(curl_errno($ch), - 0, NULL, - curl_error($ch) - ); - return $e; - } - else { - curl_close($ch); + throw new PaymentProcessorException(ts('Network error') . ' ' . curl_error($ch) . curl_errno($ch), curl_errno($ch)); } + curl_close($ch); $outcome = strtolower(CRM_Utils_Array::value('ack', $result));