When using nbdsh for scripting, it is convenient to let nbdsh fail with status 1 when encountering an API failure. However, doing so by letting the nbd.Error exception leak all the way causes ABRT (on Fedora 32 at least) to assume the program crashed from a programming error, and needlessly complicates clients to have to add try: blocks. Better is if nbdsh itself prints the same stack trace that python would, but handles the error and exits python cleanly so that ABRT does not think that the exception was unhandled. See https://github.com/libguestfs/nbdkit/commit/e13048fd9 for an example of client cleanup made more verbose if we don't patch libnbd. --- Is it worth a command-line option, off by default, but that the user can opt in to, where the option being set says we re-raise the excpetion instead of handling it ourselves? python/nbdsh.py | 18 ++++++++++++------ sh/Makefile.am | 4 +++- sh/test-error.sh | 36 ++++++++++++++++++++++++++++++++++++ 3 files changed, 51 insertions(+), 7 deletions(-) create mode 100755 sh/test-error.sh diff --git a/python/nbdsh.py b/python/nbdsh.py index c9a7beb..93d4998 100644 --- a/python/nbdsh.py +++ b/python/nbdsh.py @@ -1,5 +1,5 @@ # NBD client library in userspace -# Copyright (C) 2013-2019 Red Hat Inc. +# Copyright (C) 2013-2020 Red Hat Inc. # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -16,6 +16,9 @@ # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA +import traceback + + # The NBD shell. def shell(): import argparse @@ -95,8 +97,12 @@ help(nbd) # Display documentation else: # https://stackoverflow.com/a/11754346 d = dict(locals(), **globals()) - for c in args.command: - if c != '-': - exec(c, d, d) - else: - exec(sys.stdin.read(), d, d) + try: + for c in args.command: + if c != '-': + exec(c, d, d) + else: + exec(sys.stdin.read(), d, d) + except nbd.Error: + traceback.print_exc() + sys.exit(1) diff --git a/sh/Makefile.am b/sh/Makefile.am index 415e241..8c4b982 100644 --- a/sh/Makefile.am +++ b/sh/Makefile.am @@ -1,5 +1,5 @@ # nbd client library in userspace -# Copyright (C) 2013-2019 Red Hat Inc. +# Copyright (C) 2013-2020 Red Hat Inc. # # This library is free software; you can redistribute it and/or # modify it under the terms of the GNU Lesser General Public @@ -22,6 +22,7 @@ EXTRA_DIST = \ examples/LICENSE-FOR-EXAMPLES \ examples/hexdump.sh \ test-context.sh \ + test-error.sh \ test-help.sh \ test-pattern.sh \ test-version.sh \ @@ -49,6 +50,7 @@ LOG_COMPILER = $(top_builddir)/run TESTS = \ test-help.sh \ test-version.sh \ + test-error.sh \ $(NULL) if HAVE_NBDKIT diff --git a/sh/test-error.sh b/sh/test-error.sh new file mode 100755 index 0000000..d8a1d84 --- /dev/null +++ b/sh/test-error.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +# nbd client library in userspace +# Copyright (C) 2019-2020 Red Hat Inc. +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + +# Test how nbdsh -c handles errors +fail=0 + +. ../tests/functions.sh + +files='test-error.out test-error.err' +cleanup_fn rm -f $files +rm -f $files + +# Triggering nbd.Error non-interactively prints the error +nbdsh -c 'h.is_read_only()' > test-error.out 2> test-error.err && fail=1 +test ! -s test-error.out +cat test-error.err +grep Traceback test-error.err +grep 'in is_read_only' test-error.err +grep '^nbd\.Error: nbd_is_read_only: ' test-error.err + +exit $fail -- 2.28.0