diff --git a/app/celery/letters_pdf_tasks.py b/app/celery/letters_pdf_tasks.py index cd555f0e6..43cd55fe4 100644 --- a/app/celery/letters_pdf_tasks.py +++ b/app/celery/letters_pdf_tasks.py @@ -26,6 +26,7 @@ from app.dao.templates_dao import dao_get_template_by_id from app.errors import VirusScanError from app.exceptions import NotificationTechnicalFailureException from app.letters.utils import ( + LetterPDFNotFound, ScanErrorType, find_letter_pdf_filename, generate_letter_pdf_filename, @@ -243,7 +244,7 @@ def get_key_and_size_of_letters_to_be_sent_to_print(print_run_deadline, postage) "Size": letter_head['ContentLength'], "ServiceId": str(letter.service_id) } - except BotoClientError as e: + except (BotoClientError, LetterPDFNotFound) as e: current_app.logger.exception( f"Error getting letter from bucket for notification: {letter.id} with reference: {letter.reference}", e) diff --git a/app/dao/notifications_dao.py b/app/dao/notifications_dao.py index 2a18a0b37..ce45cc46a 100644 --- a/app/dao/notifications_dao.py +++ b/app/dao/notifications_dao.py @@ -27,7 +27,7 @@ from app.clients.sms.firetext import ( get_message_status_and_reason_from_firetext_code, ) from app.dao.dao_utils import transactional -from app.letters.utils import find_letter_pdf_filename +from app.letters.utils import LetterPDFNotFound, find_letter_pdf_filename from app.models import ( EMAIL_TYPE, KEY_TYPE_NORMAL, @@ -453,7 +453,13 @@ def _delete_letters_from_s3( Notification.status.in_(NOTIFICATION_STATUS_TYPES_COMPLETED) ).limit(query_limit).all() for letter in letters_to_delete_from_s3: - prefix = find_letter_pdf_filename(letter) + try: + prefix = find_letter_pdf_filename(letter) + except LetterPDFNotFound: + current_app.logger.exception( + "Could not delete S3 object for letter: {}".format(letter.id)) + continue + s3_objects = get_s3_bucket_objects(bucket_name=bucket_name, subfolder=prefix) for s3_object in s3_objects: try: diff --git a/app/letters/utils.py b/app/letters/utils.py index 78503d4ba..623cac84a 100644 --- a/app/letters/utils.py +++ b/app/letters/utils.py @@ -37,6 +37,10 @@ def get_folder_name(created_at): return '{}/'.format(print_datetime.date()) +class LetterPDFNotFound(Exception): + pass + + def find_letter_pdf_filename(notification): """ Retrieve the filename of a letter from s3 by searching for it based on a prefix. @@ -47,7 +51,10 @@ def find_letter_pdf_filename(notification): s3 = boto3.resource('s3') bucket = s3.Bucket(bucket_name) - item = next(x for x in bucket.objects.filter(Prefix=prefix)) + try: + item = next(x for x in bucket.objects.filter(Prefix=prefix)) + except StopIteration: + raise LetterPDFNotFound(f'File not found in bucket {bucket_name} with prefix {prefix}', ) return item.key diff --git a/tests/app/letters/test_letter_utils.py b/tests/app/letters/test_letter_utils.py index 2ae4d1031..4d99f7c0a 100644 --- a/tests/app/letters/test_letter_utils.py +++ b/tests/app/letters/test_letter_utils.py @@ -8,6 +8,7 @@ from freezegun import freeze_time from moto import mock_s3 from app.letters.utils import ( + LetterPDFNotFound, ScanErrorType, find_letter_pdf_filename, generate_letter_pdf_filename, @@ -62,6 +63,19 @@ def test_find_letter_pdf_filename_returns_filename(sample_notification): assert find_letter_pdf_filename(sample_notification) == f'{prefix}-and-then-some' +@mock_s3 +def test_find_letter_pdf_filename_raises_if_not_found(sample_notification): + bucket_name = current_app.config['LETTERS_PDF_BUCKET_NAME'] + s3 = boto3.client('s3', region_name='eu-west-1') + s3.create_bucket( + Bucket=bucket_name, + CreateBucketConfiguration={'LocationConstraint': 'eu-west-1'} + ) + + with pytest.raises(LetterPDFNotFound): + find_letter_pdf_filename(sample_notification) + + @pytest.mark.parametrize('created_at,folder', [ (datetime(2017, 1, 1, 17, 29), '2017-01-01'), (datetime(2017, 1, 1, 17, 31), '2017-01-02'),