From e73139d60f43fd2f4b455b88ddf0394cffeb4e3a Mon Sep 17 00:00:00 2001 From: Zainab Amir Date: Thu, 4 Aug 2022 13:03:03 +0500 Subject: [PATCH] feat: add error handling to email task (#30820) --- common/djangoapps/student/tests/test_email.py | 25 +++++++++++++++++++ common/djangoapps/student/views/management.py | 5 +++- openedx/core/djangoapps/user_authn/tasks.py | 2 +- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/common/djangoapps/student/tests/test_email.py b/common/djangoapps/student/tests/test_email.py index 22f1197576..914a26e4bf 100644 --- a/common/djangoapps/student/tests/test_email.py +++ b/common/djangoapps/student/tests/test_email.py @@ -15,6 +15,7 @@ from django.test import TransactionTestCase, override_settings from django.test.client import RequestFactory from django.urls import reverse from django.utils.html import escape +from testfixtures import LogCapture from common.djangoapps.edxmako.shortcuts import marketing_link from common.djangoapps.student.email_helpers import generate_proctoring_requirements_email_context @@ -188,6 +189,30 @@ class ActivationEmailTests(EmailTemplateTagMixin, CacheIsolationTestCase): assert user.is_active assert email.called is False, 'method should not have been called' + @patch('common.djangoapps.student.views.management.send_activation_email.delay') + def test_activation_email_exception(self, mock_task): + """ + Test that if an exception occurs within send_activation_email, it is logged + and not raised. + """ + mock_task.side_effect = Exception('BOOM!') + inactive_user = UserFactory(is_active=False) + Registration().register(inactive_user) + request = RequestFactory().get(settings.SOCIAL_AUTH_INACTIVE_USER_URL) + request.user = inactive_user + with patch('common.djangoapps.edxmako.request_context.get_current_request', return_value=request): + with patch('common.djangoapps.third_party_auth.pipeline.running', return_value=False): + with LogCapture() as logger: + inactive_user_view(request) + assert mock_task.called is True, 'method should have been called' + logger.check_present( + ( + 'edx.student', + 'ERROR', + f'Activation email task failed for user {inactive_user.id}.' + ) + ) + @patch('common.djangoapps.student.views.management.compose_activation_email') def test_send_email_to_inactive_user(self, email): """ diff --git a/common/djangoapps/student/views/management.py b/common/djangoapps/student/views/management.py index 5ec791ba27..d131575193 100644 --- a/common/djangoapps/student/views/management.py +++ b/common/djangoapps/student/views/management.py @@ -239,7 +239,10 @@ def compose_and_send_activation_email(user, profile, user_registration=None, red configuration_helpers.get_value('email_from_address', settings.DEFAULT_FROM_EMAIL) ) - send_activation_email.delay(str(msg), from_address) + try: + send_activation_email.delay(str(msg), from_address) + except Exception: # pylint: disable=broad-except + log.exception(f'Activation email task failed for user {user.id}.') @login_required diff --git a/openedx/core/djangoapps/user_authn/tasks.py b/openedx/core/djangoapps/user_authn/tasks.py index 0104cd2409..27ddfe466e 100644 --- a/openedx/core/djangoapps/user_authn/tasks.py +++ b/openedx/core/djangoapps/user_authn/tasks.py @@ -43,7 +43,7 @@ def check_pwned_password_and_send_track_event(user_id, password, internal_user=F return {} # lint-amnesty, pylint: disable=raise-missing-from -@shared_task(bind=True) +@shared_task(bind=True, default_retry_delay=30, max_retries=2) @set_code_owner_attribute def send_activation_email(self, msg_string, from_address=None): """