From f51583d75b47b6e8ad0edbbc16d854035c9e230d Mon Sep 17 00:00:00 2001
From: Jack Wink <jackwink@umich.edu>
Date: Mon, 16 May 2016 16:15:47 -0700
Subject: [PATCH] wrap NumberParseException so it isn't cast

SQLAlchemy automatically catches exceptions from types and wraps
them as a `StatementError` if they don't extend from the
`DontWrapMixin`. This PR creates a new exception type that combines
the native phonenumbers exception with the sqlalchemy mixin.
---
 sqlalchemy_utils/__init__.py           |  1 +
 sqlalchemy_utils/types/__init__.py     |  6 +++-
 sqlalchemy_utils/types/phone_number.py | 38 +++++++++++++++++++++++---
 tests/types/test_phonenumber.py        | 24 +++++++++++++++-
 4 files changed, 63 insertions(+), 6 deletions(-)

diff --git a/sqlalchemy_utils/__init__.py b/sqlalchemy_utils/__init__.py
index 7529efc..5af0845 100644
--- a/sqlalchemy_utils/__init__.py
+++ b/sqlalchemy_utils/__init__.py
@@ -82,6 +82,7 @@ from .types import (  # noqa
     Password,
     PasswordType,
     PhoneNumber,
+    PhoneNumberParseException,
     PhoneNumberType,
     register_composites,
     remove_composite_listeners,
diff --git a/sqlalchemy_utils/types/__init__.py b/sqlalchemy_utils/types/__init__.py
index 71dc84f..29a400e 100644
--- a/sqlalchemy_utils/types/__init__.py
+++ b/sqlalchemy_utils/types/__init__.py
@@ -20,7 +20,11 @@ from .pg_composite import (  # noqa
     register_composites,
     remove_composite_listeners
 )
-from .phone_number import PhoneNumber, PhoneNumberType  # noqa
+from .phone_number import (  # noqa
+    PhoneNumber,
+    PhoneNumberParseException,
+    PhoneNumberType
+)
 from .range import (  # noqa
     DateRangeType,
     DateTimeRangeType,
diff --git a/sqlalchemy_utils/types/phone_number.py b/sqlalchemy_utils/types/phone_number.py
index 02b6606..d216d05 100644
--- a/sqlalchemy_utils/types/phone_number.py
+++ b/sqlalchemy_utils/types/phone_number.py
@@ -1,4 +1,5 @@
-from sqlalchemy import types
+import six
+from sqlalchemy import exc, types
 
 from ..exceptions import ImproperlyConfigured
 from ..utils import str_coercible
@@ -7,9 +8,23 @@ from .scalar_coercible import ScalarCoercible
 try:
     import phonenumbers
     from phonenumbers.phonenumber import PhoneNumber as BasePhoneNumber
+    from phonenumbers.phonenumberutil import NumberParseException
 except ImportError:
     phonenumbers = None
     BasePhoneNumber = object
+    NumberParseException = Exception
+
+
+class PhoneNumberParseException(NumberParseException, exc.DontWrapMixin):
+    '''
+    Wraps exceptions from phonenumbers with SQLAlchemy's DontWrapMixin
+    so we get more meaningful exceptions on validation failure instead of the
+    StatementException
+
+    Clients can catch this as either a PhoneNumberParseException or
+    NumberParseException from the phonenumbers library.
+    '''
+    pass
 
 
 @str_coercible
@@ -62,9 +77,23 @@ class PhoneNumber(BasePhoneNumber):
         # Bail if phonenumbers is not found.
         if phonenumbers is None:
             raise ImproperlyConfigured(
-                "'phonenumbers' is required to use 'PhoneNumber'")
+                "'phonenumbers' is required to use 'PhoneNumber'"
+            )
+
+        try:
+            self._phone_number = phonenumbers.parse(raw_number, region)
+        except NumberParseException as e:
+            # Wrap exception so SQLAlchemy doesn't swallow it as a
+            # StatementError
+            #
+            # Worth noting that if -1 shows up as the error_type
+            # it's likely because the API has changed upstream and these
+            # bindings need to be updated.
+            raise PhoneNumberParseException(
+                getattr(e, 'error_type', -1),
+                six.text_type(e)
+            )
 
-        self._phone_number = phonenumbers.parse(raw_number, region)
         super(PhoneNumber, self).__init__(
             country_code=self._phone_number.country_code,
             national_number=self._phone_number.national_number,
@@ -132,7 +161,8 @@ class PhoneNumberType(types.TypeDecorator, ScalarCoercible):
         # Bail if phonenumbers is not found.
         if phonenumbers is None:
             raise ImproperlyConfigured(
-                "'phonenumbers' is required to use 'PhoneNumberType'")
+                "'phonenumbers' is required to use 'PhoneNumberType'"
+            )
 
         super(PhoneNumberType, self).__init__(*args, **kwargs)
         self.region = region
diff --git a/tests/types/test_phonenumber.py b/tests/types/test_phonenumber.py
index ddc7284..f7feee3 100644
--- a/tests/types/test_phonenumber.py
+++ b/tests/types/test_phonenumber.py
@@ -2,7 +2,12 @@ import pytest
 import six
 import sqlalchemy as sa
 
-from sqlalchemy_utils import PhoneNumber, PhoneNumberType, types  # noqa
+from sqlalchemy_utils import (  # noqa
+    PhoneNumber,
+    PhoneNumberParseException,
+    PhoneNumberType,
+    types
+)
 
 
 @pytest.fixture
@@ -77,6 +82,23 @@ class TestPhoneNumber(object):
             except:
                 pass
 
+    def test_invalid_phone_numbers_throw_dont_wrap_exception(
+        self,
+        session,
+        User
+    ):
+        try:
+            session.execute(
+                User.__table__.insert().values(
+                    name='Someone',
+                    phone_number='abc'
+                )
+            )
+        except PhoneNumberParseException:
+            pass
+        except:
+            assert False
+
     def test_phone_number_attributes(self):
         number = PhoneNumber('+358401234567')
         assert number.e164 == u'+358401234567'