diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index eeb93e7da3..ad9987cc59 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -6,7 +6,19 @@ import logging import re from dataclasses import dataclass from functools import lru_cache -from typing import TYPE_CHECKING, Any, Dict, List, Mapping, Optional, Sequence, Tuple, Type, Union +from typing import ( + TYPE_CHECKING, + Any, + Dict, + Iterable, + List, + Mapping, + Optional, + Sequence, + Tuple, + Type, + Union, +) import gcm import lxml.html @@ -242,25 +254,41 @@ def send_apple_push_notification( ) payload_data = dict(modernize_apns_payload(payload_data)) message = {**payload_data.pop("custom", {}), "aps": payload_data} - for device in devices: - # TODO obviously this should be made to actually use the async - request = aioapns.NotificationRequest( - device_token=device.token, message=message, time_to_live=24 * 3600 - ) - try: - result = apns_context.loop.run_until_complete( - apns_context.apns.send_notification(request) + async def send_all_notifications() -> Iterable[ + Tuple[DeviceToken, Union[aioapns.common.NotificationResult, BaseException]] + ]: + requests = [ + aioapns.NotificationRequest( + device_token=device.token, message=message, time_to_live=24 * 3600 ) - except aioapns.exceptions.ConnectionError: + for device in devices + ] + results = list( + await asyncio.gather( + *(apns_context.apns.send_notification(request) for request in requests), + return_exceptions=True, + ) + ) + return zip(devices, results) + + results = apns_context.loop.run_until_complete(send_all_notifications()) + + for device, result in results: + if isinstance(result, aioapns.exceptions.ConnectionError): logger.error( "APNs: ConnectionError sending for user %s to device %s; check certificate expiration", user_identity, device.token, ) - continue - - if result.is_successful: + elif isinstance(result, BaseException): + logger.exception( + "APNs: Error sending for user %s to device %s", + user_identity, + device.token, + exc_info=result, + ) + elif result.is_successful: logger.info( "APNs: Success sending for user %s to device %s", user_identity, device.token ) diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index aa8f3af073..b536165162 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -1878,6 +1878,18 @@ class TestAPNs(PushNotificationTest): logger.output, ) + def test_other_exception(self) -> None: + self.setup_apns_tokens() + with self.mock_apns() as (apns_context, send_notification), self.assertLogs( + "zerver.lib.push_notifications", level="INFO" + ) as logger: + send_notification.side_effect = IOError + self.send(devices=self.devices()[0:1]) + self.assertIn( + f"ERROR:zerver.lib.push_notifications:APNs: Error sending for user to device {self.devices()[0].token}", + logger.output[1], + ) + def test_internal_server_error(self) -> None: self.setup_apns_tokens() with self.mock_apns() as (apns_context, send_notification), self.assertLogs(