diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index 7e45633a7..79e437983 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -67,6 +67,9 @@ class RegistryError(Exception): def should_retry(self): return self.code == ErrorCode.COMMAND_FAILED + def is_session_error(self): + return self.code is not None and (self.code >= 2501 and self.code <= 2502) + def is_server_error(self): return self.code is not None and (self.code >= 2400 and self.code <= 2500) diff --git a/src/registrar/admin.py b/src/registrar/admin.py index 56f1c093a..a7e03cf75 100644 --- a/src/registrar/admin.py +++ b/src/registrar/admin.py @@ -311,7 +311,17 @@ class DomainAdmin(ListHeaderAdmin): obj.place_client_hold() obj.save() except Exception as err: - self.message_user(request, err, messages.ERROR) + # if error is an error from the registry, display useful + # and readable error + if err.code: + self.message_user( + request, + "Error placing the hold with the registry: {err}", + messages.ERROR + ) + else: + # all other type error messages, display the error + self.message_user(request, err, messages.ERROR) else: self.message_user( request, @@ -328,7 +338,17 @@ class DomainAdmin(ListHeaderAdmin): obj.revert_client_hold() obj.save() except Exception as err: - self.message_user(request, err, messages.ERROR) + # if error is an error from the registry, display useful + # and readable error + if err.code: + self.message_user( + request, + "Error removing the hold in the registry: {err}", + messages.ERROR + ) + else: + # all other type error messages, display the error + self.message_user(request, err, messages.ERROR) else: self.message_user( request, diff --git a/src/registrar/models/domain.py b/src/registrar/models/domain.py index b0bf00082..d34db2a84 100644 --- a/src/registrar/models/domain.py +++ b/src/registrar/models/domain.py @@ -635,13 +635,29 @@ class Domain(TimeStampedModel, DomainHelper): """This domain should not be active. may raises RegistryError, should be caught or handled correctly by caller""" request = commands.UpdateDomain(name=self.name, add=[self.clientHoldStatus()]) - registry.send(request, cleaned=True) + try: + registry.send(request, cleaned=True) + self._invalidate_cache() + except RegistryError as err: + # if registry error occurs, log the error, and raise it as well + logger.error( + f"registry error placing client hold: {err}" + ) + raise (err) def _remove_client_hold(self): """This domain is okay to be active. may raises RegistryError, should be caught or handled correctly by caller""" request = commands.UpdateDomain(name=self.name, rem=[self.clientHoldStatus()]) - registry.send(request, cleaned=True) + try: + registry.send(request, cleaned=True) + self._invalidate_cache() + except RegistryError as err: + # if registry error occurs, log the error, and raise it as well + logger.error( + f"registry error removing client hold: {err}" + ) + raise (err) def _delete_domain(self): """This domain should be deleted from the registry diff --git a/src/registrar/tests/test_models_domain.py b/src/registrar/tests/test_models_domain.py index 9aaac7321..edfce293c 100644 --- a/src/registrar/tests/test_models_domain.py +++ b/src/registrar/tests/test_models_domain.py @@ -634,7 +634,11 @@ class TestAnalystClientHold(TestCase): Given the analyst is logged in And a domain exists in the registry """ - pass + super().setUp() + self.domain, _ = Domain.objects.get_or_create(name="security.gov") + + def tearDown(self): + super().tearDown() @skip("not implemented yet") def test_analyst_places_client_hold(self):