From 44011942ef8376985dde24ebebe2b60e622737dd Mon Sep 17 00:00:00 2001 From: Thomas Mangin Date: Sun, 26 Apr 2020 21:57:16 +0100 Subject: util: T2226: better handle stderr Do not agreggate stderr with stdout. So if a command reports a message on stderr but does not report an error, it will not be send to the user to confuse him. Explicitely set encoding to utf-8, which does not change the code behaviour but simplify the code. --- python/vyos/util.py | 45 +++++++++++++++++++++++++++------------------ 1 file changed, 27 insertions(+), 18 deletions(-) diff --git a/python/vyos/util.py b/python/vyos/util.py index 4340332d3..f416e79f9 100644 --- a/python/vyos/util.py +++ b/python/vyos/util.py @@ -14,6 +14,7 @@ # License along with this library. If not, see . import os +import sys # # NOTE: Do not import full classes here, move your import to the function @@ -25,7 +26,7 @@ import os # which all have slighty different behaviour from subprocess import Popen, PIPE, STDOUT, DEVNULL def popen(command, flag='', shell=None, input=None, timeout=None, env=None, - stdout=PIPE, stderr=None, decode=None): + stdout=PIPE, stderr=PIPE, decode='utf-8'): """ popen is a wrapper helper aound subprocess.Popen with it default setting it will return a tuple (out, err) @@ -48,6 +49,7 @@ def popen(command, flag='', shell=None, input=None, timeout=None, env=None, - STDOUT, send the data to be merged with stdout - DEVNULL, discard the output decode: specify the expected text encoding (utf-8, ascii, ...) + the default is explicitely utf-8 which is python's own default usage: to get both stdout, and stderr: popen('command', stdout=PIPE, stderr=STDOUT) @@ -77,27 +79,34 @@ def popen(command, flag='', shell=None, input=None, timeout=None, env=None, stdin=stdin, stdout=stdout, stderr=stderr, env=env, shell=use_shell, ) - tmp = p.communicate(input, timeout) - out1 = b'' - out2 = b'' + + pipe = p.communicate(input, timeout) + + pipe_out = b'' if stdout == PIPE: - out1 = tmp[0] + pipe_out = pipe[0] + + pipe_err = b'' if stderr == PIPE: - out2 += tmp[1] - decoded1 = out1.decode(decode) if decode else out1.decode() - decoded2 = out2.decode(decode) if decode else out2.decode() - decoded1 = decoded1.replace('\r\n', '\n').strip() - decoded2 = decoded2.replace('\r\n', '\n').strip() - nl = '\n' if decoded1 and decoded2 else '' - decoded = decoded1 + nl + decoded2 - if decoded: - ret_msg = f"returned:\n{decoded}" + pipe_err = pipe[1] + + str_out = pipe_out.decode(decode).replace('\r\n', '\n').strip() + str_err = pipe_err.decode(decode).replace('\r\n', '\n').strip() + + if str_out: + ret_msg = f"returned (out):\n{str_out}" debug.message(ret_msg, flag) - return decoded, p.returncode + + if str_err: + ret_msg = f"returned (err):\n{str_err}" + # this message will also be send to syslog via airbag + debug.message(ret_msg, flag, destination=sys.stderr) + + return str_out, p.returncode def run(command, flag='', shell=None, input=None, timeout=None, env=None, - stdout=DEVNULL, stderr=None, decode=None): + stdout=DEVNULL, stderr=PIPE, decode='utf-8'): """ A wrapper around vyos.util.popen, which discard the stdout and will return the error code of a command @@ -113,7 +122,7 @@ def run(command, flag='', shell=None, input=None, timeout=None, env=None, def cmd(command, flag='', shell=None, input=None, timeout=None, env=None, - stdout=PIPE, stderr=None, decode=None, + stdout=PIPE, stderr=PIPE, decode='utf-8', raising=None, message=''): """ A wrapper around vyos.util.popen, which returns the stdout and @@ -143,7 +152,7 @@ def cmd(command, flag='', shell=None, input=None, timeout=None, env=None, def call(command, flag='', shell=None, input=None, timeout=None, env=None, - stdout=PIPE, stderr=None, decode=None): + stdout=PIPE, stderr=PIPE, decode='utf-8'): """ A wrapper around vyos.util.popen, which print the stdout and will return the error code of a command -- cgit v1.2.3 From e768f52b45e3f0d354c788c38d7dfc9964c4d8aa Mon Sep 17 00:00:00 2001 From: Thomas Mangin Date: Sun, 26 Apr 2020 22:00:26 +0100 Subject: util: T2226: expected return code for cmd add an option to cmd() allowing to define a list of error codes which are expected when runnin the command. the default is only to expect zero (no error). --- python/vyos/util.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/python/vyos/util.py b/python/vyos/util.py index f416e79f9..8430799a1 100644 --- a/python/vyos/util.py +++ b/python/vyos/util.py @@ -123,13 +123,14 @@ def run(command, flag='', shell=None, input=None, timeout=None, env=None, def cmd(command, flag='', shell=None, input=None, timeout=None, env=None, stdout=PIPE, stderr=PIPE, decode='utf-8', - raising=None, message=''): + raising=None, message='', expect=[0]): """ A wrapper around vyos.util.popen, which returns the stdout and will raise the error code of a command raising: specify which call should be used when raising (default is OSError) the class should only require a string as parameter + expect: a list of error codes to consider as normal """ decoded, code = popen( command, flag, @@ -138,7 +139,7 @@ def cmd(command, flag='', shell=None, input=None, timeout=None, env=None, env=env, shell=shell, decode=decode, ) - if code != 0: + if code not in expect: feedback = message + '\n' if message else '' feedback += f'failed to run command: {command}\n' feedback += f'returned: {decoded}\n' -- cgit v1.2.3 From 4e86c7aaa32570e8785a6caa3614c570bcd038a7 Mon Sep 17 00:00:00 2001 From: Thomas Mangin Date: Sun, 26 Apr 2020 22:11:12 +0100 Subject: util: T2226: a way to report noteworthy event debug.noteworthy can be used to record noteworhy event during the lifetime of the program. Should anything then cause the program to fail and cause an airbag report to the user, then this information will also be included. --- python/vyos/airbag.py | 17 +++++++++++++++++ python/vyos/util.py | 14 ++++++++++---- 2 files changed, 27 insertions(+), 4 deletions(-) diff --git a/python/vyos/airbag.py b/python/vyos/airbag.py index 6698aa404..b7838d8a2 100644 --- a/python/vyos/airbag.py +++ b/python/vyos/airbag.py @@ -26,6 +26,17 @@ from vyos.version import get_full_version_data DISABLE = False +_noteworthy = [] + +def noteworthy(msg): + """ + noteworthy can be use to take note things which we may not want to + report to the user may but be worth including in bug report + if something goes wrong later on + """ + _noteworthy.append(msg) + + # emulate a file object class _IO(object): def __init__(self, std, log): @@ -58,11 +69,16 @@ def bug_report(dtype, value, trace): information = get_full_version_data() trace = '\n'.join(format_exception(dtype, value, trace)).replace('\n\n','\n') + note = '' + if _noteworthy: + note = 'noteworthy:\n' + note += '\n'.join(_noteworthy) information.update({ 'date': datetime.now().strftime('%Y-%m-%d %H:%M:%S'), 'trace': trace, 'instructions': COMMUNITY if 'rolling' in get_version() else SUPPORTED, + 'note': note, }) sys.stdout.write(INTRO.format(**information)) @@ -145,6 +161,7 @@ Hardware S/N: {hardware_serial} Hardware UUID: {hardware_uuid} {trace} +{note} """ INTRO = """\ diff --git a/python/vyos/util.py b/python/vyos/util.py index 8430799a1..504d36ef8 100644 --- a/python/vyos/util.py +++ b/python/vyos/util.py @@ -56,6 +56,7 @@ def popen(command, flag='', shell=None, input=None, timeout=None, env=None, to discard stdout and get stderr: popen('command', stdout=DEVNUL, stderr=PIPE) """ from vyos import debug + from vyos import airbag # log if the flag is set, otherwise log if command is set if not debug.enabled(flag): flag = 'command' @@ -93,14 +94,19 @@ def popen(command, flag='', shell=None, input=None, timeout=None, env=None, str_out = pipe_out.decode(decode).replace('\r\n', '\n').strip() str_err = pipe_err.decode(decode).replace('\r\n', '\n').strip() + out_msg = f"returned (out):\n{str_out}" if str_out: - ret_msg = f"returned (out):\n{str_out}" - debug.message(ret_msg, flag) + debug.message(out_msg, flag) if str_err: - ret_msg = f"returned (err):\n{str_err}" + err_msg = f"returned (err):\n{str_err}" # this message will also be send to syslog via airbag - debug.message(ret_msg, flag, destination=sys.stderr) + debug.message(err_msg, flag, destination=sys.stderr) + + # should something go wrong, report this too via airbag + airbag.noteworthy(cmd_msg) + airbag.noteworthy(out_msg) + airbag.noteworthy(err_msg) return str_out, p.returncode -- cgit v1.2.3