Commit Graph

12 Commits

Author SHA1 Message Date
Ben Thorner
e3e067c795 Remove redundant @statsd timing decorators
These are superseded by timing task execution generically in the
NotifyTask superclass [1]. Note that we need to wait until we've
gathered enough data under the new metrics before removing these.

[1]: https://github.com/alphagov/notifications-api/pull/3201#pullrequestreview-633549376
2021-04-12 15:19:18 +01:00
Ben Thorner
a91fde2fda Run auto-correct on app/ and tests/ 2021-03-12 11:45:45 +00:00
David McDonald
43f1f48093 Add notification ID to SES bounce reason
At the moment we log everytime we get a bounce from SES, however we
don't link it to a particular notification so it's hard to know for what
sub reason a notifcation did not deliver by looking at the logs.

This commit changes this by now looking the bounce reason after we have
found the notification ID and including them together. So if you know
search for a notification ID in Kibana, you will see full logs for why
it failed to deliver.
2020-11-20 14:10:13 +00:00
David McDonald
7bc02ac26e Add better logging to understand callback delivery cases
In particular, we want to know how often callbacks arrive before the
notification being persisted
2020-06-04 16:00:43 +01:00
Rebecca Law
5d0830d7f7 Rename function for clarity 2020-03-27 15:48:54 +00:00
Rebecca Law
46b72a6bbb Refactor process_ses_receipt
It is possible a service has data rention that is smaller than the time it takes to get a delivery receipt.
This PR refactors process_ses_receipt to update NotificationHistory if the Notifcation has already been purged.
2020-03-27 14:12:39 +00:00
Alexey Bezhan
6f5822ae5b Downgrade log level for missing notifications in SES receipt
The timestamps available in the SES receipt don't always correspond
to the time the notification has been sent. We've seen callbacks with
a current timestamp in both 'mail' and 'bounce' objects that referenced
a notification sent a week ago, which means we can't rely on it to skip
archived notifications.

One possible approach would be to look up the notification reference in
the notification_history table, but this goes against our plans to stop
relying on it in the future.

This changes the SES receipts logic to retry missing notifications once
(if the callback timestamp is within the last 5 minutes the task will
retry after a 5 minute delay) to capture callbacks arriving before the
notification reference has been persisted to the DB. Otherwise, we log
the missing notification as a warning instead of error.
2019-03-06 11:35:32 +00:00
Alexey Bezhan
c2e15d4ee2 Allow retry exception to propagate from ses callback task
Celery `self.retry` raises an exception to communicate that the task
needs to be retried. Since our ses task is wrapped in a catch-all
except block it logs that exception as an error before retrying.

Handling Retry class separately allows us to raise it without logging
the traceback.
2019-02-25 13:25:50 +00:00
Alexey Bezhan
2932b44eb8 Add retries for SES callbacks for recent notifications
We've seen errors caused by what we suspect is a race condition when
SES callback processing tries to look up the notification before the
sender worker has saved notification reference from the SES POST
response to the database.

This adds a retry for SES callback task if the notification was not
found and the message is less than 10 minutes old and removes the
error log message for notifications older than 3 days (since they
might no longer exist in the notifications table and would've been
marked as failure by then either way).

In order to be able to call retry and silence the error log based on
notification time this change inlines `process_ses_response` and
`update_notification_by_reference` functions into the celery task.
It also removes a lot of defensive error-handling that doesn't appear
to have been triggered in the last few months (for things like missing
keys in SES callback data).
2019-02-25 10:36:37 +00:00
Leo Hemsted
267c4fc07b bump requirements, fix pyflake8 things, unpin botocore/awscli 2018-11-07 13:39:08 +00:00
Richard Chapman
d855b4e4ec Removed statsd from the api and use the statsd in the utils library.
The statsd code was added to the utils library a while ago, uses the
statsd from the util library and therefore consolidates the code into
once place.
2018-02-06 09:52:15 +00:00
venusbb
489f43a2c9 rename callback_tasks.py to process_ses_receipts.py
create service_callback_tasks.py for tasks to send delivery statuses to services
2017-12-01 16:15:21 +00:00