From d31af94a15d5a7d29d3df4b7cf125d1a4e32be45 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 24 Mar 2025 13:16:20 -0400 Subject: [PATCH 1/7] wip --- src/epplibwrapper/client.py | 11 +++++++---- src/epplibwrapper/errors.py | 8 ++++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index 9d203b246..81d1f414e 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -100,7 +100,7 @@ class EPPLibWrapper: response = self._client.send(self._login) # type: ignore if response.code >= 2000: # type: ignore self._client.close() # type: ignore - raise LoginError(response.msg) # type: ignore + raise LoginError(response.msg, response=response) # type: ignore def _disconnect(self) -> None: """Close the connection. Sends a logout command and closes the connection.""" @@ -144,14 +144,14 @@ class EPPLibWrapper: text = "failed to execute due to a registry login error." message = f"{cmd_type} {text}" logger.error(f"{message} Error: {err}") - raise RegistryError(message) from err + raise LoginError(message, response=err.response) from err except Exception as err: message = f"{cmd_type} failed to execute due to an unknown error." logger.error(f"{message} Error: {err}") raise RegistryError(message) from err else: if response.code >= 2000: - raise RegistryError(response.msg, code=response.code) + raise RegistryError(response.msg, code=response.code, response=response) else: return response @@ -182,7 +182,10 @@ class EPPLibWrapper: or err.should_retry() ): message = f"{cmd_type} failed and will be retried" - logger.info(f"{message} Error: {err}") + info_message = f"{message} Error: {err}" + if err.response: + info_message = f"{info_message}. cltrid is {err.response.cl_tr_id} svtrid is {err.response.sv_tr_id}" + logger.info(f"{info_message}") return self._retry(command) else: raise err diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index 95db40ab8..b27005998 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -62,14 +62,18 @@ class RegistryError(Exception): - 2501 - 2502 Something malicious or abusive may have occurred """ - def __init__(self, *args, code=None, note="", **kwargs): + def __init__(self, *args, code=None, note="", response=None, **kwargs): super().__init__(*args, **kwargs) self.code = code + self.response = response # note is a string that can be used to provide additional context self.note = note + def should_retry(self): - return self.code == ErrorCode.COMMAND_FAILED + # COMMAND_USE_ERROR is returning with message, Registrar is not logged in, + # which can be recovered from with a retry + return self.code == ErrorCode.COMMAND_FAILED or self.code == ErrorCode.COMMAND_USE_ERROR def is_transport_error(self): return self.code == ErrorCode.TRANSPORT_ERROR From d498ba38187845088b03df0a7757b79a5848979b Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 24 Mar 2025 16:32:38 -0400 Subject: [PATCH 2/7] added unit test --- src/epplibwrapper/client.py | 2 +- src/epplibwrapper/tests/test_client.py | 51 ++++++++++++++++++++++++++ 2 files changed, 52 insertions(+), 1 deletion(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index 81d1f414e..f5bfc8fb6 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -184,7 +184,7 @@ class EPPLibWrapper: message = f"{cmd_type} failed and will be retried" info_message = f"{message} Error: {err}" if err.response: - info_message = f"{info_message}. cltrid is {err.response.cl_tr_id} svtrid is {err.response.sv_tr_id}" + info_message = f"{info_message}| cltrid is {err.response.cl_tr_id} svtrid is {err.response.sv_tr_id}" logger.info(f"{info_message}") return self._retry(command) else: diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index 57c99a05f..d894e369a 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -264,6 +264,57 @@ class TestClient(TestCase): # send() is called 5 times: send(login), send(command) fail, send(logout), send(login), send(command) self.assertEquals(mock_send.call_count, 5) + @less_console_noise_decorator + @patch("epplibwrapper.client.Client") + @patch("epplibwrapper.client.logger") + def test_send_command_2002_failure_prompts_successful_retry(self, mock_logger, mock_client): + """Test when the send("InfoDomainCommand) call fails with a 2002, prompting a retry + and the subsequent send("InfoDomainCommand) call succeeds + Flow: + Initialization succeeds + Send command fails (with 2002 code) prompting retry + Client closes and re-initializes, and command succeeds""" + # Mock the Client instance and its methods + # connect() and close() should succeed throughout + mock_connect = MagicMock() + mock_close = MagicMock() + # create success and failure result messages + send_command_success_result = self.fake_result(1000, "Command completed successfully") + send_command_failure_result = self.fake_result(2002, "Registrar is not logging in.") + # side_effect for send call, initial send(login) succeeds during initialization, next send(command) + # fails, subsequent sends (logout, login, command) all succeed + send_call_count = 0 + + # Create a mock command + mock_command = MagicMock() + mock_command.__class__.__name__ = "InfoDomainCommand" + + def side_effect(*args, **kwargs): + nonlocal send_call_count + send_call_count += 1 + if send_call_count == 2: + return send_command_failure_result + else: + return send_command_success_result + + mock_send = MagicMock(side_effect=side_effect) + mock_client.return_value.connect = mock_connect + mock_client.return_value.close = mock_close + mock_client.return_value.send = mock_send + # Create EPPLibWrapper instance and initialize client + wrapper = EPPLibWrapper() + wrapper.send(mock_command, cleaned=True) + # connect() is called twice, once during initialization of app, once during retry + self.assertEquals(mock_connect.call_count, 2) + # close() is called once, during retry + mock_close.assert_called_once() + # send() is called 5 times: send(login), send(command) fail, send(logout), send(login), send(command) + self.assertEquals(mock_send.call_count, 5) + # Assertion proper logging; note that the + mock_logger.info.assert_called_once_with( + "InfoDomainCommand failed and will be retried Error: Registrar is not logging in.| cltrid is cl_tr_id svtrid is sv_tr_id" + ) + @less_console_noise_decorator def fake_failure_send_concurrent_threads(self, command=None, cleaned=None): """ From 39f577c1b40624817f9ba82398da0b10cda74289 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 24 Mar 2025 17:27:28 -0400 Subject: [PATCH 3/7] reset behavior when LoginError --- src/epplibwrapper/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index f5bfc8fb6..72922ba83 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -144,7 +144,7 @@ class EPPLibWrapper: text = "failed to execute due to a registry login error." message = f"{cmd_type} {text}" logger.error(f"{message} Error: {err}") - raise LoginError(message, response=err.response) from err + raise RegistryError(message) from err except Exception as err: message = f"{cmd_type} failed to execute due to an unknown error." logger.error(f"{message} Error: {err}") From 681963343a3e41ab2609b70a42a7f1201ceef7e2 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 24 Mar 2025 17:28:39 -0400 Subject: [PATCH 4/7] lint --- src/epplibwrapper/client.py | 4 +++- src/epplibwrapper/errors.py | 1 - src/epplibwrapper/tests/test_client.py | 4 ++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index 72922ba83..2a10e552e 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -184,7 +184,9 @@ class EPPLibWrapper: message = f"{cmd_type} failed and will be retried" info_message = f"{message} Error: {err}" if err.response: - info_message = f"{info_message}| cltrid is {err.response.cl_tr_id} svtrid is {err.response.sv_tr_id}" + info_message = ( + f"{info_message}| cltrid is {err.response.cl_tr_id} svtrid is {err.response.sv_tr_id}" + ) logger.info(f"{info_message}") return self._retry(command) else: diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index b27005998..02b6e5c36 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -69,7 +69,6 @@ class RegistryError(Exception): # note is a string that can be used to provide additional context self.note = note - def should_retry(self): # COMMAND_USE_ERROR is returning with message, Registrar is not logged in, # which can be recovered from with a retry diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index d894e369a..d47dbdfae 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -310,10 +310,10 @@ class TestClient(TestCase): mock_close.assert_called_once() # send() is called 5 times: send(login), send(command) fail, send(logout), send(login), send(command) self.assertEquals(mock_send.call_count, 5) - # Assertion proper logging; note that the + # Assertion proper logging; note that the mock_logger.info.assert_called_once_with( "InfoDomainCommand failed and will be retried Error: Registrar is not logging in.| cltrid is cl_tr_id svtrid is sv_tr_id" - ) + ) @less_console_noise_decorator def fake_failure_send_concurrent_threads(self, command=None, cleaned=None): From 6948d6c2d49e637c87225a24f10a8ad650789eff Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Mon, 24 Mar 2025 17:36:42 -0400 Subject: [PATCH 5/7] removing unnecessary additional code --- src/epplibwrapper/client.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index 2a10e552e..c4e64ec84 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -100,7 +100,7 @@ class EPPLibWrapper: response = self._client.send(self._login) # type: ignore if response.code >= 2000: # type: ignore self._client.close() # type: ignore - raise LoginError(response.msg, response=response) # type: ignore + raise LoginError(response.msg) # type: ignore def _disconnect(self) -> None: """Close the connection. Sends a logout command and closes the connection.""" From 7a935d11fc2f9ca380a68ca68b1228b3b19ba9d2 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Tue, 25 Mar 2025 13:23:29 -0400 Subject: [PATCH 6/7] more specific retry condition implemented --- src/epplibwrapper/errors.py | 12 ++++++++++-- src/epplibwrapper/tests/test_client.py | 4 ++-- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/epplibwrapper/errors.py b/src/epplibwrapper/errors.py index 02b6e5c36..234bed611 100644 --- a/src/epplibwrapper/errors.py +++ b/src/epplibwrapper/errors.py @@ -1,4 +1,4 @@ -from enum import IntEnum +from enum import IntEnum, Enum class ErrorCode(IntEnum): @@ -52,6 +52,10 @@ class ErrorCode(IntEnum): SESSION_LIMIT_EXCEEDED_SERVER_CLOSING_CONNECTION = 2502 +class RegistryErrorMessage(Enum): + REGISTRAR_NOT_LOGGED_IN = "Registrar is not logged in." + + class RegistryError(Exception): """ Overview of registry response codes from RFC 5730. See RFC 5730 for full text. @@ -72,7 +76,11 @@ class RegistryError(Exception): def should_retry(self): # COMMAND_USE_ERROR is returning with message, Registrar is not logged in, # which can be recovered from with a retry - return self.code == ErrorCode.COMMAND_FAILED or self.code == ErrorCode.COMMAND_USE_ERROR + return self.code == ErrorCode.COMMAND_FAILED or ( + self.code == ErrorCode.COMMAND_USE_ERROR + and self.response + and getattr(self.response, "msg", None) == RegistryErrorMessage.REGISTRAR_NOT_LOGGED_IN.value + ) def is_transport_error(self): return self.code == ErrorCode.TRANSPORT_ERROR diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index d47dbdfae..868d3d6f1 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -280,7 +280,7 @@ class TestClient(TestCase): mock_close = MagicMock() # create success and failure result messages send_command_success_result = self.fake_result(1000, "Command completed successfully") - send_command_failure_result = self.fake_result(2002, "Registrar is not logging in.") + send_command_failure_result = self.fake_result(2002, "Registrar is not logged in.") # side_effect for send call, initial send(login) succeeds during initialization, next send(command) # fails, subsequent sends (logout, login, command) all succeed send_call_count = 0 @@ -312,7 +312,7 @@ class TestClient(TestCase): self.assertEquals(mock_send.call_count, 5) # Assertion proper logging; note that the mock_logger.info.assert_called_once_with( - "InfoDomainCommand failed and will be retried Error: Registrar is not logging in.| cltrid is cl_tr_id svtrid is sv_tr_id" + "InfoDomainCommand failed and will be retried Error: Registrar is not logged in.| cltrid is cl_tr_id svtrid is sv_tr_id" ) @less_console_noise_decorator From 72f256e0b81d935fcf168d2e1586992fe8443c85 Mon Sep 17 00:00:00 2001 From: David Kennedy Date: Wed, 26 Mar 2025 18:32:59 -0400 Subject: [PATCH 7/7] split message and apply to more scenarios --- src/epplibwrapper/client.py | 9 +++------ src/epplibwrapper/tests/test_client.py | 5 +++-- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index c4e64ec84..a7102b0e9 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -174,6 +174,8 @@ class EPPLibWrapper: try: return self._send(command) except RegistryError as err: + if err.response: + logger.info(f"cltrid is {err.response.cl_tr_id} svtrid is {err.response.sv_tr_id}") if ( err.is_transport_error() or err.is_connection_error() @@ -182,12 +184,7 @@ class EPPLibWrapper: or err.should_retry() ): message = f"{cmd_type} failed and will be retried" - info_message = f"{message} Error: {err}" - if err.response: - info_message = ( - f"{info_message}| cltrid is {err.response.cl_tr_id} svtrid is {err.response.sv_tr_id}" - ) - logger.info(f"{info_message}") + logger.info(f"{message} Error: {err}") return self._retry(command) else: raise err diff --git a/src/epplibwrapper/tests/test_client.py b/src/epplibwrapper/tests/test_client.py index 868d3d6f1..2850ae316 100644 --- a/src/epplibwrapper/tests/test_client.py +++ b/src/epplibwrapper/tests/test_client.py @@ -311,9 +311,10 @@ class TestClient(TestCase): # send() is called 5 times: send(login), send(command) fail, send(logout), send(login), send(command) self.assertEquals(mock_send.call_count, 5) # Assertion proper logging; note that the - mock_logger.info.assert_called_once_with( - "InfoDomainCommand failed and will be retried Error: Registrar is not logged in.| cltrid is cl_tr_id svtrid is sv_tr_id" + mock_logger.info.assert_any_call( + "InfoDomainCommand failed and will be retried Error: Registrar is not logged in." ) + mock_logger.info.assert_any_call("cltrid is cl_tr_id svtrid is sv_tr_id") @less_console_noise_decorator def fake_failure_send_concurrent_threads(self, command=None, cleaned=None):