Merge pull request #5 from Shin--/improved-error-handling

Improved error handling

CHANGELOG.md

@@ -1,3 +1,11 @@
+# 0.2.1 (2019.05.10)
+
+### Breaking Changes
+
+- `init` doesn't return `False` anymore if API set up/request has failed. Instead it now returns an Exception elaborating on what has happened.
+- Several `loggers` replaced by Exceptions. If you rely on logging you can still catch those exceptions and gain more control over your logging.
+
+
 # 0.2.0 (2019.02.07)
 
 - Added new model `SofortRefund` to store refund data and base64-encoded PAIN file

django_sofortueberweisung/__init__.py

@@ -1 +1 @@
-__version__ = '0.2.0'
+__version__ = '0.2.1'

django_sofortueberweisung/wrappers.py

@@ -2,6 +2,7 @@
 import logging
 import os
 import sys
+import json
 
 import xmltodict
 from django.conf import settings
@@ -24,11 +25,15 @@
     from urllib2 import HTTPError, Request, urlopen
 
 
-class SofortWrapper(object):
+class DjangoSofortError(Exception):
+    pass
+
 
+class SofortWrapper(object):
     api_url = 'https://api.sofort.com/api/xml'
     interface_version = 'django_sofortueberweisung_v%s' % __version__
-    cafile = os.path.join(os.path.abspath(os.path.join(os.path.abspath(os.path.dirname(os.path.dirname(__file__))), 'django_sofortueberweisung')), 'cacert.pem')
+    cafile = os.path.join(os.path.abspath(os.path.join(os.path.abspath(os.path.dirname(os.path.dirname(__file__))), 'django_sofortueberweisung')),
+                          'cacert.pem')
     auth = None
 
     def __init__(self, auth=None):
@@ -37,19 +42,19 @@ def __init__(self, auth=None):
             self.auth = auth
 
     def init(self,
-        amount,
-        email_customer=None,
-        phone_customer=None,
-        user_variables=None,
-        sender=None,
-        reasons=None,
-        currency_code='EUR',
-        success_url=django_sofortueberweisung_settings.SOFORT_SUCCESS_URL,
-        success_link_redirect=django_sofortueberweisung_settings.SOFORT_SUCCESS_REDIRECT,
-        abort_url=django_sofortueberweisung_settings.SOFORT_ABORT_URL,
-        timeout_url=django_sofortueberweisung_settings.SOFORT_TIMEOUT_URL,
-        notification_urls=django_sofortueberweisung_settings.SOFORT_NOTIFICATION_URLS,
-        language_code=django_sofortueberweisung_settings.SOFORT_LANGUAGE_CODE):
+             amount,
+             email_customer=None,
+             phone_customer=None,
+             user_variables=None,
+             sender=None,
+             reasons=None,
+             currency_code='EUR',
+             success_url=django_sofortueberweisung_settings.SOFORT_SUCCESS_URL,
+             success_link_redirect=django_sofortueberweisung_settings.SOFORT_SUCCESS_REDIRECT,
+             abort_url=django_sofortueberweisung_settings.SOFORT_ABORT_URL,
+             timeout_url=django_sofortueberweisung_settings.SOFORT_TIMEOUT_URL,
+             notification_urls=django_sofortueberweisung_settings.SOFORT_NOTIFICATION_URLS,
+             language_code=django_sofortueberweisung_settings.SOFORT_LANGUAGE_CODE):
 
         if not self.auth:
             return False
@@ -76,9 +81,6 @@ def init(self,
         xml_data = render_to_string('django_sofortueberweisung/transaction_init.xml', context=data)
         response = self.call_api(xml_data=xml_data)
 
-        if response is False:
-            return False
-
         errors = []
         if 'errors' in response:
             for error in response['errors']:
@@ -93,11 +95,12 @@ def init(self,
                 for warning in new_transaction['warnings']:
                     warnings.append(warning)
 
+        if errors:
+            raise DjangoSofortError('Errors: [{}]'.format(', '.join([json.dumps(x) for x in errors])))
+
         try:
-            if errors != {} or warnings != {}:
+            if warnings != {}:
                 logger = logging.getLogger(__name__)
-                for error in errors:
-                    logger.error("Sofort: ".format(error.get('message', '')))
                 for warning in warnings:
                     logger.warning("Sofort: ".format(warning.get('message', '')))
         except:
@@ -111,11 +114,11 @@ def init(self,
                 logger.error(_("Sofort: transaction id already in database"))
                 return False
         else:
-            return False
+            raise DjangoSofortError('Failed to create new transaction')
 
     def call_api(self, url=None, xml_data=None):
         if not self.auth:
-            return False
+            raise ValueError('Missing or incorrect authentication.')
         if url is None:
             url = SofortWrapper.api_url
         request = Request(url)
@@ -130,21 +133,23 @@ def call_api(self, url=None, xml_data=None):
             request.data = xml_data
 
         try:
-            if sys.version_info.major > 2 or (sys.version_info.major == 2 and sys.version_info.major > 7 or (sys.version_info.major == 7 and sys.version_info.major >= 9)):
+            if sys.version_info.major > 2 or (
+                    sys.version_info.major == 2 and sys.version_info.major > 7 or (sys.version_info.major == 7 and sys.version_info.major >= 9)):
                 response = urlopen(request, cafile=self.cafile)
             else:
                 response = urlopen(request)
         except HTTPError as e:
-            logger = logging.getLogger(__name__)
             fp = e.fp
             body = fp.read()
             fp.close()
             if hasattr(e, 'code'):
-                logger.error("Paydirekt Error {0}({1}): {2}".format(e.code, e.msg, body))
+                raise DjangoSofortError('Paydirekt Error {0}({1}): {2}'.format(e.code, e.msg, body))
             else:
-                logger.error("Paydirekt Error({0}): {1}".format(e.msg, body))
+                raise DjangoSofortError('Paydirekt Error ({0}): {1}'.format(e.msg, body))
         else:
-            if (hasattr(response, 'status') and str(response.status).startswith('2')) or (hasattr(response, 'status_code') and str(response.status_code).startswith('2')) or (hasattr(response, 'code') and str(response.code).startswith('2')):
+            if (hasattr(response, 'status') and str(response.status).startswith('2')) or (
+                    hasattr(response, 'status_code') and str(response.status_code).startswith('2')) or (
+                    hasattr(response, 'code') and str(response.code).startswith('2')):
                 response_body = response.read()
                 return xmltodict.parse(response_body)
-        return False
+        raise DjangoSofortError('Api request failed.')

tests/tests.py

@@ -14,7 +14,7 @@
 
 from django_sofortueberweisung import settings as django_sofortueberweisung_settings
 from django_sofortueberweisung.models import SofortTransaction
-from django_sofortueberweisung.wrappers import SofortWrapper
+from django_sofortueberweisung.wrappers import SofortWrapper, DjangoSofortError
 
 from .test_response_mockups import TEST_RESPONSES
 
@@ -124,8 +124,7 @@ def test_known_transaction_unknown_at_sofort(self):
         self._create_test_transaction(transaction_id='123-abc-unknown')
         post_data = {'status_notification': {'transaction': '123-abc-unknown'}}
         xml_data = xmltodict.unparse(post_data)
-        response = client.post('/sofort/notify/', data=xml_data, content_type='application/hal+json')
-        self.assertEqual(response.status_code, 400)
+        self.assertRaises(Exception, client.post, '/sofort/notify/', data=xml_data, content_type='application/hal+json')
 
     # TODO: Test XML response
     @replace('django_sofortueberweisung.wrappers.urlopen', mock_urlopen)