From 0c1878786b75832d6375ad25c7955e9561c2877b Mon Sep 17 00:00:00 2001 From: Douglas Raillard Date: Fri, 20 Aug 2021 15:54:46 +0100 Subject: [PATCH] Target.execute(): Add .returncode and .output exception attributes Add Target{Stable,Transient}CalledProcessError exceptions, with an .returncode and .output attributes, raised by Target.execute(), mirroring subprocess.CalledProcessError. This is very useful in client code that uses "exit N" to signal an abnormal condition, and then inspects the output to find out more. --- devlib/exception.py | 38 ++++++++++++++++++++++++++++++ devlib/host.py | 18 ++++++++------ devlib/utils/android.py | 23 ++++++++++++++---- devlib/utils/misc.py | 2 +- devlib/utils/ssh.py | 52 ++++++++++++++++++++++++++++++----------- 5 files changed, 107 insertions(+), 26 deletions(-) diff --git a/devlib/exception.py b/devlib/exception.py index 7776c86..a7884c8 100644 --- a/devlib/exception.py +++ b/devlib/exception.py @@ -13,6 +13,8 @@ # limitations under the License. # +import subprocess + class DevlibError(Exception): """Base class for all Devlib exceptions.""" @@ -69,6 +71,42 @@ class TargetStableError(TargetError, DevlibStableError): pass +class TargetCalledProcessError(subprocess.CalledProcessError, TargetError): + """Exception raised when a command executed on the target fails.""" + def __str__(self): + msg = super().__str__() + def decode(s): + try: + s = s.decode() + except AttributeError: + s = str(s) + + return s.strip() + + if self.stdout is not None and self.stderr is None: + out = ['OUTPUT: {}'.format(decode(self.output))] + else: + out = [ + 'STDOUT: {}'.format(decode(self.output)) if self.output is not None else '', + 'STDERR: {}'.format(decode(self.stderr)) if self.stderr is not None else '', + ] + + return '\n'.join(( + msg, + *out, + )) + + +class TargetStableCalledProcessError(TargetCalledProcessError, TargetStableError): + """Variant of :exc:`devlib.exception.TargetCalledProcessError` that indicates a stable error""" + pass + + +class TargetTransientCalledProcessError(TargetCalledProcessError, TargetTransientError): + """Variant of :exc:`devlib.exception.TargetCalledProcessError` that indicates a transient error""" + pass + + class TargetNotRespondingError(TargetTransientError): """The target is unresponsive.""" pass diff --git a/devlib/host.py b/devlib/host.py index 62b85ed..b2a566a 100644 --- a/devlib/host.py +++ b/devlib/host.py @@ -22,7 +22,10 @@ from distutils.dir_util import copy_tree from getpass import getpass from pipes import quote -from devlib.exception import TargetTransientError, TargetStableError +from devlib.exception import ( + TargetTransientError, TargetStableError, + TargetTransientCalledProcessError, TargetStableCalledProcessError +) from devlib.utils.misc import check_output from devlib.connection import ConnectionBase, PopenBackgroundCommand @@ -102,12 +105,13 @@ class LocalConnection(ConnectionBase): try: stdout, stderr = check_output(command, shell=True, timeout=timeout, ignore=ignore) except subprocess.CalledProcessError as e: - message = 'Got exit code {}\nfrom: {}\nOUTPUT: {}'.format( - e.returncode, command, e.output) - if will_succeed: - raise TargetTransientError(message) - else: - raise TargetStableError(message) + cls = TargetTransientCalledProcessError if will_succeed else TargetStableCalledProcessError + raise cls( + e.returncode, + command, + e.output, + e.stderr, + ) # Remove the one-character prompt of sudo -S -p if use_sudo and stderr: diff --git a/devlib/utils/android.py b/devlib/utils/android.py index 018d3d2..9f5083a 100755 --- a/devlib/utils/android.py +++ b/devlib/utils/android.py @@ -40,7 +40,7 @@ try: except ImportError: from pipes import quote -from devlib.exception import TargetTransientError, TargetStableError, HostError +from devlib.exception import TargetTransientError, TargetStableError, HostError, TargetTransientCalledProcessError, TargetStableCalledProcessError from devlib.utils.misc import check_output, which, ABI_MAP, redirect_streams, get_subprocess from devlib.connection import ConnectionBase, AdbBackgroundCommand, PopenBackgroundCommand, PopenTransferManager @@ -336,6 +336,14 @@ class AdbConnection(ConnectionBase): try: return adb_shell(self.device, command, timeout, check_exit_code, as_root, adb_server=self.adb_server, su_cmd=self.su_cmd) + except subprocess.CalledProcessError as e: + cls = TargetTransientCalledProcessError if will_succeed else TargetStableCalledProcessError + raise cls( + e.returncode, + command, + e.output, + e.stderr, + ) except TargetStableError as e: if will_succeed: raise TargetTransientError(e) @@ -559,10 +567,15 @@ def adb_shell(device, command, timeout=None, check_exit_code=False, exit_code = exit_code.strip() re_search = AM_START_ERROR.findall(output) if exit_code.isdigit(): - if int(exit_code): - message = ('Got exit code {}\nfrom target command: {}\n' - 'OUTPUT: {}\nSTDERR: {}\n') - raise TargetStableError(message.format(exit_code, command, output, error)) + exit_code = int(exit_code) + if exit_code: + raise subprocess.CalledProcessError( + exit_code, + command, + output, + error, + ) + elif re_search: message = 'Could not start activity; got the following:\n{}' raise TargetStableError(message.format(re_search[0])) diff --git a/devlib/utils/misc.py b/devlib/utils/misc.py index 752b758..de4944b 100644 --- a/devlib/utils/misc.py +++ b/devlib/utils/misc.py @@ -197,7 +197,7 @@ def check_subprocess_output(process, timeout=None, ignore=None, inputtext=None): retcode = process.poll() if retcode and ignore != 'all' and retcode not in ignore: - raise subprocess.CalledProcessError(retcode, process.args, output='\n'.join([output, error])) + raise subprocess.CalledProcessError(retcode, process.args, output, error) return output, error diff --git a/devlib/utils/ssh.py b/devlib/utils/ssh.py index cc78624..2586d62 100644 --- a/devlib/utils/ssh.py +++ b/devlib/utils/ssh.py @@ -52,7 +52,10 @@ from pexpect import EOF, TIMEOUT, spawn # pylint: disable=redefined-builtin,wrong-import-position from devlib.exception import (HostError, TargetStableError, TargetNotRespondingError, - TimeoutError, TargetTransientError) + TimeoutError, TargetTransientError, + TargetCalledProcessError, + TargetTransientCalledProcessError, + TargetStableCalledProcessError) from devlib.utils.misc import (which, strip_bash_colors, check_output, sanitize_cmd_template, memoized, redirect_streams) from devlib.utils.types import boolean @@ -524,6 +527,8 @@ class SshConnection(SshConnectionBase): try: with _handle_paramiko_exceptions(command): exit_code, output = self._execute(command, timeout, as_root, strip_colors) + except TargetCalledProcessError: + raise except TargetStableError as e: if will_succeed: raise TargetTransientError(e) @@ -531,8 +536,13 @@ class SshConnection(SshConnectionBase): raise else: if check_exit_code and exit_code: - message = 'Got exit code {}\nfrom: {}\nOUTPUT: {}' - raise TargetStableError(message.format(exit_code, command, output)) + cls = TargetTransientCalledProcessError if will_succeed else TargetStableCalledProcessError + raise cls( + exit_code, + command, + output, + None, + ) return output def background(self, command, stdout=subprocess.PIPE, stderr=subprocess.PIPE, as_root=False): @@ -843,16 +853,23 @@ class TelnetConnection(SshConnectionBase): if check_exit_code: try: exit_code = int(exit_code_text) - if exit_code: - message = 'Got exit code {}\nfrom: {}\nOUTPUT: {}' - raise TargetStableError(message.format(exit_code, command, output)) except (ValueError, IndexError): - logger.warning( + raise ValueError( 'Could not get exit code for "{}",\ngot: "{}"'\ .format(command, exit_code_text)) + if exit_code: + cls = TargetTransientCalledProcessError if will_succeed else TargetStableCalledProcessError + raise cls( + exit_code, + command, + output, + None, + ) return output except EOF: raise TargetNotRespondingError('Connection lost.') + except TargetCalledProcessError: + raise except TargetStableError as e: if will_succeed: raise TargetTransientError(e) @@ -1130,7 +1147,10 @@ class Gem5Connection(TelnetConnection): try: output = self._gem5_shell(command, check_exit_code=check_exit_code, - as_root=as_root) + as_root=as_root, + will_succeed=will_succeed) + except TargetCalledProcessError: + raise except TargetStableError as e: if will_succeed: raise TargetTransientError(e) @@ -1364,7 +1384,7 @@ class Gem5Connection(TelnetConnection): raise TargetStableError('Path to m5 binary on simulated system is not set!') self._gem5_shell('{} {}'.format(self.m5_path, command)) - def _gem5_shell(self, command, as_root=False, timeout=None, check_exit_code=True, sync=True): # pylint: disable=R0912 + def _gem5_shell(self, command, as_root=False, timeout=None, check_exit_code=True, sync=True, will_succeed=False): # pylint: disable=R0912 """ Execute a command in the gem5 shell @@ -1429,11 +1449,17 @@ class Gem5Connection(TelnetConnection): sync=False) try: exit_code = int(exit_code_text.split()[0]) - if exit_code: - message = 'Got exit code {}\nfrom: {}\nOUTPUT: {}' - raise TargetStableError(message.format(exit_code, command, output)) except (ValueError, IndexError): - gem5_logger.warning('Could not get exit code for "{}",\ngot: "{}"'.format(command, exit_code_text)) + raise ValueError('Could not get exit code for "{}",\ngot: "{}"'.format(command, exit_code_text)) + else: + if exit_code: + cls = TragetTransientCalledProcessError if will_succeed else TargetStableCalledProcessError + raise cls( + exit_code, + command, + output, + None, + ) return output