scheduled_messages: Add reasonable failure handling.

Previously, it seemed possible for the scheduled messages API to try
to send infinite copies of a message if we had the very poor luck of a
persistent failure happening after a message was sent.

The failure_message field supports being able to display what happened
in the scheduled messages modal, though that's not exposed to the API
yet.

zerver/actions/scheduled_messages.py

@@ -10,7 +10,7 @@ from django.utils.translation import gettext as _
 from zerver.actions.message_send import build_message_send_dict, check_message, do_send_messages
 from zerver.actions.uploads import check_attachment_reference_change, do_claim_attachments
 from zerver.lib.addressee import Addressee
-from zerver.lib.exceptions import JsonableError
+from zerver.lib.exceptions import JsonableError, RealmDeactivatedError, UserDeactivatedError
 from zerver.lib.message import SendMessageRequest, render_markdown
 from zerver.lib.scheduled_messages import access_scheduled_message
 from zerver.models import (
@@ -206,6 +206,16 @@ def construct_send_request(scheduled_message: ScheduledMessage) -> SendMessageRe
 
 def send_scheduled_message(scheduled_message: ScheduledMessage) -> None:
     assert not scheduled_message.delivered
+    assert not scheduled_message.failed
+
+    # Repeat the checks from validate_account_and_subdomain, in case
+    # the state changed since the message as scheduled.
+    if scheduled_message.realm.deactivated:
+        raise RealmDeactivatedError
+
+    if not scheduled_message.sender.is_active:
+        raise UserDeactivatedError
+
     message_send_request = construct_send_request(scheduled_message)
     do_send_messages([message_send_request])
     scheduled_message.delivered = True

zerver/lib/scheduled_messages.py

@@ -24,7 +24,11 @@ def get_undelivered_scheduled_messages(
     user_profile: UserProfile,
 ) -> List[Union[APIScheduledDirectMessageDict, APIScheduledStreamMessageDict]]:
     scheduled_messages = ScheduledMessage.objects.filter(
-        sender=user_profile, delivered=False, delivery_type=ScheduledMessage.SEND_LATER
+        sender=user_profile,
+        # Notably, we don't require failed=False, since we will want
+        # to display those to users.
+        delivered=False,
+        delivery_type=ScheduledMessage.SEND_LATER,
     ).order_by("scheduled_timestamp")
     scheduled_message_dicts: List[
         Union[APIScheduledDirectMessageDict, APIScheduledStreamMessageDict]

zerver/management/commands/deliver_scheduled_messages.py

@@ -7,8 +7,10 @@ from django.conf import settings
 from django.core.management.base import BaseCommand
 from django.db import transaction
 from django.utils.timezone import now as timezone_now
+from django.utils.translation import gettext as _
 
 from zerver.actions.scheduled_messages import send_scheduled_message
+from zerver.lib.exceptions import JsonableError
 from zerver.lib.logging_util import log_to_file
 from zerver.models import ScheduledMessage
 
@@ -32,7 +34,9 @@ Usage: ./manage.py deliver_scheduled_messages
                 with transaction.atomic():
                     scheduled_message = (
                         ScheduledMessage.objects.filter(
-                            scheduled_timestamp__lte=timezone_now(), delivered=False
+                            scheduled_timestamp__lte=timezone_now(),
+                            delivered=False,
+                            failed=False,
                         )
                         .select_for_update()
                         .first()
@@ -44,7 +48,29 @@ Usage: ./manage.py deliver_scheduled_messages
                             scheduled_message.scheduled_timestamp,
                             scheduled_message.sender_id,
                         )
-                        send_scheduled_message(scheduled_message)
+                        try:
+                            send_scheduled_message(scheduled_message)
+                        except JsonableError as e:
+                            scheduled_message.failed = True
+                            scheduled_message.failure_message = e.msg
+                            scheduled_message.save(update_fields=["failed", "failure_message"])
+                            logging.info(
+                                "Failed with message: %s", scheduled_message.failure_message
+                            )
+                        except Exception:
+                            # An unexpected failure; store as a generic 500 error.
+                            scheduled_message.refresh_from_db()
+                            was_delivered = scheduled_message.delivered
+                            scheduled_message.failed = True
+                            scheduled_message.failure_message = _("Internal server error")
+                            scheduled_message.save(update_fields=["failed", "failure_message"])
+
+                            logging.exception(
+                                "Unexpected error sending scheduled message %s (sent: %s)",
+                                scheduled_message.id,
+                                was_delivered,
+                                stack_info=True,
+                            )
                         continue
 
                 # If there's no overdue scheduled messages, go to sleep until the next minute.

zerver/migrations/0448_scheduledmessage_new_fields.py

@@ -0,0 +1,22 @@
+# Generated by Django 4.2 on 2023-05-05 00:18
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("zerver", "0447_attachment_scheduled_messages_and_more"),
+    ]
+
+    operations = [
+        migrations.AddField(
+            model_name="scheduledmessage",
+            name="failed",
+            field=models.BooleanField(default=False),
+        ),
+        migrations.AddField(
+            model_name="scheduledmessage",
+            name="failure_message",
+            field=models.TextField(null=True),
+        ),
+    ]

zerver/models.py

@@ -4327,6 +4327,11 @@ class ScheduledMessage(models.Model):
     delivered = models.BooleanField(default=False)
     has_attachment = models.BooleanField(default=False, db_index=True)
 
+    # Metadata for messages that failed to send when their scheduled
+    # moment arrived.
+    failed = models.BooleanField(default=False)
+    failure_message = models.TextField(null=True)
+
     SEND_LATER = 1
     REMIND = 2