diff --git a/CHANGELOG.md b/CHANGELOG.md index feac5dfe..125a54d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to this project will be documented in this file. - Reduce the number of SQL queries used in the "Get Checks" API call - Add support for script's exit status in ping URLs (#429) - Improve phone number sanitization: remove spaces and hyphens +- Change the "Test Integration" behavior for webhooks: don't retry failed requests ## v1.17.0 - 2020-10-14 diff --git a/hc/api/models.py b/hc/api/models.py index 52d4a562..bc2cda39 100644 --- a/hc/api/models.py +++ b/hc/api/models.py @@ -489,7 +489,8 @@ class Channel(models.Model): n.save() # These are not database fields. It is just a convenient way to pass - # status_url to transport classes. + # status_url and the is_test flag to transport classes. + check.is_test = is_test check.status_url = n.status_url() error = self.transport.notify(check) or "" diff --git a/hc/api/tests/test_notify.py b/hc/api/tests/test_notify.py index deae430c..637966da 100644 --- a/hc/api/tests/test_notify.py +++ b/hc/api/tests/test_notify.py @@ -58,6 +58,9 @@ class NotifyTestCase(BaseTestCase): self._setup_data("webhook", json.dumps(definition)) self.channel.notify(self.check) + # The transport should have retried 3 times + self.assertEqual(mock_get.call_count, 3) + n = Notification.objects.get() self.assertEqual(n.error, "Connection timed out") @@ -75,6 +78,9 @@ class NotifyTestCase(BaseTestCase): self._setup_data("webhook", json.dumps(definition)) self.channel.notify(self.check) + # The transport should have retried 3 times + self.assertEqual(mock_get.call_count, 3) + n = Notification.objects.get() self.assertEqual(n.error, "Connection failed") @@ -92,9 +98,30 @@ class NotifyTestCase(BaseTestCase): self.channel.notify(self.check) + # The transport should have retried 3 times + self.assertEqual(mock_get.call_count, 3) + n = Notification.objects.get() self.assertEqual(n.error, "Received status code 500") + @patch("hc.api.transports.requests.request", side_effect=Timeout) + def test_webhooks_dont_retry_when_sending_test_notifications(self, mock_get): + definition = { + "method_down": "GET", + "url_down": "http://example", + "body_down": "", + "headers_down": {}, + } + + self._setup_data("webhook", json.dumps(definition)) + self.channel.notify(self.check, is_test=True) + + # is_test flag is set, the transport should not retry: + self.assertEqual(mock_get.call_count, 1) + + n = Notification.objects.get() + self.assertEqual(n.error, "Connection timed out") + @patch("hc.api.transports.requests.request") def test_webhooks_support_variables(self, mock_get): definition = { diff --git a/hc/api/transports.py b/hc/api/transports.py index ba0f1006..4da12981 100644 --- a/hc/api/transports.py +++ b/hc/api/transports.py @@ -169,9 +169,8 @@ class HttpTransport(Transport): return "Connection failed" @classmethod - def get(cls, url, **kwargs): - # Make 3 attempts-- - for x in range(0, 3): + def get(cls, url, num_tries=3, **kwargs): + for x in range(0, num_tries): error = cls._request("get", url, **kwargs) if error is None: break @@ -179,9 +178,8 @@ class HttpTransport(Transport): return error @classmethod - def post(cls, url, **kwargs): - # Make 3 attempts-- - for x in range(0, 3): + def post(cls, url, num_tries=3, **kwargs): + for x in range(0, num_tries): error = cls._request("post", url, **kwargs) if error is None: break @@ -189,9 +187,8 @@ class HttpTransport(Transport): return error @classmethod - def put(cls, url, **kwargs): - # Make 3 attempts-- - for x in range(0, 3): + def put(cls, url, num_tries=3, **kwargs): + for x in range(0, num_tries): error = cls._request("put", url, **kwargs) if error is None: break @@ -240,14 +237,19 @@ class Webhook(HttpTransport): body = spec["body"] if body: - body = self.prepare(body, check) + body = self.prepare(body, check).encode() + + num_tries = 3 + if getattr(check, "is_test"): + # When sending a test notification, don't retry on failures. + num_tries = 1 if spec["method"] == "GET": - return self.get(url, headers=headers) + return self.get(url, num_tries=num_tries, headers=headers) elif spec["method"] == "POST": - return self.post(url, data=body.encode(), headers=headers) + return self.post(url, num_tries=num_tries, data=body, headers=headers) elif spec["method"] == "PUT": - return self.put(url, data=body.encode(), headers=headers) + return self.put(url, num_tries=num_tries, data=body, headers=headers) class Slack(HttpTransport):