Andal.LND/edx-platform
6
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: replacing exceptions (#31827)

Browse Source 
* fix: fixing test.
* fix: updating exceptions.
This commit is contained in:
Awais Qureshi
2023-05-25 17:45:11 +05:00
committed by GitHub
parent 16955828e7
commit fcb4c4d098
2 changed files with 68 additions and 92 deletions
Download Patch File Download Diff File Expand all files Collapse all files

96
lms/djangoapps/bulk_email/tasks.py
View File

@@ -10,21 +10,10 @@ import re
import time
from collections import Counter
from datetime import datetime
from smtplib import SMTPConnectError, SMTPDataError, SMTPException, SMTPServerDisconnected, SMTPSenderRefused
from smtplib import SMTPConnectError, SMTPDataError, SMTPException, SMTPSenderRefused, SMTPServerDisconnected
from time import sleep
from boto.exception import AWSConnectionError
from boto.ses.exceptions import (
SESAddressBlacklistedError,
SESAddressNotVerifiedError,
SESDailyQuotaExceededError,
SESDomainEndsWithDotError,
SESDomainNotConfirmedError,
SESIdentityNotVerifiedError,
SESIllegalAddressError,
SESLocalAddressCharacterError,
SESMaxSendingRateExceededError
)
from botocore.exceptions import ClientError, EndpointConnectionError
from celery import current_task, shared_task
from celery.exceptions import RetryTaskError
from celery.states import FAILURE, RETRY, SUCCESS
@@ -34,8 +23,8 @@ from django.core.mail import get_connection
from django.core.mail.message import forbid_multi_line_headers
from django.urls import reverse
from django.utils import timezone
from django.utils.translation import override as override_language
from django.utils.translation import gettext as _
from django.utils.translation import override as override_language
from edx_django_utils.monitoring import set_code_owner_attribute
from markupsafe import escape
@@ -43,15 +32,12 @@ from common.djangoapps.util.date_utils import get_default_time_display
from common.djangoapps.util.string_utils import _has_non_ascii_characters
from lms.djangoapps.branding.api import get_logo_url_for_email
from lms.djangoapps.bulk_email.api import get_unsubscribed_link
from lms.djangoapps.bulk_email.messages import (
DjangoEmail,
ACEEmail,
)
from lms.djangoapps.bulk_email.messages import ACEEmail, DjangoEmail
from lms.djangoapps.bulk_email.models import CourseEmail, Optout
from lms.djangoapps.bulk_email.toggles import (
is_bulk_email_edx_ace_enabled,
is_email_use_course_id_from_for_bulk_enabled,
is_email_use_course_id_from_for_bulk_enabled
)
from lms.djangoapps.bulk_email.models import CourseEmail, Optout
from lms.djangoapps.courseware.courses import get_course
from lms.djangoapps.instructor_task.models import InstructorTask
from lms.djangoapps.instructor_task.subtasks import (
@@ -66,13 +52,11 @@ from openedx.core.lib.courses import course_image_url
log = logging.getLogger('edx.celery.task')
# Errors that an individual email is failing to be sent, and should just
# be treated as a fail.
SINGLE_EMAIL_FAILURE_ERRORS = (
SESAddressBlacklistedError, # Recipient's email address has been temporarily blacklisted.
SESDomainEndsWithDotError, # Recipient's email address' domain ends with a period/dot.
SESIllegalAddressError, # Raised when an illegal address is encountered.
SESLocalAddressCharacterError, # An address contained a control or whitespace character.
ClientError
)
# Exceptions that, if caught, should cause the task to be re-tried.
@@ -80,7 +64,7 @@ SINGLE_EMAIL_FAILURE_ERRORS = (
LIMITED_RETRY_ERRORS = (
SMTPConnectError,
SMTPServerDisconnected,
AWSConnectionError,
EndpointConnectionError,
)
# Errors that indicate that a mailing task should be retried without limit.
@@ -91,20 +75,17 @@ LIMITED_RETRY_ERRORS = (
# Those not in this range (i.e. in the 5xx range) are treated as hard failures
# and thus like SINGLE_EMAIL_FAILURE_ERRORS.
INFINITE_RETRY_ERRORS = (
SESMaxSendingRateExceededError, # Your account's requests/second limit has been exceeded.
SMTPDataError,
SMTPSenderRefused,
ClientError
)
# Errors that are known to indicate an inability to send any more emails,
# and should therefore not be retried. For example, exceeding a quota for emails.
# Also, any SMTP errors that are not explicitly enumerated above.
BULK_EMAIL_FAILURE_ERRORS = (
SESAddressNotVerifiedError, # Raised when a "Reply-To" address has not been validated in SES yet.
SESIdentityNotVerifiedError, # Raised when an identity has not been verified in SES yet.
SESDomainNotConfirmedError, # Raised when domain ownership is not confirmed for DKIM.
SESDailyQuotaExceededError, # 24-hour allotment of outbound email has been exceeded.
SMTPException,
ClientError,
SMTPException
)
@@ -588,13 +569,16 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
except SINGLE_EMAIL_FAILURE_ERRORS as exc:
# This will fall through and not retry the message.
total_recipients_failed += 1
log.exception(
f"BulkEmail ==> Status: Failed(SINGLE_EMAIL_FAILURE_ERRORS), Task: {parent_task_id}, SubTask: "
f"{task_id}, EmailId: {email_id}, Recipient num: {recipient_num}/{total_recipients}, Recipient "
f"UserId: {current_recipient['pk']}"
)
subtask_status.increment(failed=1)
if exc.response['Error']['Code'] in ['MessageRejected', 'MailFromDomainNotVerified', 'MailFromDomainNotVerifiedException', 'FromEmailAddressNotVerifiedException']: # lint-amnesty, pylint: disable=line-too-long
total_recipients_failed += 1
log.exception(
f"BulkEmail ==> Status: Failed(SINGLE_EMAIL_FAILURE_ERRORS), Task: {parent_task_id}, SubTask: "
f"{task_id}, EmailId: {email_id}, Recipient num: {recipient_num}/{total_recipients}, Recipient "
f"UserId: {current_recipient['pk']}"
)
subtask_status.increment(failed=1)
else:
raise exc
else:
total_recipients_successful += 1
@@ -631,10 +615,13 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
except INFINITE_RETRY_ERRORS as exc:
# Increment the "retried_nomax" counter, update other counters with progress to date,
# and set the state to RETRY:
subtask_status.increment(retried_nomax=1, state=RETRY)
return _submit_for_retry(
entry_id, email_id, to_list, global_email_context, exc, subtask_status, skip_retry_max=True
)
if isinstance(exc, (SMTPDataError, SMTPSenderRefused)) or exc.response['Error']['Code'] in ['LimitExceededException']: # lint-amnesty, pylint: disable=line-too-long
subtask_status.increment(retried_nomax=1, state=RETRY)
return _submit_for_retry(
entry_id, email_id, to_list, global_email_context, exc, subtask_status, skip_retry_max=True
)
else:
raise exc
except LIMITED_RETRY_ERRORS as exc:
# Errors caught here cause the email to be retried. The entire task is actually retried
@@ -642,21 +629,28 @@ def _send_course_email(entry_id, email_id, to_list, global_email_context, subtas
# Errors caught are those that indicate a temporary condition that might succeed on retry.
# Increment the "retried_withmax" counter, update other counters with progress to date,
# and set the state to RETRY:
subtask_status.increment(retried_withmax=1, state=RETRY)
return _submit_for_retry(
entry_id, email_id, to_list, global_email_context, exc, subtask_status, skip_retry_max=False
)
except BULK_EMAIL_FAILURE_ERRORS as exc:
num_pending = len(to_list)
log.exception(
f"Task {task_id}: email with id {email_id} caused send_course_email task to fail with 'fatal' exception. "
f"{num_pending} emails unsent."
)
# Update counters with progress to date, counting unsent emails as failures,
# and set the state to FAILURE:
subtask_status.increment(failed=num_pending, state=FAILURE)
return subtask_status, exc
if isinstance(exc, SMTPException) or exc.response['Error']['Code'] in [
'AccountSendingPausedException', 'MailFromDomainNotVerifiedException', 'LimitExceededException'
]:
num_pending = len(to_list)
log.exception(
f"Task {task_id}: email with id {email_id} caused send_course_email "
f"task to fail with 'fatal' exception. "
f"{num_pending} emails unsent."
)
# Update counters with progress to date, counting unsent emails as failures,
# and set the state to FAILURE:
subtask_status.increment(failed=num_pending, state=FAILURE)
return subtask_status, exc
else:
raise exc
except Exception as exc: # pylint: disable=broad-except
# Errors caught here cause the email to be retried. The entire task is actually retried

64
lms/djangoapps/bulk_email/tests/test_tasks.py
View File

@@ -7,27 +7,23 @@ paths actually work.
"""
from datetime import datetime
from dateutil.relativedelta import relativedelta
import json # lint-amnesty, pylint: disable=wrong-import-order
from datetime import datetime
from itertools import chain, cycle, repeat # lint-amnesty, pylint: disable=wrong-import-order
from smtplib import SMTPAuthenticationError, SMTPConnectError, SMTPDataError, SMTPServerDisconnected, SMTPSenderRefused # lint-amnesty, pylint: disable=wrong-import-order
from smtplib import ( # lint-amnesty, pylint: disable=wrong-import-order
SMTPAuthenticationError,
SMTPConnectError,
SMTPDataError,
SMTPSenderRefused,
SMTPServerDisconnected
)
from unittest.mock import Mock, patch # lint-amnesty, pylint: disable=wrong-import-order
from uuid import uuid4 # lint-amnesty, pylint: disable=wrong-import-order
import pytest
from boto.exception import AWSConnectionError
from boto.ses.exceptions import (
SESAddressBlacklistedError,
SESAddressNotVerifiedError,
SESDailyQuotaExceededError,
SESDomainEndsWithDotError,
SESDomainNotConfirmedError,
SESIdentityNotVerifiedError,
SESIllegalAddressError,
SESLocalAddressCharacterError,
SESMaxSendingRateExceededError
)
from botocore.exceptions import ClientError, EndpointConnectionError
from celery.states import FAILURE, SUCCESS
from dateutil.relativedelta import relativedelta
from django.conf import settings
from django.core.management import call_command
from django.test.utils import override_settings
@@ -268,19 +264,22 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
def test_ses_blacklisted_user(self):
# Test that celery handles permanent SMTPDataErrors by failing and not retrying.
self._test_email_address_failures(SESAddressBlacklistedError(554, "Email address is blacklisted"))
operation_name = ''
parsed_response = {'Error': {'Code': 'MessageRejected', 'Message': 'Error Uploading'}}
self._test_email_address_failures(ClientError(parsed_response, operation_name))
def test_ses_illegal_address(self):
# Test that celery handles permanent SMTPDataErrors by failing and not retrying.
self._test_email_address_failures(SESIllegalAddressError(554, "Email address is illegal"))
def test_ses_local_address_character_error(self):
# Test that celery handles permanent SMTPDataErrors by failing and not retrying.
self._test_email_address_failures(SESLocalAddressCharacterError(554, "Email address contains a bad character"))
operation_name = ''
parsed_response = {'Error': {'Code': 'MailFromDomainNotVerifiedException', 'Message': 'Error Uploading'}}
self._test_email_address_failures(ClientError(parsed_response, operation_name))
def test_ses_domain_ends_with_dot(self):
# Test that celery handles permanent SMTPDataErrors by failing and not retrying.
self._test_email_address_failures(SESDomainEndsWithDotError(554, "Email address ends with a dot"))
operation_name = ''
parsed_response = {'Error': {'Code': 'MailFromDomainNotVerifiedException', 'Message': 'invalid domain'}}
self._test_email_address_failures(ClientError(parsed_response, operation_name))
def test_bulk_email_skip_with_non_ascii_emails(self):
"""
@@ -367,12 +366,12 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
def test_retry_after_aws_connect_error(self):
self._test_retry_after_limited_retry_error(
AWSConnectionError("Unable to provide secure connection through proxy")
EndpointConnectionError(endpoint_url="Could not connect to the endpoint URL:")
)
def test_max_retry_after_aws_connect_error(self):
self._test_max_retry_limit_causes_failure(
AWSConnectionError("Unable to provide secure connection through proxy")
EndpointConnectionError(endpoint_url="Could not connect to the endpoint URL:")
)
def test_retry_after_general_error(self):
@@ -416,11 +415,6 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
SMTPSenderRefused(421, "Throttling: Sending rate exceeded", self.instructor.email)
)
def test_retry_after_ses_throttling_error(self):
self._test_retry_after_unlimited_retry_error(
SESMaxSendingRateExceededError(455, "Throttling: Sending rate exceeded")
)
def _test_immediate_failure(self, exception):
"""Test that celery can hit a maximum number of retries."""
# Doesn't really matter how many recipients, since we expect
@@ -444,18 +438,6 @@ class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase):
def test_failure_on_unhandled_smtp(self):
self._test_immediate_failure(SMTPAuthenticationError(403, "That password doesn't work!"))
def test_failure_on_ses_quota_exceeded(self):
self._test_immediate_failure(SESDailyQuotaExceededError(403, "You're done for the day!"))
def test_failure_on_ses_address_not_verified(self):
self._test_immediate_failure(SESAddressNotVerifiedError(403, "Who *are* you?"))
def test_failure_on_ses_identity_not_verified(self):
self._test_immediate_failure(SESIdentityNotVerifiedError(403, "May I please see an ID!"))
def test_failure_on_ses_domain_not_confirmed(self):
self._test_immediate_failure(SESDomainNotConfirmedError(403, "You're out of bounds!"))
def test_bulk_emails_with_unicode_course_image_name(self):
# Test bulk email with unicode characters in course image name
course_image = '在淡水測試.jpg'