diff --git a/src/epplibwrapper/client.py b/src/epplibwrapper/client.py index b6359d494..e4b7a5d53 100644 --- a/src/epplibwrapper/client.py +++ b/src/epplibwrapper/client.py @@ -111,21 +111,21 @@ class EPPLibWrapper: self.start_connection_pool() except (ValueError, ParsingError) as err: message = f"{cmd_type} failed to execute due to some syntax error." - logger.error(message, exc_info=True) + logger.error(f"{message} Error: {err}", exc_info=True) raise RegistryError(message) from err except TransportError as err: message = f"{cmd_type} failed to execute due to a connection error." - logger.error(message, exc_info=True) + logger.error(f"{message} Error: {err}", exc_info=True) raise RegistryError(message) from err except LoginError as err: - # For linter + # For linter due to it not liking this line length text = "failed to execute due to a registry login error." message = f"{cmd_type} {text}" - logger.error(message, exc_info=True) + logger.error(f"{message} Error: {err}", exc_info=True) raise RegistryError(message) from err except Exception as err: message = f"{cmd_type} failed to execute due to an unknown error." - logger.error(message, exc_info=True) + logger.error(f"{message} Error: {err}", exc_info=True) raise RegistryError(message) from err else: if response.code >= 2000: @@ -155,6 +155,8 @@ class EPPLibWrapper: except RegistryError as err: raise err finally: + # Code execution will halt after here. + # The end user will need to recall .send. self.start_connection_pool() counter = 0 # we'll try 3 times diff --git a/src/epplibwrapper/socket.py b/src/epplibwrapper/socket.py index c44d07910..6040f6682 100644 --- a/src/epplibwrapper/socket.py +++ b/src/epplibwrapper/socket.py @@ -48,7 +48,7 @@ class Socket: def send(self, command): """Sends a command to the registry. - If the response code is >= 2000, + If the RegistryError code is >= 2000, then this function raises a LoginError. The calling function should handle this.""" response = self.client.send(command) @@ -59,7 +59,9 @@ class Socket: return response def is_login_error(self, code): - """Returns the result of code >= 2000""" + """Returns the result of code >= 2000 for RegistryError. + This indicates that something weird happened on the Registry, + and that we should return a LoginError.""" return code >= 2000 def test_connection_success(self): @@ -90,7 +92,7 @@ class Socket: # If we encounter a login error, fail if self.is_login_error(response.code): - logger.warning("was login error") + logger.warning("A login error was found in test_connection_success") return False # Otherwise, just return true diff --git a/src/epplibwrapper/tests/test_pool.py b/src/epplibwrapper/tests/test_pool.py index 3a431ef1e..4e919ba76 100644 --- a/src/epplibwrapper/tests/test_pool.py +++ b/src/epplibwrapper/tests/test_pool.py @@ -28,6 +28,7 @@ class TestConnectionPool(TestCase): """Tests for our connection pooling behaviour""" def setUp(self): + # Mimic the settings added to settings.py self.pool_options = { # Current pool size "size": 1, diff --git a/src/epplibwrapper/utility/pool.py b/src/epplibwrapper/utility/pool.py index 99d5326ab..8979c9744 100644 --- a/src/epplibwrapper/utility/pool.py +++ b/src/epplibwrapper/utility/pool.py @@ -51,7 +51,8 @@ class EPPConnectionPool(ConnectionPool): self.keepalive = options["keepalive"] # Determines the period in which new - # gevent threads are spun up + # gevent threads are spun up. + # This time period is in seconds. So for instance, .1 would be .1 seconds. self.spawn_frequency = 0.1 if "spawn_frequency" in options: self.spawn_frequency = options["spawn_frequency"] @@ -77,7 +78,7 @@ class EPPConnectionPool(ConnectionPool): def _keepalive(self, c): """Sends a command to the server to keep the connection alive.""" try: - # Sends a ping to EPPLib + # Sends a ping to the registry via EPPLib c.send(Hello()) except Exception as err: message = "Failed to keep the connection alive." @@ -108,7 +109,7 @@ class EPPConnectionPool(ConnectionPool): logger.info("No connections to kill.") except Exception as err: logger.error("Could not kill all connections.") - raise err + raise PoolError(code=PoolErrorCodes.KILL_ALL_FAILED) from err def populate_all_connections(self): """Generates the connection pool. diff --git a/src/epplibwrapper/utility/pool_error.py b/src/epplibwrapper/utility/pool_error.py index 70312f32e..16aa0e08d 100644 --- a/src/epplibwrapper/utility/pool_error.py +++ b/src/epplibwrapper/utility/pool_error.py @@ -22,12 +22,20 @@ class PoolError(Exception): - 2000 KILL_ALL_FAILED - 2001 NEW_CONNECTION_FAILED - 2002 KEEP_ALIVE_FAILED + + Note: These are separate from the error codes returned from EppLib """ - # For linter - kill_failed = "Could not kill all connections." - conn_failed = "Failed to execute due to a registry error." - alive_failed = "Failed to keep the connection alive." + # Used variables due to linter requirements + kill_failed = "Could not kill all connections. Are multiple pools running?" + conn_failed = ( + "Failed to execute due to a registry error." + " See previous logs to determine the cause of the error." + ) + alive_failed = ( + "Failed to keep the connection alive. " + "It is likely that the registry returned a LoginError." + ) _error_mapping = { PoolErrorCodes.KILL_ALL_FAILED: kill_failed, PoolErrorCodes.NEW_CONNECTION_FAILED: conn_failed, diff --git a/src/epplibwrapper/utility/pool_status.py b/src/epplibwrapper/utility/pool_status.py index 214bf8ac1..64ebbe5eb 100644 --- a/src/epplibwrapper/utility/pool_status.py +++ b/src/epplibwrapper/utility/pool_status.py @@ -1,5 +1,10 @@ class PoolStatus: - """A list of Booleans to keep track of Pool Status""" + """A list of Booleans to keep track of Pool Status. + + pool_running -> bool: Tracks if the pool itself is active or not. + connection_success -> bool: Tracks if connection is possible with the registry. + pool_hanging -> pool: Tracks if the pool has exceeded its timeout period. + """ def __init__(self): self.pool_running = False diff --git a/src/registrar/config/settings.py b/src/registrar/config/settings.py index e4c4ae1f8..385f2a1e3 100644 --- a/src/registrar/config/settings.py +++ b/src/registrar/config/settings.py @@ -536,11 +536,12 @@ SECRET_REGISTRY_HOSTNAME = secret_registry_hostname # Use this variable to set the size of our connection pool in client.py # WARNING: Setting this value too high could cause frequent app crashes! +# Having too many connections open could cause the sandbox to timeout, +# as the spinup time could exceed the timeout time. EPP_CONNECTION_POOL_SIZE = 1 # Determines the interval in which we ping open connections in seconds # Calculated as POOL_KEEP_ALIVE / EPP_CONNECTION_POOL_SIZE -# WARNING: Setting this value too high could cause frequent app crashes! POOL_KEEP_ALIVE = 60 # Determines how long we try to keep a pool alive for,