feat(plugins): Add ApiClient to Twilio (#16987)

* feat(plugins): Add ApiClient to Twilio

src/sentry_plugins/twilio/client.py

@@ -0,0 +1,26 @@
+from __future__ import absolute_import
+
+from sentry_plugins.client import ApiClient
+
+
+class TwilioApiClient(ApiClient):
+    plugin_name = "twilio"
+    allow_redirects = False
+    twilio_messages_endpoint = "https://api.twilio.com/2010-04-01/Accounts/{0}/Messages.json"
+
+    def __init__(self, account_sid, auth_token, sms_from, sms_to):
+        self.account_sid = account_sid
+        self.auth_token = auth_token
+        self.sms_from = sms_from
+        self.sms_to = sms_to
+        super(TwilioApiClient, self).__init__()
+
+    def basic_auth(self, user, password):
+        return "Basic " + (user + ":" + password).encode("base64").replace("\n", "")
+
+    def request(self, data):
+        endpoint = self.twilio_messages_endpoint.format(self.account_sid)
+        headers = {"Authorization": self.basic_auth(self.account_sid, self.auth_token)}
+        # Twilio doesn't accept the json headers, so set this to False
+        # https://www.twilio.com/docs/usage/your-request-to-twilio#post
+        return self._request(path=endpoint, method="post", data=data, headers=headers, json=False)

src/sentry_plugins/twilio/plugin.py

@@ -6,16 +6,16 @@ import phonenumbers
 from django import forms
 from django.utils.translation import ugettext_lazy as _
 
-from sentry import http
 from sentry.plugins.bases.notify import NotificationPlugin
 
+from .client import TwilioApiClient
+from sentry_plugins.base import CorePluginMixin
+
 import sentry
 
 DEFAULT_REGION = "US"
 MAX_SMS_LENGTH = 160
 
-twilio_sms_endpoint = "https://api.twilio.com/2010-04-01/Accounts/{0}/SMS/Messages.json"
-
 
 def validate_phone(phone):
     try:
@@ -36,10 +36,6 @@ def clean_phone(phone):
     )
 
 
-def basic_auth(user, password):
-    return "Basic " + (user + ":" + password).encode("base64").replace("\n", "")
-
-
 # XXX: can likely remove the dedupe here after notify_users has test coverage;
 #      in theory only cleaned data would make it to the plugin via the form,
 #      and cleaned numbers are deduped already.
@@ -92,7 +88,7 @@ class TwilioConfigurationForm(forms.Form):
         return self.cleaned_data
 
 
-class TwilioPlugin(NotificationPlugin):
+class TwilioPlugin(CorePluginMixin, NotificationPlugin):
     author = "Matt Robenolt"
     author_url = "https://github.com/mattrobenolt"
     version = sentry.VERSION
@@ -129,7 +125,18 @@ class TwilioPlugin(NotificationPlugin):
         # This doesn't depend on email permission... stuff.
         return True
 
+    def error_message_from_json(self, data):
+        code = data.get("code")
+        message = data.get("message")
+        more_info = data.get("more_info")
+        error_message = "%s - %s %s" % (code, message, more_info)
+        if message:
+            return error_message
+        return None
+
     def notify_users(self, group, event, **kwargs):
+        if not self.is_configured(group.project):
+            return
         project = group.project
 
         body = "Sentry [{0}] {1}: {2}".format(
@@ -139,38 +146,31 @@ class TwilioPlugin(NotificationPlugin):
         )
         body = body[:MAX_SMS_LENGTH]
 
-        account_sid = self.get_option("account_sid", project)
-        auth_token = self.get_option("auth_token", project)
-        sms_from = clean_phone(self.get_option("sms_from", project))
-        endpoint = twilio_sms_endpoint.format(account_sid)
-
-        sms_to = self.get_option("sms_to", project)
-        if not sms_to:
-            return
-        sms_to = split_sms_to(sms_to)
+        client = self.get_client(group.project)
 
-        headers = {"Authorization": basic_auth(account_sid, auth_token)}
+        payload = {"From": client.sms_from, "Body": body}
 
         errors = []
 
-        for phone in sms_to:
+        for phone in client.sms_to:
             if not phone:
                 continue
             try:
                 # TODO: Use API client with raise_error
                 phone = clean_phone(phone)
-                http.safe_urlopen(
-                    endpoint,
-                    method="POST",
-                    headers=headers,
-                    data={"From": sms_from, "To": phone, "Body": body},
-                ).raise_for_status()
+                payload = payload.copy()
+                payload["To"] = phone
+                client.request(payload)
             except Exception as e:
                 errors.append(e)
 
         if errors:
-            if len(errors) == 1:
-                raise errors[0]
+            self.raise_error(errors[0])
 
-            # TODO: multi-exception
-            raise Exception(errors)
+    def get_client(self, project):
+        account_sid = self.get_option("account_sid", project)
+        auth_token = self.get_option("auth_token", project)
+        sms_from = clean_phone(self.get_option("sms_from", project))
+        sms_to = self.get_option("sms_to", project)
+        sms_to = split_sms_to(sms_to)
+        return TwilioApiClient(account_sid, auth_token, sms_from, sms_to)

tests/sentry_plugins/twilio/test_plugin.py

@@ -1,8 +1,13 @@
 from __future__ import absolute_import
 
+import responses
+
 from exam import fixture
+from sentry.models import Rule
+from sentry.plugins.base import Notification
 from sentry.testutils import TestCase, PluginTestCase
 from sentry_plugins.twilio.plugin import TwilioConfigurationForm, TwilioPlugin
+from six.moves.urllib.parse import parse_qs
 
 
 class TwilioConfigurationFormTest(TestCase):
@@ -49,8 +54,6 @@ class TwilioConfigurationFormTest(TestCase):
 
 
 class TwilioPluginTest(PluginTestCase):
-    # TODO: actually test the plugin
-
     @fixture
     def plugin(self):
         return TwilioPlugin()
@@ -62,7 +65,40 @@ class TwilioPluginTest(PluginTestCase):
         self.assertPluginInstalled("twilio", self.plugin)
 
     def test_is_configured(self):
-        assert self.plugin.is_configured(self.project) is False
         for o in ("account_sid", "auth_token", "sms_from", "sms_to"):
+            assert self.plugin.is_configured(self.project) is False
             self.plugin.set_option(o, "foo", self.project)
         assert self.plugin.is_configured(self.project) is True
+
+    @responses.activate
+    def test_simple_notification(self):
+        responses.add("POST", "https://api.twilio.com/2010-04-01/Accounts/abcdef/Messages.json")
+        self.plugin.set_option("account_sid", "abcdef", self.project)
+        self.plugin.set_option("auth_token", "abcd", self.project)
+        self.plugin.set_option("sms_from", "4158675309", self.project)
+        self.plugin.set_option("sms_to", "4154444444", self.project)
+
+        event = self.store_event(
+            data={
+                "message": "Hello world",
+                "level": "warning",
+                "platform": "python",
+                "culprit": "foo.bar",
+            },
+            project_id=self.project.id,
+        )
+
+        rule = Rule.objects.create(project=self.project, label="my rule")
+
+        notification = Notification(event=event, rule=rule)
+
+        with self.options({"system.url-prefix": "http://example.com"}):
+            self.plugin.notify(notification)
+
+        request = responses.calls[0].request
+        payload = parse_qs(request.body)
+        assert payload == {
+            "To": ["+14154444444"],
+            "From": ["+14158675309"],
+            "Body": ["Sentry [%s] WARNING: Hello world" % self.project.slug.title()],
+        }