From bcf9b2ea3ce6a6ab178055c6d4e2106412f875ed Mon Sep 17 00:00:00 2001 From: Nathaniel Case Date: Fri, 23 Jul 2021 11:53:34 -0400 Subject: [PATCH] Handle failures in ansible-connection instead of returning None --- ...75313-ansible-connection-returns-none.yaml | 3 ++ .../scripts/ansible_connection_cli_stub.py | 41 +++++++++++-------- lib/ansible/utils/jsonrpc.py | 18 ++++++++ 3 files changed, 44 insertions(+), 18 deletions(-) create mode 100644 changelogs/fragments/75313-ansible-connection-returns-none.yaml diff --git a/changelogs/fragments/75313-ansible-connection-returns-none.yaml b/changelogs/fragments/75313-ansible-connection-returns-none.yaml new file mode 100644 index 00000000000..14fbee7cbc4 --- /dev/null +++ b/changelogs/fragments/75313-ansible-connection-returns-none.yaml @@ -0,0 +1,3 @@ +--- +bugfixes: +- ansible-connection - handle errors when passing methods to persistent connection instead of closing connection (https://github.com/ansible-collections/ansible.netcommon/issues/301). diff --git a/lib/ansible/cli/scripts/ansible_connection_cli_stub.py b/lib/ansible/cli/scripts/ansible_connection_cli_stub.py index b1ed18c9c69..e3fdac91b8d 100755 --- a/lib/ansible/cli/scripts/ansible_connection_cli_stub.py +++ b/lib/ansible/cli/scripts/ansible_connection_cli_stub.py @@ -135,31 +135,36 @@ class ConnectionProcess(object): self.exception = None (s, addr) = self.sock.accept() - signal.alarm(0) - signal.signal(signal.SIGALRM, self.command_timeout) - while True: - data = recv_data(s) - if not data: - break + try: + signal.alarm(0) + signal.signal(signal.SIGALRM, self.command_timeout) + while True: + data = recv_data(s) + if not data: + break - if log_messages: - display.display("jsonrpc request: %s" % data, log_only=True) + if log_messages: + display.display("jsonrpc request: %s" % data, log_only=True) - request = json.loads(to_text(data, errors='surrogate_or_strict')) - if request.get('method') == "exec_command" and not self.connection.connected: - self.connection._connect() + request = json.loads(to_text(data, errors='surrogate_or_strict')) + if request.get('method') == "exec_command" and not self.connection.connected: + self.connection._connect() - signal.alarm(self.connection.get_option('persistent_command_timeout')) + signal.alarm(self.connection.get_option('persistent_command_timeout')) - resp = self.srv.handle_request(data) - signal.alarm(0) + resp = self.srv.handle_request(data) + signal.alarm(0) - if log_messages: - display.display("jsonrpc response: %s" % resp, log_only=True) + if log_messages: + display.display("jsonrpc response: %s" % resp, log_only=True) - send_data(s, to_bytes(resp)) + send_data(s, to_bytes(resp)) - s.close() + except Exception as exc: + error = self.srv.make_error(data, to_text(exc)) + send_data(s, to_bytes(error)) + finally: + s.close() except Exception as e: # socket.accept() will raise EINTR if the socket.close() is called diff --git a/lib/ansible/utils/jsonrpc.py b/lib/ansible/utils/jsonrpc.py index 2af8bd3581e..95e081a47b7 100644 --- a/lib/ansible/utils/jsonrpc.py +++ b/lib/ansible/utils/jsonrpc.py @@ -72,6 +72,24 @@ class JsonRpcServer(object): return response + def make_error(self, request, error_msg): + """Produce a correct error payload based on request + + This is here to allow a caller to produce an error response with the + correct format and response id outside of handle_request, say if an + error has occurred before handle_request could be called. + """ + request = json.loads(to_text(request, errors='surrogate_then_replace')) + + setattr(self, '_identifier', request.get('id')) + + error = self.internal_error(data=to_text(error_msg)) + response = json.dumps(error) + + delattr(self, '_identifier') + + return response + def register(self, obj): self._objects.add(obj)