push_notifs: Improve error responses from /test_notification endpoint.

zerver/lib/exceptions.py

@@ -25,6 +25,8 @@ class ErrorCode(Enum):
     CSRF_FAILED = auto()
     INVITATION_FAILED = auto()
     INVALID_ZULIP_SERVER = auto()
+    INVALID_PUSH_DEVICE_TOKEN = auto()
+    INVALID_REMOTE_PUSH_DEVICE_TOKEN = auto()
     INVALID_MARKDOWN_INCLUDE_STATEMENT = auto()
     REQUEST_CONFUSING_VAR = auto()
     INVALID_API_KEY = auto()

zerver/lib/push_notifications.py

@@ -34,7 +34,7 @@ from typing_extensions import TypeAlias, override
 
 from zerver.lib.avatar import absolute_avatar_url
 from zerver.lib.emoji_utils import hex_codepoint_to_emoji
-from zerver.lib.exceptions import JsonableError
+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 send_json_to_push_bouncer, send_to_push_bouncer
@@ -1251,3 +1251,27 @@ def send_test_push_notification(user_profile: UserProfile, devices: List[PushDev
     send_test_push_notification_directly_to_devices(
         user_identity, devices, base_payload, remote=None
     )
+
+
+class InvalidPushDeviceTokenError(JsonableError):
+    code = ErrorCode.INVALID_PUSH_DEVICE_TOKEN
+
+    def __init__(self) -> None:
+        pass
+
+    @staticmethod
+    @override
+    def msg_format() -> str:
+        return _("Device not recognized")
+
+
+class InvalidRemotePushDeviceTokenError(JsonableError):
+    code = ErrorCode.INVALID_REMOTE_PUSH_DEVICE_TOKEN
+
+    def __init__(self) -> None:
+        pass
+
+    @staticmethod
+    @override
+    def msg_format() -> str:
+        return _("Device not recognized by the push bouncer")

zerver/lib/remote_server.py

@@ -92,6 +92,17 @@ def send_to_push_bouncer(
             raise PushNotificationBouncerError(
                 _("Push notifications bouncer error: {error}").format(error=msg)
             )
+        elif (
+            endpoint == "push/test_notification"
+            and "code" in result_dict
+            and result_dict["code"] == "INVALID_REMOTE_PUSH_DEVICE_TOKEN"
+        ):
+            # This error from the notification debugging endpoint should just be directly
+            # communicated to the device.
+            # TODO: Extend this to use a more general mechanism when we add more such error responses.
+            from zerver.lib.push_notifications import InvalidRemotePushDeviceTokenError
+
+            raise InvalidRemotePushDeviceTokenError
         else:
             # But most other errors coming from the push bouncer
             # server are client errors (e.g. never-registered token)

zerver/openapi/zulip.yaml

@@ -9175,21 +9175,14 @@ paths:
               schema:
                 $ref: "#/components/schemas/JsonSuccess"
         "400":
-          description: Bad request.
+          description: |
+            Bad request.
           content:
             application/json:
               schema:
-                allOf:
+                oneOf:
-                  - $ref: "#/components/schemas/CodedError"
+                  - $ref: "#/components/schemas/InvalidPushDeviceTokenError"
-                  - example:
+                  - $ref: "#/components/schemas/InvalidRemotePushDeviceTokenError"
-                      {
-                        "code": "BAD_REQUEST",
-                        "msg": "Token does not exist",
-                        "result": "error",
-                      }
-                    description: |
-                      An example JSON response for when a device with the specified token
-                      does not exist:
   /user_topics:
     post:
       operationId: update-user-topic
@@ -19619,6 +19612,33 @@ components:
             before Zulip 5.0 (feature level 76).
 
             A typical failed json response for when user's organization is deactivated:
+    InvalidPushDeviceTokenError:
+      allOf:
+        - $ref: "#/components/schemas/CodedError"
+        - example:
+            {
+              "code": "INVALID_PUSH_DEVICE_TOKEN",
+              "msg": "Device not recognized",
+              "result": "error",
+            }
+          description: |
+            ## Invalid push device token
+
+            A typical failed JSON response for when the push device token is invalid:
+    InvalidRemotePushDeviceTokenError:
+      allOf:
+        - $ref: "#/components/schemas/CodedError"
+        - example:
+            {
+              "code": "INVALID_REMOTE_PUSH_DEVICE_TOKEN",
+              "msg": "Device not recognized by the push bouncer",
+              "result": "error",
+            }
+          description: |
+            ## Invalid push device token
+
+            A typical failed JSON response for when the push device token is not recognized
+            by the push notification bouncer:
 
   ###################
   # Shared responses

zerver/tests/test_push_notifications.py

@@ -35,6 +35,7 @@ from zerver.lib.exceptions import JsonableError
 from zerver.lib.push_notifications import (
     APNsContext,
     DeviceToken,
+    InvalidRemotePushDeviceTokenError,
     UserPushIdentityCompat,
     b64_to_hex,
     get_apns_badge_count,
@@ -150,13 +151,20 @@ class BouncerTestCase(ZulipTestCase):
 
 
 class SendTestPushNotificationEndpointTest(BouncerTestCase):
+    @override_settings(PUSH_NOTIFICATION_BOUNCER_URL="https://push.zulip.org.example.com")
+    @responses.activate
     def test_send_test_push_notification_api_invalid_token(self) -> None:
+        # What happens when the mobile device isn't registered with its server,
+        # and makes a request to this API:
         user = self.example_user("cordelia")
         result = self.api_post(
             user, "/api/v1/mobile_push/test_notification", {"token": "invalid"}, subdomain="zulip"
         )
-        self.assert_json_error(result, "Token does not exist")
+        self.assert_json_error(result, "Device not recognized")
+        self.assertEqual(orjson.loads(result.content)["code"], "INVALID_PUSH_DEVICE_TOKEN")
 
+        # What response the server receives when it makes a request to the bouncer
+        # to the /test_notification endpoint:
         payload = {
             "user_uuid": str(user.uuid),
             "user_id": user.id,
@@ -171,7 +179,33 @@ class SendTestPushNotificationEndpointTest(BouncerTestCase):
             subdomain="",
             content_type="application/json",
         )
-        self.assert_json_error(result, "Token does not exist")
+        self.assert_json_error(result, "Device not recognized by the push bouncer")
+        self.assertEqual(orjson.loads(result.content)["code"], "INVALID_REMOTE_PUSH_DEVICE_TOKEN")
+
+        # Finally, test the full scenario where the mobile device is registered with its
+        # server, but for some reason the server failed to register it with the bouncer.
+
+        token = "111222"
+        token_kind = PushDeviceToken.GCM
+        # We create a PushDeviceToken object, but no RemotePushDeviceToken object, to simulate
+        # a missing registration on the bouncer.
+        PushDeviceToken.objects.create(user=user, token=token, kind=token_kind)
+
+        # As verified above, this is the response the server receives from the bouncer in this kind of case.
+        # We have to simulate it with a response mock.
+        error_response = json_response_from_error(InvalidRemotePushDeviceTokenError())
+        responses.add(
+            responses.POST,
+            f"{settings.PUSH_NOTIFICATION_BOUNCER_URL}/api/v1/remotes/push/test_notification",
+            body=error_response.content,
+            status=error_response.status_code,
+        )
+
+        result = self.api_post(
+            user, "/api/v1/mobile_push/test_notification", {"token": token}, subdomain="zulip"
+        )
+        self.assert_json_error(result, "Device not recognized by the push bouncer")
+        self.assertEqual(orjson.loads(result.content)["code"], "INVALID_REMOTE_PUSH_DEVICE_TOKEN")
 
     def test_send_test_push_notification_api_no_bouncer_config(self) -> None:
         """

zerver/views/push_notifications.py

@@ -7,6 +7,7 @@ from django.utils.translation import gettext as _
 from zerver.decorator import human_users_only
 from zerver.lib.exceptions import JsonableError
 from zerver.lib.push_notifications import (
+    InvalidPushDeviceTokenError,
     add_push_device_token,
     b64_to_hex,
     remove_push_device_token,
@@ -83,7 +84,7 @@ def send_test_push_notification_api(
         try:
             devices = [PushDeviceToken.objects.get(token=token, user=user_profile)]
         except PushDeviceToken.DoesNotExist:
-            raise JsonableError(_("Token does not exist"))
+            raise InvalidPushDeviceTokenError
     else:
         devices = list(PushDeviceToken.objects.filter(user=user_profile))
 

zilencer/views.py

@@ -22,6 +22,7 @@ from corporate.lib.stripe import do_deactivate_remote_server
 from zerver.decorator import require_post
 from zerver.lib.exceptions import JsonableError
 from zerver.lib.push_notifications import (
+    InvalidRemotePushDeviceTokenError,
     UserPushIdentityCompat,
     send_android_push_notification,
     send_apple_push_notification,
@@ -322,7 +323,7 @@ def remote_server_send_test_notification(
             user_identity.filter_q(), token=token, kind=token_kind, server=server
         )
     except RemotePushDeviceToken.DoesNotExist:
-        raise JsonableError(err_("Token does not exist"))
+        raise InvalidRemotePushDeviceTokenError
 
     send_test_push_notification_directly_to_devices(
         user_identity, [device], base_payload, remote=server