darkhelm/notifications-api
1
0
Fork 0
mirror of https://github.com/GSA/notifications-api.git synced 2026-09-10 01:56:03 -04:00
Code Issues Packages Projects Releases Wiki Activity

Improve and clarify large task error handling

Browse Source 
Previously we changed this as an experiment [1] and forgot to check
if the new exception was appropriate. It is: testing with a large
task invocation ('process_job.apply_async(["a" * 200000])') gives...

Before (Celery 3, large-ish task):

    boto.exception.SQSError: SQSError: 400 Bad Request
    <?xml version="1.0"?><ErrorResponse xmlns="http://queue.amazonaws.com/doc/2012-11-05/"><Error><Type>Sender</Type><Code>InvalidParameterValue</Code><Message>One or more parameters are invalid. Reason: Message must be shorter than 262144 bytes.</Message><Detail/></Error><RequestId>96162552-cd96-5a14-b3a5-7f503300a662</RequestId></ErrorResponse>

Before (Celery 3, very large task):

    <hangs forever>

After (Celery 5, large-ish task):

    botocore.exceptions.ClientError: An error occurred (InvalidParameterValue) when calling the SendMessage operation: One or more parameters are invalid. Reason: Message must be shorter than 262144 bytes.

After (Celery 5, very large task):

    botocore.parsers.ResponseParserError: Unable to parse response (syntax error: line 1, column 0), invalid XML received. Further retries may succeed:
    b'HTTP content length exceeded 1662976 bytes.'
This commit is contained in:
Ben Thorner
2021-11-08 10:38:53 +00:00
parent 98b6c1d67d
commit 81671e7eb2
2 changed files with 14 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
app/v2/notifications/post_notifications.py
View File

@@ -232,9 +232,10 @@ def process_sms_or_email_notification(
reply_to_text=reply_to_text reply_to_text=reply_to_text
) )
return resp return resp
except botocore.exceptions.ClientError: except (botocore.exceptions.ClientError, botocore.parsers.ResponseParserError):
# if SQS cannot put the task on the queue, it's probably because the notification body was too long and it # If SQS cannot put the task on the queue, it's probably because the notification body was too long and it
# went over SQS's 256kb message limit. If so, we # went over SQS's 256kb message limit. If the body is very large, it may exceed the HTTP max content length;
# the exception we get here isn't handled correctly by botocore - we get a ResponseParserError instead.
current_app.logger.info( current_app.logger.info(
f'Notification {notification_id} failed to save to high volume queue. Using normal flow instead' f'Notification {notification_id} failed to save to high volume queue. Using normal flow instead'
) )

13
tests/app/v2/notifications/test_post_notifications.py
View File

@@ -1067,14 +1067,21 @@ def test_post_notifications_saves_email_or_sms_to_queue(client, notify_db_sessio
assert not mock_send_task.called assert not mock_send_task.called
assert len(Notification.query.all()) == 0 assert len(Notification.query.all()) == 0
@pytest.mark.parametrize("exception", [
botocore.exceptions.ClientError({'some': 'json'}, 'some opname'),
botocore.parsers.ResponseParserError('exceeded max HTTP body length'),
])
@pytest.mark.parametrize("notification_type", ("email", "sms")) @pytest.mark.parametrize("notification_type", ("email", "sms"))
def test_post_notifications_saves_email_or_sms_normally_if_saving_to_queue_fails( def test_post_notifications_saves_email_or_sms_normally_if_saving_to_queue_fails(
client, notify_db_session, mocker, notification_type client,
notify_db_session,
mocker,
notification_type,
exception
): ):
save_task = mocker.patch( save_task = mocker.patch(
f"app.celery.tasks.save_api_{notification_type}.apply_async", f"app.celery.tasks.save_api_{notification_type}.apply_async",
side_effect=botocore.exceptions.ClientError({'some': 'json'}, 'some opname') side_effect=exception,
) )
mock_send_task = mocker.patch(f'app.celery.provider_tasks.deliver_{notification_type}.apply_async') mock_send_task = mocker.patch(f'app.celery.provider_tasks.deliver_{notification_type}.apply_async')