send_email: Delete ScheduledEmail objects with no recipients.

9d97af6ebb addressed the one major source of inconsistent data which
would be solved by simply re-attempting the ScheduledEmail row.  Every
other instance that we have seen since then has been a corrupt or
modified database in some way, which does not self-resolve.  This
results in an endless stream of emails to the administrator, and no
forward progress.

Drop this to a warning, and make it remove the offending row.  This
ensures we make forward progress.
This commit is contained in:
Alex Vandiver 2023-06-14 16:54:51 +00:00 committed by Tim Abbott
parent 47d0bb6b7d
commit 77c146b8b0
2 changed files with 15 additions and 6 deletions

View File

@ -477,10 +477,14 @@ def deliver_scheduled_emails(email: ScheduledEmail) -> None:
data = orjson.loads(email.data) data = orjson.loads(email.data)
user_ids = list(email.users.values_list("id", flat=True)) user_ids = list(email.users.values_list("id", flat=True))
if not user_ids and not email.address: if not user_ids and not email.address:
# This state doesn't make sense, so something must be mutating, # This state doesn't make sense, so something must have mutated the object
# or in the process of deleting, the object. We assume it will bring logger.warning(
# things to a correct state, and we just do nothing except logging this event. "ScheduledEmail %s at %s had empty users and address attributes: %r",
logger.error("ScheduledEmail id %s has empty users and address attributes.", email.id) email.id,
email.scheduled_timestamp,
data,
)
email.delete()
return return
if user_ids: if user_ids:

View File

@ -1698,16 +1698,21 @@ class ActivateTest(ZulipTestCase):
assert email is not None assert email is not None
email.users.remove(*to_user_ids) email.users.remove(*to_user_ids)
email_id = email.id
scheduled_at = email.scheduled_timestamp
with self.assertLogs("zulip.send_email", level="INFO") as info_log: with self.assertLogs("zulip.send_email", level="INFO") as info_log:
deliver_scheduled_emails(email) deliver_scheduled_emails(email)
from django.core.mail import outbox from django.core.mail import outbox
self.assert_length(outbox, 0) self.assert_length(outbox, 0)
self.assertEqual(ScheduledEmail.objects.count(), 1) self.assertEqual(ScheduledEmail.objects.count(), 0)
self.assertEqual( self.assertEqual(
info_log.output, info_log.output,
[ [
f"ERROR:zulip.send_email:ScheduledEmail id {email.id} has empty users and address attributes." f"WARNING:zulip.send_email:ScheduledEmail {email_id} at {scheduled_at} "
"had empty users and address attributes: "
"{'template_prefix': 'zerver/emails/followup_day1', 'from_name': None, "
"'from_address': None, 'language': None, 'context': {}}"
], ],
) )