APNs: Handle HTTP connection errors, and retry.

Should help with #6321 as at least a band-aid.

zerver/lib/push_notifications.py

@@ -15,6 +15,7 @@ from django.conf import settings
 from django.utils.timezone import now as timezone_now
 from django.utils.translation import ugettext as _
 from gcm import GCM
+from hyper.http20.exceptions import HTTP20Error
 import requests
 from six.moves import urllib
 import ujson
@@ -64,6 +65,8 @@ def get_apns_client():
                                   use_sandbox=settings.APNS_SANDBOX)
     return _apns_client
 
+APNS_MAX_RETRIES = 3
+
 @statsd_increment("apple_push_notification")
 def send_apple_push_notification(user_id, devices, payload_data):
     # type: (int, List[DeviceToken], Dict[str, Any]) -> None
@@ -74,12 +77,29 @@ def send_apple_push_notification(user_id, devices, payload_data):
     payload = APNsPayload(**payload_data)
     expiration = int(time.time() + 24 * 3600)
     client = get_apns_client()
+    retries_left = APNS_MAX_RETRIES
     for device in devices:
         # TODO obviously this should be made to actually use the async
+
+        def attempt_send():
+            # type: () -> Optional[str]
             stream_id = client.send_notification_async(
                 device.token, payload, topic='org.zulip.Zulip',
                 expiration=expiration)
-        result = client.get_notification_result(stream_id)
+            try:
+                return client.get_notification_result(stream_id)
+            except HTTP20Error as e:
+                logging.warn("APNs: HTTP error sending for user %d to device %s: %s",
+                             user_id, device.token, e.__class__.__name__)
+                return None
+
+        result = attempt_send()
+        while result is None and retries_left > 0:
+            retries_left -= 1
+            result = attempt_send()
+        if result is None:
+            result = "HTTP error, retries exhausted"
+
         if result == 'Success':
             logging.info("APNs: Success sending for user %d to device %s",
                          user_id, device.token)

zerver/tests/test_push_notifications.py

@@ -1,6 +1,7 @@
 from __future__ import absolute_import
 from __future__ import print_function
 
+import itertools
 import requests
 import mock
 from mock import call
@@ -526,6 +527,23 @@ class TestAPNs(PushNotificationTest):
                     "APNs: Success sending for user %d to device %s",
                     self.user_profile.id, device.token)
 
+    def test_http_retry(self):
+        # type: () -> None
+        import hyper
+        with mock.patch('zerver.lib.push_notifications._apns_client') as mock_apns, \
+                mock.patch('zerver.lib.push_notifications.logging') as mock_logging:
+            mock_apns.get_notification_result.side_effect = itertools.chain(
+                [hyper.http20.exceptions.StreamResetError()],
+                itertools.repeat('Success'))
+            self.send()
+            mock_logging.warn.assert_called_once_with(
+                "APNs: HTTP error sending for user %d to device %s: %s",
+                self.user_profile.id, self.devices()[0].token, "StreamResetError")
+            for device in self.devices():
+                mock_logging.info.assert_any_call(
+                    "APNs: Success sending for user %d to device %s",
+                    self.user_profile.id, device.token)
+
 class TestGetAlertFromMessage(PushNotificationTest):
     def test_get_alert_from_message(self):
         # type: () -> None