notifications: Don't send error emails on bouncer 500s.

Since the individual server administrator can't do anything about
these, this should not trigger an email notification.

zerver/lib/remote_server.py

@@ -1,3 +1,4 @@
+import logging
 import requests
 import ujson
 import urllib
@@ -18,7 +19,7 @@ class PushNotificationBouncerException(Exception):
 def send_to_push_bouncer(method: str,
                          endpoint: str,
                          post_data: Union[str, Dict[str, Any]],
-                         extra_headers: Optional[Dict[str, Any]]=None) -> Dict[str, Any]:
+                         extra_headers: Optional[Dict[str, Any]]=None) -> Tuple[Dict[str, Any], bool]:
     """While it does actually send the notice, this function has a lot of
     code and comments around error handling for the push notifications
     bouncer.  There are several classes of failures, each with its own
@@ -53,11 +54,12 @@ def send_to_push_bouncer(method: str,
 
     if res.status_code >= 500:
         # 500s should be resolved by the people who run the push
-        # notification bouncer service, since they'll get an email
-        # too.  For now we email the server admin, but we'll likely
-        # want to do some sort of retry logic eventually.
-        raise PushNotificationBouncerException(
-            _("Received 500 from push notification bouncer"))
+        # notification bouncer service, and they'll get an appropriate
+        # error notification from the server.  We just return after
+        # doing something.  Ideally, we'll add do some sort of spaced
+        # retry eventually.
+        logging.warning("Received 500 from push notification bouncer")
+        return {}, True
     elif res.status_code >= 400:
         # If JSON parsing errors, just let that exception happen
         result_dict = ujson.loads(res.content)
@@ -79,7 +81,7 @@ def send_to_push_bouncer(method: str,
             "Push notification bouncer returned unexpected status code %s" % (res.status_code,))
 
     # If we don't throw an exception, it's a successful bounce!
-    return ujson.loads(res.content)
+    return ujson.loads(res.content), False
 
 def send_json_to_push_bouncer(method: str, endpoint: str, post_data: Dict[str, Any]) -> None:
     send_to_push_bouncer(
@@ -110,7 +112,10 @@ def build_analytics_data(realm_count_query: Any,
 
 def send_analytics_to_remote_server() -> None:
     # first, check what's latest
-    result = send_to_push_bouncer("GET", "server/analytics/status", {})
+    (result, failed) = send_to_push_bouncer("GET", "server/analytics/status", {})
+    if failed:  # nocoverage
+        return
+
     last_acked_realm_count_id = result['last_realm_count_id']
     last_acked_installation_count_id = result['last_installation_count_id']
 

zerver/tests/test_push_notifications.py

@@ -1239,11 +1239,12 @@ class Result:
 
 class TestSendToPushBouncer(ZulipTestCase):
     @mock.patch('requests.request', return_value=Result(status=500))
-    def test_500_error(self, mock_request: mock.MagicMock) -> None:
-        with self.assertRaises(PushNotificationBouncerException) as exc:
-            send_to_push_bouncer('register', 'register', {'data': True})
-        self.assertEqual(str(exc.exception),
-                         'Received 500 from push notification bouncer')
+    @mock.patch('logging.warning')
+    def test_500_error(self, mock_request: mock.MagicMock, mock_warning: mock.MagicMock) -> None:
+        result, failed = send_to_push_bouncer('register', 'register', {'data': True})
+        self.assertEqual(result, {})
+        self.assertEqual(failed, True)
+        mock_warning.assert_called_once()
 
     @mock.patch('requests.request', return_value=Result(status=400))
     def test_400_error(self, mock_request: mock.MagicMock) -> None: