diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index d1c8092cc9..7893e823b4 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -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, @@ -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 @@ -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 @@ -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( @@ -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) @@ -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) @@ -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: + 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) diff --git a/engine/apps/phone_notifications/exceptions.py b/engine/apps/phone_notifications/exceptions.py index 97b9348b2a..a605ac8118 100644 --- a/engine/apps/phone_notifications/exceptions.py +++ b/engine/apps/phone_notifications/exceptions.py @@ -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): + 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 diff --git a/engine/apps/twilioapp/phone_provider.py b/engine/apps/twilioapp/phone_provider.py index 87a6450f2c..6bc3a46a7b 100644 --- a/engine/apps/twilioapp/phone_provider.py +++ b/engine/apps/twilioapp/phone_provider.py @@ -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( @@ -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( @@ -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 + 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"{message}" @@ -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? diff --git a/engine/apps/twilioapp/tests/test_twilio_provider.py b/engine/apps/twilioapp/tests/test_twilio_provider.py index 20892109bd..1d7384b635 100644 --- a/engine/apps/twilioapp/tests/test_twilio_provider.py +++ b/engine/apps/twilioapp/tests/test_twilio_provider.py @@ -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 @@ -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") @@ -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() + + +class MockTwilioSMSInstance: + status = "mock_status" + sid = "mock_sid"