From 156f4dbeeae435ff2fac07bb47bd57662e114100 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C4=93teris=20Caune?= <cuu508@gmail.com>
Date: Sun, 31 Jan 2016 19:30:02 +0200
Subject: [PATCH] More tests for notifications.

---
 hc/api/models.py            | 11 ++++++-----
 hc/api/tests/test_notify.py | 25 ++++++++++++++++++++-----
 hc/api/transports.py        | 16 ++++++++++++----
 3 files changed, 38 insertions(+), 14 deletions(-)

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):