diff --git a/zerver/lib/push_notifications.py b/zerver/lib/push_notifications.py index 0430917a1e..916b441eb6 100644 --- a/zerver/lib/push_notifications.py +++ b/zerver/lib/push_notifications.py @@ -44,6 +44,7 @@ from zerver.lib.exceptions import ErrorCode, JsonableError from zerver.lib.message import access_message, huddle_users from zerver.lib.outgoing_http import OutgoingSession from zerver.lib.remote_server import ( + PushNotificationBouncerServerError, send_json_to_push_bouncer, send_realms_only_to_push_bouncer, send_to_push_bouncer, @@ -786,6 +787,12 @@ def initialize_push_notifications() -> None: try: realms = send_realms_only_to_push_bouncer() + except PushNotificationBouncerServerError: # nocoverage + # 50x errors from the bouncer cannot be addressed by the + # administrator of this server, and may be localized to + # this endpoint; don't rashly mark push notifications as + # disabled when they are likely working perfectly fine. + pass except Exception: # An exception was thrown trying to ask the bouncer service whether we can send # push notifications or not. There may be certain transient failures that we could diff --git a/zerver/lib/remote_server.py b/zerver/lib/remote_server.py index 4c6fa0963e..9c14fe81d1 100644 --- a/zerver/lib/remote_server.py +++ b/zerver/lib/remote_server.py @@ -32,6 +32,10 @@ class PushNotificationBouncerRetryLaterError(JsonableError): http_status_code = 502 +class PushNotificationBouncerServerError(PushNotificationBouncerRetryLaterError): + http_status_code = 502 + + class RealmDataForAnalytics(BaseModel): model_config = ConfigDict(extra="forbid") @@ -114,7 +118,7 @@ def send_to_push_bouncer( # to the callers that the attempt failed and they can retry. error_msg = "Received 500 from push notification bouncer" logging.warning(error_msg) - raise PushNotificationBouncerRetryLaterError(error_msg) + raise PushNotificationBouncerServerError(error_msg) elif res.status_code >= 400: # If JSON parsing errors, just let that exception happen result_dict = orjson.loads(res.content) diff --git a/zerver/tests/test_push_notifications.py b/zerver/tests/test_push_notifications.py index 48fdac1b1c..6faddd4752 100644 --- a/zerver/tests/test_push_notifications.py +++ b/zerver/tests/test_push_notifications.py @@ -62,6 +62,7 @@ from zerver.lib.push_notifications import ( from zerver.lib.remote_server import ( PushNotificationBouncerError, PushNotificationBouncerRetryLaterError, + PushNotificationBouncerServerError, build_analytics_data, get_realms_info_for_push_bouncer, send_analytics_to_push_bouncer, @@ -906,7 +907,7 @@ class PushBouncerNotificationTest(BouncerTestCase): with responses.RequestsMock() as resp, self.assertLogs(level="WARNING") as warn_log: resp.add(responses.POST, URL, body=orjson.dumps({"msg": "error"}), status=500) with self.assertRaisesRegex( - PushNotificationBouncerRetryLaterError, + PushNotificationBouncerServerError, r"Received 500 from push notification bouncer$", ): self.client_post(endpoint, {"token": token, **appid}, subdomain="zulip") @@ -3492,7 +3493,7 @@ class TestSendToPushBouncer(ZulipTestCase): def test_500_error(self) -> None: self.add_mock_response(status=500) with self.assertLogs(level="WARNING") as m: - with self.assertRaises(PushNotificationBouncerRetryLaterError): + with self.assertRaises(PushNotificationBouncerServerError): send_to_push_bouncer("POST", "register", {"data": "true"}) self.assertEqual(m.output, ["WARNING:root:Received 500 from push notification bouncer"])