register_server: Improve UX with the "hostname already in use" error.

An even better way than the current json error message recommending the
--registration-transfer option is to return an appropriate error code
and have that get picked up by the register_server command.

The register_server command can then display a more comprehensive,
better formatted error message with proper whitespaces and a pointer to
the documentation.
This commit is contained in:
Mateusz Mandera
2025-01-29 20:53:46 +08:00
committed by Tim Abbott
parent 6d555b01d0
commit ddcc36c3aa
5 changed files with 38 additions and 14 deletions

View File

@@ -57,6 +57,7 @@ class ErrorCode(Enum):
CANNOT_DEACTIVATE_GROUP_IN_USE = auto()
CANNOT_ADMINISTER_CHANNEL = auto()
REMOTE_SERVER_VERIFICATION_SECRET_NOT_PREPARED = auto()
HOSTNAME_ALREADY_IN_USE_BOUNCER_ERROR = auto()
class JsonableError(Exception):

View File

@@ -1528,3 +1528,19 @@ class InvalidRemotePushDeviceTokenError(JsonableError):
class PushNotificationsDisallowedByBouncerError(Exception):
def __init__(self, reason: str) -> None:
self.reason = reason
class HostnameAlreadyInUseBouncerError(JsonableError):
code = ErrorCode.HOSTNAME_ALREADY_IN_USE_BOUNCER_ERROR
data_fields = ["hostname"]
def __init__(self, hostname: str) -> None:
self.hostname: str = hostname
@staticmethod
@override
def msg_format() -> str:
# This message is not read by any of the client apps, just potentially displayed
# via server administration tools, so it doesn't need translations.
return "A server with hostname {hostname} already exists"

View File

@@ -233,6 +233,23 @@ that registration and saving the updated secret in
except Exception:
raise e
if (
"code" in content_dict
and content_dict["code"] == "HOSTNAME_ALREADY_IN_USE_BOUNCER_ERROR"
):
print(
"--------------------------------\n"
"The hostname is already in use by another server. If you control the hostname \n"
"and want to transfer the registration to this server, you can run manage.py register_server \n"
"with the --registration-transfer flag.\n"
"Note that this will invalidate old credentials if another server is still using them.\n"
"\n"
"For more information, see: \n"
"\n"
"https://zulip.readthedocs.io/en/latest/production/mobile-push-notifications.html#moving-your-registration-to-a-new-server"
)
raise CommandError
error_message = content_dict["msg"]
raise CommandError(
f'Error received from the push notification service: "{error_message}"'

View File

@@ -5359,13 +5359,8 @@ class PushBouncerSignupTest(ZulipTestCase):
contact_email="server-admin@zulip.com",
)
result = self.client_post("/api/v1/remotes/server/register", request)
self.assert_json_error(
result,
"A server with hostname example.com already exists. "
"If you control the hostname "
"and want to transfer the registration to this server, you can run manage.py register_server "
"with the --registration-transfer flag.",
)
self.assert_json_error(result, "A server with hostname example.com already exists")
self.assertEqual(result.json()["code"], "HOSTNAME_ALREADY_IN_USE_BOUNCER_ERROR")
def test_register_contact_email_validation_rules(self) -> None:
zulip_org_id = str(uuid.uuid4())

View File

@@ -48,6 +48,7 @@ from zerver.lib.exceptions import (
)
from zerver.lib.outgoing_http import OutgoingSession
from zerver.lib.push_notifications import (
HostnameAlreadyInUseBouncerError,
InvalidRemotePushDeviceTokenError,
UserPushIdentityCompat,
send_android_push_notification,
@@ -259,13 +260,7 @@ def register_remote_server(
raise RemoteServerDeactivatedError
if remote_server is None and RemoteZulipServer.objects.filter(hostname=hostname).exists():
raise JsonableError(
_(
"A server with hostname {hostname} already exists. If you control the hostname "
"and want to transfer the registration to this server, you can run manage.py register_server "
"with the --registration-transfer flag."
).format(hostname=hostname)
)
raise HostnameAlreadyInUseBouncerError(hostname)
with transaction.atomic(durable=True):
if remote_server is None: