Fix AttributeError on Cloudflare cache invalidation (#5150)

CHANGELOG.txt

@@ -39,6 +39,7 @@ Changelog
  * Fix: User add/edit forms now support form widgets with JS/CSS media (Damian Grinwis)
  * Fix: Rich text processing now preserves non-breaking spaces instead of converting them to normal spaces (Wesley van Lee)
  * Fix: Prevent autocomplete dropdowns from appearing over date choosers on Chrome (Kevin Howbrook)
+ * Fix: Prevent crash when logging HTTP errors from Cloudflare (Kevin Howbrook)
 
 
 2.4 (19.12.2018)

docs/releases/2.5.rst

@@ -55,6 +55,7 @@ Bug fixes
  * User add/edit forms now support form widgets with JS/CSS media (Damian Grinwis)
  * Rich text processing now preserves non-breaking spaces instead of converting them to normal spaces (Wesley van Lee)
  * Prevent autocomplete dropdowns from appearing over date choosers on Chrome (Kevin Howbrook)
+ * Prevent crash when logging HTTP errors on Cloudflare cache purging (Kevin Howbrook)
 
 
 Upgrade considerations

wagtail/contrib/frontend_cache/backends.py

@@ -100,7 +100,7 @@ class CloudflareBackend(BaseBackend):
 
         except requests.exceptions.HTTPError as e:
             for url in urls:
-                logger.error("Couldn't purge '%s' from Cloudflare. HTTPError: %d %s", url, e.response.status_code, e.message)
+                logging.exception("Couldn't purge '%s' from Cloudflare. HTTPError: %d", url, e.response.status_code)
             return
 
         if response_json['success'] is False:

wagtail/contrib/frontend_cache/tests.py

@@ -1,6 +1,7 @@
 from unittest import mock
 from urllib.error import HTTPError, URLError
 
+import requests
 from django.core.exceptions import ImproperlyConfigured
 from django.test import TestCase
 from django.test.utils import override_settings
@@ -78,12 +79,23 @@ class TestBackendConfiguration(TestCase):
             hdrs={},
             fp=None
         )
-        self._test_http_with_side_effect(urlopen_side_effect=http_error)
+        with self.assertLogs(level='ERROR') as log_output:
+            self._test_http_with_side_effect(urlopen_side_effect=http_error)
+
+        self.assertIn(
+            "Couldn't purge 'http://www.wagtail.io/home/events/christmas/' from HTTP cache. HTTPError: 500 Internal Server Error",
+            log_output.output[0]
+        )
 
     def test_http_urlerror(self):
         """Test that `HTTPBackend.purge` can handle `URLError`"""
         url_error = URLError(reason='just for tests')
-        self._test_http_with_side_effect(urlopen_side_effect=url_error)
+        with self.assertLogs(level='ERROR') as log_output:
+            self._test_http_with_side_effect(urlopen_side_effect=url_error)
+        self.assertIn(
+            "Couldn't purge 'http://www.wagtail.io/home/events/christmas/' from HTTP cache. URLError: just for tests",
+            log_output.output[0]
+        )
 
     @mock.patch('wagtail.contrib.frontend_cache.backends.urlopen')
     def _test_http_with_side_effect(self, urlopen_mock, urlopen_side_effect):
@@ -308,3 +320,34 @@ class TestPurgeBatchClass(TestCase):
         batch.purge()
 
         self.assertEqual(batch.urls, ['http://localhost/events/', 'http://localhost/events/past/', 'http://localhost/foo'])
+
+    @mock.patch('wagtail.contrib.frontend_cache.backends.requests.delete')
+    def test_http_error_on_cloudflare_purge_batch(self, requests_delete_mock):
+        backend_settings = {
+            'cloudflare': {
+                'BACKEND': 'wagtail.contrib.frontend_cache.backends.CloudflareBackend',
+                'EMAIL': 'test@test.com',
+                'TOKEN': 'this is the token',
+                'ZONEID': 'this is a zone id',
+            },
+        }
+
+        class MockResponse:
+            def __init__(self, status_code=200):
+                self.status_code = status_code
+
+        http_error = requests.exceptions.HTTPError(response=MockResponse(status_code=500))
+        requests_delete_mock.side_effect = http_error
+
+        page = EventIndex.objects.get(url_path='/home/events/')
+
+        batch = PurgeBatch()
+        batch.add_page(page)
+
+        with self.assertLogs(level='ERROR') as log_output:
+            batch.purge(backend_settings=backend_settings)
+
+        self.assertIn(
+            "Couldn't purge 'http://localhost/events/' from Cloudflare. HTTPError: 500",
+            log_output.output[0]
+        )