From f09765a03af9fe9576acff4041b342d2ffb6324d Mon Sep 17 00:00:00 2001 From: Darko Poljak Date: Tue, 17 Apr 2018 11:15:28 +0200 Subject: [PATCH] Improve error reporting. --- cdist/__init__.py | 81 +++++++++++++++++++++++++++++++++----------- cdist/config.py | 3 +- cdist/exec/local.py | 6 ++-- cdist/exec/remote.py | 6 ++-- cdist/exec/util.py | 1 + 5 files changed, 68 insertions(+), 29 deletions(-) diff --git a/cdist/__init__.py b/cdist/__init__.py index e6fdfac6..b1b9df9f 100644 --- a/cdist/__init__.py +++ b/cdist/__init__.py @@ -83,41 +83,72 @@ class CdistBetaRequired(cdist.Error): class CdistEntityError(Error): """Something went wrong while executing cdist entity""" - def __init__(self, entity_name, entity_params, stderr_paths, subject=''): + def __init__(self, entity_name, entity_params, stdout_paths, + stderr_paths, subject=''): self.entity_name = entity_name self.entity_params = entity_params self.stderr_paths = stderr_paths + self.stdout_paths = stdout_paths if isinstance(subject, Error): self.original_error = subject else: self.original_error = None self.message = str(subject) - @property - def stderr(self): - output = [] - for stderr_name, stderr_path in self.stderr_paths: - if (os.path.exists(stderr_path) and - os.path.getsize(stderr_path) > 0): - label_begin = '---- BEGIN ' + stderr_name + ':stderr ----' - label_end = '---- END ' + stderr_name + ':stderr ----' - output.append('\n' + label_begin) - with open(stderr_path, 'r') as fd: + def _stdpath(self, stdpaths, header_name): + result = {} + for name, path in stdpaths: + if name not in result: + result[name] = [] + if os.path.exists(path) and os.path.getsize(path) > 0: + output = [] + label_begin = name + ":" + header_name + output.append(label_begin) + output.append('\n') + output.append('-' * len(label_begin)) + output.append('\n') + with open(path, 'r') as fd: output.append(fd.read()) - output.append(label_end) - return '\n'.join(output) + output.append('\n') + result[name].append(''.join(output)) + return result + + def _stderr(self): + return self._stdpath(self.stderr_paths, 'stderr') + + def _stdout(self): + return self._stdpath(self.stdout_paths, 'stdout') + + def _update_dict_list(self, target, source): + for x in source: + if x not in target: + target[x] = [] + target[x].extend(source[x]) + + @property + def std_streams(self): + std_dict = {} + self._update_dict_list(std_dict, self._stdout()) + self._update_dict_list(std_dict, self._stderr()) + return std_dict def __str__(self): output = [] output.append(self.message) - header = "\nError processing " + self.entity_name + output.append('\n\n') + header = "Error processing " + self.entity_name under_header = '=' * len(header) output.append(header) + output.append('\n') output.append(under_header) + output.append('\n') for param_name, param_value in self.entity_params: output.append(param_name + ': ' + str(param_value)) - output.append(self.stderr + '\n') - return '\n'.join(output) + output.append('\n') + output.append('\n') + for x in self.std_streams: + output.append(''.join(self.std_streams[x])) + return ''.join(output) class CdistObjectError(CdistEntityError): @@ -127,28 +158,38 @@ class CdistObjectError(CdistEntityError): ('name', cdist_object.name, ), ('path', cdist_object.absolute_path, ), ('source', " ".join(cdist_object.source), ), - ('type', cdist_object.cdist_type.absolute_path, ), + ('type', os.path.realpath(cdist_object.cdist_type.absolute_path), ), ] stderr_paths = [] for stderr_name in os.listdir(cdist_object.stderr_path): stderr_path = os.path.join(cdist_object.stderr_path, stderr_name) stderr_paths.append((stderr_name, stderr_path, )) + stdout_paths = [] + for stdout_name in os.listdir(cdist_object.stdout_path): + stdout_path = os.path.join(cdist_object.stdout_path, + stdout_name) + stdout_paths.append((stdout_name, stdout_path, )) super().__init__("object '{}'".format(cdist_object.name), - params, stderr_paths, subject) + params, stdout_paths, stderr_paths, subject) class InitialManifestError(CdistEntityError): """Something went wrong while executing initial manifest""" - def __init__(self, initial_manifest, stderr_path, subject=''): + def __init__(self, initial_manifest, stdout_path, stderr_path, subject=''): params = [ ('path', initial_manifest, ), ] + stdout_paths = [] + stdout_paths = [ + ('init', stdout_path, ), + ] stderr_paths = [] stderr_paths = [ ('init', stderr_path, ), ] - super().__init__('initial manifest', params, stderr_paths, subject) + super().__init__('initial manifest', params, stdout_paths, + stderr_paths, subject) def file_to_list(filename): diff --git a/cdist/config.py b/cdist/config.py index 74f68a72..2dcb1005 100644 --- a/cdist/config.py +++ b/cdist/config.py @@ -440,9 +440,10 @@ class Config(object): self.manifest.run_initial_manifest(self.local.initial_manifest) except cdist.Error as e: which = "init" + stdout_path = os.path.join(self.local.stdout_base_path, which) stderr_path = os.path.join(self.local.stderr_base_path, which) raise cdist.InitialManifestError(self.local.initial_manifest, - stderr_path, e) + stdout_path, stderr_path, e) self.iterate_until_finished() self.cleanup() self._remove_files_dirs() diff --git a/cdist/exec/local.py b/cdist/exec/local.py index a50fe072..f83c85df 100644 --- a/cdist/exec/local.py +++ b/cdist/exec/local.py @@ -259,10 +259,8 @@ class Local(object): util.log_std_fd(self.log, command, stderr, 'Local stderr') util.log_std_fd(self.log, command, stdout, 'Local stdout') return output - except subprocess.CalledProcessError as e: - util.handle_called_process_error(e, command) - except OSError as error: - raise cdist.Error(" ".join(command) + ": " + error.args[1]) + except (OSError, subprocess.CalledProcessError) as error: + raise cdist.Error(" ".join(command) + ": " + str(error.args[1])) finally: if message_prefix: message.merge_messages() diff --git a/cdist/exec/remote.py b/cdist/exec/remote.py index b75905ba..4466545e 100644 --- a/cdist/exec/remote.py +++ b/cdist/exec/remote.py @@ -343,10 +343,8 @@ class Remote(object): util.log_std_fd(self.log, command, stdout, 'Remote stdout') return output - except subprocess.CalledProcessError as e: - util.handle_called_process_error(e, command) - except OSError as error: - raise cdist.Error(" ".join(command) + ": " + error.args[1]) + except (OSError, subprocess.CalledProcessError) as error: + raise cdist.Error(" ".join(command) + ": " + str(error.args[1])) except UnicodeDecodeError: raise DecodeError(command) finally: diff --git a/cdist/exec/util.py b/cdist/exec/util.py index 2f2aa38c..c96f757b 100644 --- a/cdist/exec/util.py +++ b/cdist/exec/util.py @@ -127,6 +127,7 @@ def call_get_output(command, env=None, stderr=None): return (_call_get_stdout(command, env, stderr), None) +# Currently not used. def handle_called_process_error(err, command): # Currently, stderr is not captured. # errout = None