Handle status callbacks from Twilio, show SMS delivery failures in the Integrations page.

CHANGELOG.md

@@ -5,6 +5,7 @@ All notable changes to this project will be documented in this file.
 
 ## Improvements
 - Django 3.1
+- Handle status callbacks from Twilio, show SMS delivery failures in Integrations
 
 ## v1.16.0 - 2020-08-04
 

hc/api/models.py

@@ -482,16 +482,14 @@ class Channel(models.Model):
         n.error = "Sending"
         n.save()
 
-        if self.kind == "email":
-            error = self.transport.notify(check, n.bounce_url()) or ""
-        else:
-            error = self.transport.notify(check) or ""
+        # These are not database fields. It is just a convenient way to pass
+        # status_url to transport classes.
+        check.bounce_url = n.bounce_url()
+        check.status_url = n.status_url()
 
-        n.error = error
-        n.save()
-
-        self.last_error = error
-        self.save()
+        error = self.transport.notify(check) or ""
+        Notification.objects.filter(id=n.id).update(error=error)
+        Channel.objects.filter(id=self.id).update(last_error=error)
 
         return error
 
@@ -765,6 +763,10 @@ class Notification(models.Model):
     def bounce_url(self):
         return settings.SITE_ROOT + reverse("hc-api-bounce", args=[self.code])
 
+    def status_url(self):
+        path = reverse("hc-api-notification-status", args=[self.code])
+        return settings.SITE_ROOT + path
+
 
 class Flip(models.Model):
     owner = models.ForeignKey(Check, models.CASCADE)

hc/api/tests/test_notification_status.py

@@ -0,0 +1,91 @@
+from datetime import timedelta
+
+from hc.api.models import Channel, Check, Notification
+from hc.test import BaseTestCase
+
+
+class NotificationStatusTestCase(BaseTestCase):
+    def setUp(self):
+        super(NotificationStatusTestCase, self).setUp()
+
+        self.check = Check(project=self.project, status="up")
+        self.check.save()
+
+        self.channel = Channel(project=self.project, kind="email")
+        self.channel.value = "alice@example.org"
+        self.channel.email_verified = True
+        self.channel.save()
+
+        self.n = Notification(owner=self.check, channel=self.channel)
+        self.n.save()
+
+        self.url = "/api/v1/notifications/%s/status" % self.n.code
+
+    def test_it_handles_twilio_failed_status(self):
+        r = self.client.post(self.url, {"MessageStatus": "failed"})
+        self.assertEqual(r.status_code, 200)
+
+        self.n.refresh_from_db()
+        self.assertEqual(self.n.error, "Delivery failed (status=failed).")
+
+        self.channel.refresh_from_db()
+        self.assertEqual(self.channel.last_error, "Delivery failed (status=failed).")
+        self.assertTrue(self.channel.email_verified)
+
+    def test_it_handles_twilio_undelivered_status(self):
+        r = self.client.post(self.url, {"MessageStatus": "undelivered"})
+        self.assertEqual(r.status_code, 200)
+
+        self.n.refresh_from_db()
+        self.assertEqual(self.n.error, "Delivery failed (status=undelivered).")
+
+        self.channel.refresh_from_db()
+        self.assertIn("status=undelivered", self.channel.last_error)
+
+    def test_it_handles_twilio_delivered_status(self):
+        r = self.client.post(self.url, {"MessageStatus": "delivered"})
+        self.assertEqual(r.status_code, 200)
+
+        self.n.refresh_from_db()
+        self.assertEqual(self.n.error, "")
+
+        self.channel.refresh_from_db()
+        self.assertEqual(self.channel.last_error, "")
+
+    def test_it_checks_ttl(self):
+        self.n.created = self.n.created - timedelta(minutes=61)
+        self.n.save()
+
+        r = self.client.post(self.url, {"MessageStatus": "failed"})
+        self.assertEqual(r.status_code, 403)
+
+    def test_it_handles_missing_notification(self):
+        fake_code = "07c2f548-9850-4b27-af5d-6c9dc157ec02"
+        url = f"/api/v1/notifications/{fake_code}/status"
+        r = self.client.post(url, {"MessageStatus": "failed"})
+        self.assertEqual(r.status_code, 404)
+
+    def test_it_requires_post(self):
+        r = self.client.get(self.url)
+        self.assertEqual(r.status_code, 405)
+
+    def test_it_handles_error_key(self):
+        r = self.client.post(self.url, {"error": "Something went wrong."})
+        self.assertEqual(r.status_code, 200)
+
+        self.n.refresh_from_db()
+        self.assertEqual(self.n.error, "Something went wrong.")
+
+        self.channel.refresh_from_db()
+        self.assertEqual(self.channel.last_error, "Something went wrong.")
+        self.assertTrue(self.channel.email_verified)
+
+    def test_it_handles_mark_not_verified_key(self):
+        payload = {"error": "Received complaint.", "mark_not_verified": "1"}
+
+        r = self.client.post(self.url, payload)
+        self.assertEqual(r.status_code, 200)
+
+        self.channel.refresh_from_db()
+        self.assertEqual(self.channel.last_error, "Received complaint.")
+        self.assertFalse(self.channel.email_verified)

