Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add BaseFailed exceptions for phone_notificator #2074

Merged
merged 8 commits into from
Jun 8, 2023
32 changes: 18 additions & 14 deletions engine/apps/api/views/user.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
from apps.mobile_app.demo_push import send_test_push
from apps.mobile_app.exceptions import DeviceNotSet
from apps.phone_notifications.exceptions import (
BaseFailed,
FailedToFinishVerification,
FailedToMakeCall,
FailedToStartVerification,
Expand Down Expand Up @@ -340,8 +341,8 @@ def get_verification_code(self, request, pk):
phone_backend.send_verification_sms(user)
except NumberAlreadyVerified:
return Response("Phone number already verified", status=status.HTTP_400_BAD_REQUEST)
except FailedToStartVerification:
return Response("Something went wrong while sending code", status=status.HTTP_500_INTERNAL_SERVER_ERROR)
except FailedToStartVerification as e:
return handle_phone_notificator_failed(e)
except ProviderNotSupports:
return Response(
"Phone provider not supports sms verification", status=status.HTTP_500_INTERNAL_SERVER_ERROR
Expand All @@ -367,8 +368,8 @@ def get_verification_call(self, request, pk):
phone_backend.make_verification_call(user)
except NumberAlreadyVerified:
return Response("Phone number already verified", status=status.HTTP_400_BAD_REQUEST)
except FailedToStartVerification:
return Response("Something went wrong while calling", status=status.HTTP_500_INTERNAL_SERVER_ERROR)
except FailedToStartVerification as e:
return handle_phone_notificator_failed(e)
except ProviderNotSupports:
return Response(
"Phone provider not supports call verification", status=status.HTTP_500_INTERNAL_SERVER_ERROR
Expand All @@ -390,8 +391,8 @@ def verify_number(self, request, pk):
phone_backend = PhoneBackend()
try:
verified = phone_backend.verify_phone_number(target_user, code)
except FailedToFinishVerification:
return Response("Something went wrong while verifying code", status=status.HTTP_503_SERVICE_UNAVAILABLE)
except FailedToFinishVerification as e:
return handle_phone_notificator_failed(e)
if verified:
new_state = target_user.insight_logs_serialized
write_resource_insight_log(
Expand Down Expand Up @@ -432,10 +433,8 @@ def make_test_call(self, request, pk):
phone_backend.make_test_call(user)
except NumberNotVerified:
return Response("Phone number is not verified", status=status.HTTP_400_BAD_REQUEST)
except FailedToMakeCall:
return Response(
"Something went wrong while making a test call", status=status.HTTP_500_INTERNAL_SERVER_ERROR
)
except FailedToMakeCall as e:
return handle_phone_notificator_failed(e)
except ProviderNotSupports:
return Response("Phone provider not supports phone calls", status=status.HTTP_500_INTERNAL_SERVER_ERROR)

Expand All @@ -449,10 +448,8 @@ def send_test_sms(self, request, pk):
phone_backend.send_test_sms(user)
except NumberNotVerified:
return Response("Phone number is not verified", status=status.HTTP_400_BAD_REQUEST)
except FailedToMakeCall:
return Response(
"Something went wrong while making a test call", status=status.HTTP_500_INTERNAL_SERVER_ERROR
)
except FailedToMakeCall as e:
return handle_phone_notificator_failed(e)
except ProviderNotSupports:
return Response("Phone provider not supports phone calls", status=status.HTTP_500_INTERNAL_SERVER_ERROR)

Expand Down Expand Up @@ -650,3 +647,10 @@ def check_availability(self, request, pk):
user = self.get_object()
warnings = check_user_availability(user=user, team=request.user.current_team)
return Response(data={"warnings": warnings}, status=status.HTTP_200_OK)


def handle_phone_notificator_failed(exc: BaseFailed) -> Response:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
def handle_phone_notificator_failed(exc: BaseFailed) -> Response:
def handle_phone_notification_failed(exc: BaseFailed) -> Response:

if exc.graceful_msg:
return Response(exc.graceful_msg, status=status.HTTP_400_BAD_REQUEST)
else:
return Response("Something went wrong", status=status.HTTP_503_SERVICE_UNAVAILABLE)
28 changes: 22 additions & 6 deletions engine/apps/phone_notifications/exceptions.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,40 @@
class FailedToMakeCall(Exception):
class BaseFailed(Exception):
"""
Failed is base exception for all Failed... exceptions.
This exception is indicates error while performing some phone notification operation.
Optionally can contain graceful_msg attribute. When graceful_msg is provided it mean that error on provider side is
not our fault, but some provider error (number is blocked, fraud guard, ...).
By default, graceful_msg is None - it means that error is our fault (network problems, invalid configuration,...).

Attributes:
graceful_msg: string with some details about exception which can be exposed to caller.
"""

def __init__(self, graceful_msg=None):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

where is graceful_msg being set?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nowhere yet. but it's expected to be set in concrete providers, me or @mderynck will add this

self.graceful_msg = graceful_msg


class FailedToMakeCall(BaseFailed):
pass


class FailedToSendSMS(Exception):
class FailedToSendSMS(BaseFailed):
pass


class NumberNotVerified(Exception):
class FailedToStartVerification(BaseFailed):
pass


class NumberAlreadyVerified(Exception):
class FailedToFinishVerification(BaseFailed):
pass


class FailedToStartVerification(Exception):
class NumberNotVerified(Exception):
pass


class FailedToFinishVerification(Exception):
class NumberAlreadyVerified(Exception):
pass


Expand Down
43 changes: 35 additions & 8 deletions engine/apps/twilioapp/phone_provider.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,14 @@ def make_notification_call(self, number: str, message: str) -> TwilioPhoneCall:
try_without_callback = True
else:
logger.error(f"TwilioPhoneProvider.make_notification_call: failed {e}")
raise FailedToMakeCall
raise FailedToMakeCall(graceful_msg=self._get_graceful_msg(e, number))

if try_without_callback:
try:
response = self._call_create(twiml_query, number, with_callback=False)
except TwilioRestException as e:
logger.error(f"TwilioPhoneProvider.make_notification_call: failed {e}")
raise FailedToMakeCall
raise FailedToMakeCall(graceful_msg=self._get_graceful_msg(e, number))

if response and response.status and response.sid:
return TwilioPhoneCall(
Expand All @@ -72,14 +72,14 @@ def send_notification_sms(self, number: str, message: str) -> TwilioSMS:
try_without_callback = True
else:
logger.error(f"TwilioPhoneProvider.send_notification_sms: failed {e}")
raise FailedToSendSMS
raise FailedToSendSMS(graceful_msg=self._get_graceful_msg(e, number))

if try_without_callback:
try:
response = self._messages_create(number, message, with_callback=False)
except TwilioRestException as e:
logger.error(f"TwilioPhoneProvider.send_notification_sms: failed {e}")
raise FailedToSendSMS
raise FailedToSendSMS(graceful_msg=self._get_graceful_msg(e, number))

if response and response.status and response.sid:
return TwilioSMS(
Expand All @@ -106,24 +106,51 @@ def finish_verification(self, number: str, code: str):
return normalized_number
except TwilioRestException as e:
logger.error(f"TwilioPhoneProvider.finish_verification: failed to verify number {number}: {e}")
raise FailedToFinishVerification
raise FailedToFinishVerification(graceful_msg=self._get_graceful_msg(e, number))
else:
return None

"""
Errors we will raise without graceful messages:

20404 - We should not be requesting missing resources
30808 - Unknown error, likely on the carrier side
30007, 32017 - Blocked or filtered, Intermediary / Carrier Analytics blocked call
due to poor reputation score on the telephone number:
* We need to register our number or sender with the analytics provider or carrier for that jurisdiction
"""

def _get_graceful_msg(self, e, number):
if e.code in (30003, 30005):
return f"Destination handset {number} is unreachable"
elif e.code == 30004:
return f"Sending message to {number} is blocked"
elif e.code == 30006:
return f"Cannot send to {number} is landline or unreachable carrier"
elif e.code == 30410:
return f"Provider for {number} is experiencing timeouts"
elif e.code == 60200:
return f"{number} is incorrectly formatted"
elif e.code in (21215, 60410, 60605):
return f"Verification to {number} is blocked"
elif e.code == 60203:
return f"Max verification attempts for {number} reached"
return None
Comment on lines +113 to +138
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice 👍


def make_call(self, number: str, message: str):
twiml_query = self._message_to_twiml(message, with_gather=False)
try:
self._call_create(twiml_query, number, with_callback=False)
except TwilioRestException as e:
logger.error(f"TwilioPhoneProvider.make_call: failed {e}")
raise FailedToMakeCall
raise FailedToMakeCall(graceful_msg=self._get_graceful_msg(e, number))

def send_sms(self, number: str, message: str):
try:
self._messages_create(number, message, with_callback=False)
except TwilioRestException as e:
logger.error(f"TwilioPhoneProvider.send_sms: failed {e}")
raise FailedToSendSMS
raise FailedToSendSMS(graceful_msg=self._get_graceful_msg(e, number))

def _message_to_twiml(self, message: str, with_gather=False):
q = f"<Response><Say>{message}</Say></Response>"
Expand Down Expand Up @@ -178,7 +205,7 @@ def _send_verification_code(self, number: str, via: str):
logger.info(f"TwilioPhoneProvider._send_verification_code: verification status {verification.status}")
except TwilioRestException as e:
logger.error(f"Twilio verification start error: {e} to number {number}")
raise FailedToStartVerification
raise FailedToStartVerification(graceful_msg=self._get_graceful_msg(e, number))

def _normalize_phone_number(self, number: str):
# TODO: phone_provider: is it best place to parse phone number?
Expand Down
91 changes: 91 additions & 0 deletions engine/apps/twilioapp/tests/test_twilio_provider.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
from unittest import mock

import pytest
from twilio.base.exceptions import TwilioRestException

from apps.phone_notifications.exceptions import FailedToFinishVerification, FailedToMakeCall, FailedToSendSMS
from apps.twilioapp.phone_provider import TwilioPhoneProvider


Expand All @@ -10,6 +12,11 @@ class MockTwilioCallInstance:
sid = "mock_sid"


class MockTwilioMessageInstance:
status = "mock_status"
sid = "mock_sid"


@pytest.mark.django_db
@mock.patch("apps.twilioapp.phone_provider.TwilioPhoneProvider._call_create", return_value=MockTwilioCallInstance())
@mock.patch("apps.twilioapp.phone_provider.TwilioPhoneProvider._message_to_twiml", return_value="mocked_twiml")
Expand Down Expand Up @@ -63,3 +70,87 @@ def test_send_sms(mock_messages_create):
provider_sms = provider.send_sms(number, message)
assert provider_sms is None # test that provider_sms is not returned from send_sms
mock_messages_create.assert_called_once_with(number, message, with_callback=False)


TEST_NUMBER = "+1234567890"
TEST_MESSAGE = "Hello"
TEST_CODE = "12345"


@pytest.mark.django_db
@pytest.mark.parametrize(
"twilio_code,expected_exception,graceful_msg,provider_method,mock_method",
[
(60200, FailedToMakeCall, True, lambda p: p.make_call(TEST_NUMBER, TEST_MESSAGE), "_call_create"),
(30808, FailedToMakeCall, False, lambda p: p.make_call(TEST_NUMBER, TEST_MESSAGE), "_call_create"),
(None, FailedToMakeCall, False, lambda p: p.make_call(TEST_NUMBER, TEST_MESSAGE), "_call_create"),
(30410, FailedToMakeCall, True, lambda p: p.make_notification_call(TEST_NUMBER, TEST_MESSAGE), "_call_create"),
(30808, FailedToMakeCall, False, lambda p: p.make_notification_call(TEST_NUMBER, TEST_MESSAGE), "_call_create"),
(None, FailedToMakeCall, False, lambda p: p.make_notification_call(TEST_NUMBER, TEST_MESSAGE), "_call_create"),
(30004, FailedToSendSMS, True, lambda p: p.send_sms(TEST_NUMBER, TEST_MESSAGE), "_messages_create"),
(30808, FailedToSendSMS, False, lambda p: p.send_sms(TEST_NUMBER, TEST_MESSAGE), "_messages_create"),
(None, FailedToSendSMS, False, lambda p: p.send_sms(TEST_NUMBER, TEST_MESSAGE), "_messages_create"),
(
30006,
FailedToSendSMS,
True,
lambda p: p.send_notification_sms(TEST_NUMBER, TEST_MESSAGE),
"_messages_create",
),
(
30808,
FailedToSendSMS,
False,
lambda p: p.send_notification_sms(TEST_NUMBER, TEST_MESSAGE),
"_messages_create",
),
(
None,
FailedToSendSMS,
False,
lambda p: p.send_notification_sms(TEST_NUMBER, TEST_MESSAGE),
"_messages_create",
),
(
60203,
FailedToFinishVerification,
True,
lambda p: p.finish_verification(TEST_NUMBER, TEST_CODE),
"_verify_sender",
),
(
30808,
FailedToFinishVerification,
False,
lambda p: p.finish_verification(TEST_NUMBER, TEST_CODE),
"_verify_sender",
),
(
None,
FailedToFinishVerification,
False,
lambda p: p.finish_verification(TEST_NUMBER, TEST_CODE),
"_verify_sender",
),
],
)
@mock.patch("apps.twilioapp.phone_provider.TwilioPhoneProvider._normalize_phone_number", return_value=(TEST_NUMBER, 1))
def test_twilio_provider_exceptions(
mocked_normalize, twilio_code, expected_exception, graceful_msg, provider_method, mock_method
):
provider = TwilioPhoneProvider()

with mock.patch(f"apps.twilioapp.phone_provider.TwilioPhoneProvider.{mock_method}") as twilio_mock:
twilio_mock.side_effect = TwilioRestException(500, "", code=twilio_code)
with pytest.raises(expected_exception) as exc:
provider_method(provider)
if graceful_msg:
assert len(exc.value.graceful_msg) > 0
else:
assert exc.value.graceful_msg is None
twilio_mock.assert_called_once()
Comment on lines +80 to +151
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍



class MockTwilioSMSInstance:
status = "mock_status"
sid = "mock_sid"