Handle response body properly in delete_user

Author: jonathanlloyd

pusher_push_notifications/__init__.py

@@ -142,9 +142,7 @@ def _make_request(self, method, path, path_params, body=None):
         try:
             response_body = response.json()
         except ValueError:
-            raise PusherBadResponseError(
-                'The server returned a malformed response',
-            )
+            response_body = None
 
         return response_body
 
@@ -262,6 +260,11 @@ def publish_to_interests(self, interests, publish_body):
             body=publish_body,
         )
 
+        if response_body is None:
+            raise PusherBadResponseError(
+                'The server returned a malformed response',
+            )
+
         return response_body
 
     def publish_to_users(self, user_ids, publish_body):
@@ -331,6 +334,11 @@ def publish_to_users(self, user_ids, publish_body):
             body=publish_body,
         )
 
+        if response_body is None:
+            raise PusherBadResponseError(
+                'The server returned a malformed response',
+            )
+
         return response_body
 
     def authenticate_user(self, user_id):

tests/test_users.py

@@ -474,3 +474,97 @@ def test_delete_user_should_fail_if_user_id_too_long(self):
         with self.assertRaises(ValueError) as e:
             pn_client.delete_user('A'*165)
         self.assertIn('longer than the maximum of 164 chars', str(e.exception))
+
+    def test_delete_user_should_raise_on_http_4xx_error(self):
+        pn_client = PushNotifications(
+            'INSTANCE_ID',
+            'SECRET_KEY'
+        )
+        with requests_mock.Mocker() as http_mock:
+            http_mock.register_uri(
+                requests_mock.ANY,
+                requests_mock.ANY,
+                status_code=400,
+                json={'error': 'Invalid request', 'description': 'blah'},
+            )
+            with self.assertRaises(PusherValidationError) as e:
+                pn_client.delete_user('user-0001')
+            self.assertIn('Invalid request: blah', str(e.exception))
+
+    def test_delete_user_should_raise_on_http_5xx_error(self):
+        pn_client = PushNotifications(
+            'INSTANCE_ID',
+            'SECRET_KEY'
+        )
+        with requests_mock.Mocker() as http_mock:
+            http_mock.register_uri(
+                requests_mock.ANY,
+                requests_mock.ANY,
+                status_code=500,
+                json={'error': 'Server error', 'description': 'blah'},
+            )
+            with self.assertRaises(PusherServerError) as e:
+                pn_client.delete_user('user-0001')
+            self.assertIn('Server error: blah', str(e.exception))
+
+    def test_delete_user_should_raise_on_http_401_error(self):
+        pn_client = PushNotifications(
+            'INSTANCE_ID',
+            'SECRET_KEY'
+        )
+        with requests_mock.Mocker() as http_mock:
+            http_mock.register_uri(
+                requests_mock.ANY,
+                requests_mock.ANY,
+                status_code=401,
+                json={'error': 'Auth error', 'description': 'blah'},
+            )
+            with self.assertRaises(PusherAuthError) as e:
+                pn_client.delete_user('user-0001')
+            self.assertIn('Auth error: blah', str(e.exception))
+
+    def test_publish_to_users_should_raise_on_http_404_error(self):
+        pn_client = PushNotifications(
+            'INSTANCE_ID',
+            'SECRET_KEY'
+        )
+        with requests_mock.Mocker() as http_mock:
+            http_mock.register_uri(
+                requests_mock.ANY,
+                requests_mock.ANY,
+                status_code=404,
+                json={'error': 'Instance not found', 'description': 'blah'},
+            )
+            with self.assertRaises(PusherMissingInstanceError) as e:
+                pn_client.delete_user('user-0001')
+            self.assertIn('Instance not found: blah', str(e.exception))
+
+    def test_delete_user_should_error_correctly_if_error_not_json(self):
+        pn_client = PushNotifications(
+            'INSTANCE_ID',
+            'SECRET_KEY'
+        )
+        with requests_mock.Mocker() as http_mock:
+            http_mock.register_uri(
+                requests_mock.ANY,
+                requests_mock.ANY,
+                status_code=500,
+                text='<notjson></notjson>',
+            )
+            with self.assertRaises(PusherServerError) as e:
+                pn_client.delete_user('user-0001')
+            self.assertIn('Unknown error: no description', str(e.exception))
+
+    def test_delete_user_should_not_error_on_not_json_success(self):
+        pn_client = PushNotifications(
+            'INSTANCE_ID',
+            'SECRET_KEY'
+        )
+        with requests_mock.Mocker() as http_mock:
+            http_mock.register_uri(
+                requests_mock.ANY,
+                requests_mock.ANY,
+                status_code=200,
+                text='<notjson></notjson>',
+            )
+            pn_client.delete_user('alice')