hc/api/tests/test_notify.py

@@ -60,6 +60,8 @@ class NotifyTestCase(BaseTestCase):
 
         n = Notification.objects.get()
         self.assertEqual(n.error, "Connection timed out")
+
+        self.channel.refresh_from_db()
         self.assertEqual(self.channel.last_error, "Connection timed out")
 
     @patch("hc.api.transports.requests.request", side_effect=ConnectionError)
@@ -313,6 +315,7 @@ class NotifyTestCase(BaseTestCase):
         email = mail.outbox[0]
         self.assertEqual(email.to[0], "alice@example.org")
         self.assertTrue("X-Bounce-Url" in email.extra_headers)
+        self.assertTrue("X-Status-Url" in email.extra_headers)
         self.assertTrue("List-Unsubscribe" in email.extra_headers)
         self.assertTrue("List-Unsubscribe-Post" in email.extra_headers)
 
@@ -638,13 +641,17 @@ class NotifyTestCase(BaseTestCase):
         mock_post.return_value.status_code = 200
 
         self.channel.notify(self.check)
-        self.assertEqual(Notification.objects.count(), 1)
+
+        n = Notification.objects.get()
 
         args, kwargs = mock_post.call_args
         payload = kwargs["data"]
         self.assertEqual(payload["To"], "+1234567890")
         self.assertFalse("\xa0" in payload["Body"])
 
+        callback_path = f"/api/v1/notifications/{n.code}/status"
+        self.assertTrue(payload["StatusCallback"].endswith(callback_path))
+
         # sent SMS counter should go up
         self.profile.refresh_from_db()
         self.assertEqual(self.profile.sms_sent, 1)

hc/api/transports.py

@@ -55,14 +55,15 @@ class Transport(object):
 
 
 class Email(Transport):
-    def notify(self, check, bounce_url):
+    def notify(self, check):
         if not self.channel.email_verified:
             return "Email not verified"
 
         unsub_link = self.channel.get_unsub_link()
 
         headers = {
-            "X-Bounce-Url": bounce_url,
+            "X-Bounce-Url": check.bounce_url,
+            "X-Status-Url": check.status_url,
             "List-Unsubscribe": "<%s>" % unsub_link,
             "List-Unsubscribe-Post": "List-Unsubscribe=One-Click",
         }
@@ -473,6 +474,7 @@ class Sms(HttpTransport):
             "From": settings.TWILIO_FROM,
             "To": self.channel.phone_number,
             "Body": text,
+            "StatusCallback": check.status_url,
         }
 
         return self.post(url, data=data, auth=auth)

hc/api/urls.py

@@ -37,6 +37,11 @@ urlpatterns = [
     path("api/v1/checks/<sha1:unique_key>", views.get_check_by_unique_key),
     path("api/v1/checks/<uuid:code>/pause", views.pause, name="hc-api-pause"),
     path("api/v1/notifications/<uuid:code>/bounce", views.bounce, name="hc-api-bounce"),
+    path(
+        "api/v1/notifications/<uuid:code>/status",
+        views.notification_status,
+        name="hc-api-notification-status",
+    ),
     path("api/v1/checks/<uuid:code>/pings/", views.pings, name="hc-api-pings"),
     path("api/v1/checks/<uuid:code>/flips/", views.flips_by_uuid, name="hc-api-flips"),
     path("api/v1/checks/<sha1:unique_key>/flips/", views.flips_by_unique_key),

hc/api/views.py

@@ -415,6 +415,42 @@ def bounce(request, code):
     return HttpResponse()
 
 
+@csrf_exempt
+@require_POST
+def notification_status(request, code):
+    """ Handle notification delivery status callbacks. """
+
+    notification = get_object_or_404(Notification, code=code)
+
+    # If webhook is more than 1 hour late, don't accept it:
+    td = timezone.now() - notification.created
+    if td.total_seconds() > 3600:
+        return HttpResponseForbidden()
+
+    error, mark_not_verified = None, False
+
+    # Look for "error" and "unsub" keys:
+    if request.POST.get("error"):
+        error = request.POST["error"][:200]
+        mark_not_verified = request.POST.get("mark_not_verified")
+
+    # Handle "failed" and "undelivered" callbacks from Twilio
+    if request.POST.get("MessageStatus") in ("failed", "undelivered"):
+        status = request.POST["MessageStatus"]
+        error = f"Delivery failed (status={status})."
+
+    if error:
+        notification.error = error
+        notification.save(update_fields=["error"])
+
+        channel_q = Channel.objects.filter(id=notification.channel_id)
+        channel_q.update(last_error=error)
+        if mark_not_verified:
+            channel_q.update(email_verified=False)
+
+    return HttpResponse()
+
+
 def metrics(request):
     if not settings.METRICS_KEY:
         return HttpResponseForbidden()