diff --git a/hc/api/models.py b/hc/api/models.py index 7bf8d944..4f808fc4 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -144,13 +144,14 @@ class Channel(models.Model): # Make 3 attempts-- for x in range(0, 3): error = self.transport.notify(check) or "" - if error == "": + if error in ("", "no-op"): break # Success! - n = Notification(owner=check, channel=self) - n.check_status = check.status - n.error = error - n.save() + if error != "no-op": + n = Notification(owner=check, channel=self) + n.check_status = check.status + n.error = error + n.save() def test(self): return self.transport().test() diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index 7865e669..121bd430 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -1,6 +1,6 @@ from django.core import mail from mock import patch -from requests.exceptions import ReadTimeout +from requests.exceptions import ConnectionError, Timeout from hc.api.models import Channel, Check, Notification from hc.test import BaseTestCase @@ -30,7 +30,7 @@ class NotifyTestCase(BaseTestCase): u"http://example", headers={"User-Agent": "healthchecks.io"}, timeout=5) - @patch("hc.api.transports.requests.get", side_effect=ReadTimeout) + @patch("hc.api.transports.requests.get", side_effect=Timeout) def test_webhooks_handle_timeouts(self, mock_get): self._setup_data("webhook", "http://example") self.channel.notify(self.check) @@ -38,15 +38,21 @@ class NotifyTestCase(BaseTestCase): n = Notification.objects.get() self.assertEqual(n.error, "Connection timed out") + @patch("hc.api.transports.requests.get", side_effect=ConnectionError) + def test_webhooks_handle_connection_errors(self, mock_get): + self._setup_data("webhook", "http://example") + self.channel.notify(self.check) + + n = Notification.objects.get() + self.assertEqual(n.error, "A connection to http://example failed") + @patch("hc.api.transports.requests.get") def test_webhooks_ignore_up_events(self, mock_get): self._setup_data("webhook", "http://example", status="up") self.channel.notify(self.check) self.assertFalse(mock_get.called) - - n = Notification.objects.get() - self.assertEqual(n.error, "") + self.assertEqual(Notification.objects.count(), 0) @patch("hc.api.transports.requests.get") def test_webhooks_handle_500(self, mock_get): @@ -113,6 +119,15 @@ class NotifyTestCase(BaseTestCase): n = Notification.objects.get() self.assertEqual(n.error, "Received status code 500") + @patch("hc.api.transports.requests.post", side_effect=Timeout) + def test_slack_handles_timeout(self, mock_post): + self._setup_data("slack", "123") + + self.channel.notify(self.check) + + n = Notification.objects.get() + self.assertEqual(n.error, "Connection timed out") + @patch("hc.api.transports.requests.post") def test_hipchat(self, mock_post): self._setup_data("hipchat", "123") diff --git a/hc/api/transports.py b/hc/api/transports.py index 2421a9ad..bf37099d 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -57,7 +57,7 @@ class Webhook(Transport): def notify(self, check): # Webhook integration only fires when check goes down. if check.status != "down": - return + return "no-op" # Webhook transport sends no arguments, so the # notify and test actions are the same @@ -72,14 +72,22 @@ class Webhook(Transport): except requests.exceptions.Timeout: # Well, we tried return "Connection timed out" + except requests.exceptions.ConnectionError: + return "A connection to %s failed" % self.channel.value class JsonTransport(Transport): def post(self, url, payload): headers = {"User-Agent": "healthchecks.io"} - r = requests.post(url, json=payload, timeout=5, headers=headers) - if r.status_code not in (200, 201): - return "Received status code %d" % r.status_code + try: + r = requests.post(url, json=payload, timeout=5, headers=headers) + if r.status_code not in (200, 201): + return "Received status code %d" % r.status_code + except requests.exceptions.Timeout: + # Well, we tried + return "Connection timed out" + except requests.exceptions.ConnectionError: + return "A connection to %s failed" % url class Slack(JsonTransport):