[Libguestfs] [libnbd PATCH v2] nbdsh: Catch nbd.Error from -c arguments
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 (at least
on Fedora 32 with abrt-python3-handler installed) to assume the
program crashed from a programming error, and needlessly complicates
clients to have to add try: blocks. Better is if nbdsh itself handles
the problem, and only prints a stack trace when debugging is in
effect, but otherwise just prints the error message. In this way, the
user is not presented with a wall of python stack trace, and 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.
---
On IRC, we decided that printing the stack trace can be useful when
debugging (if -c triggers calls through some deeply-nested python
code), but generally gets in the way for default behavior.
python/nbdsh.py | 20 ++++++++++++++++----
sh/test-error.sh | 23 ++++++++++++++++++++++-
2 files changed, 38 insertions(+), 5 deletions(-)
diff --git a/python/nbdsh.py b/python/nbdsh.py
index 61d38e8..9ed2938 100644
--- a/python/nbdsh.py
+++ b/python/nbdsh.py
@@ -16,6 +16,10 @@
# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
+import os
+import traceback
+
+
# The NBD shell.
def shell():
import argparse
@@ -100,8 +104,16 @@ 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)
+ try:
+ for c in args.command:
+ if c != '-':
+ exec(c, d, d)
+ else:
+ exec(sys.stdin.read(), d, d)
+ except nbd.Error as ex:
+ if os.environ.get("LIBNBD_DEBUG", "0") == "1":
+ traceback.print_exc()
else:
- exec(sys.stdin.read(), d, d)
+ print("nbdsh: command line script failed: %s" % ex.string,
+ file=sys.stderr)
+ sys.exit(1)
diff --git a/sh/test-error.sh b/sh/test-error.sh
index c6ab474..a33ce47 100755
--- a/sh/test-error.sh
+++ b/sh/test-error.sh
@@ -40,6 +40,27 @@ nbdsh -u 'nbd+unix:///?socket=/nosuchsock' >$out 2>$err
&& fail=1
test ! -s $out
cat $err
grep Traceback $err && fail=1
-grep '^nbdsh: unable to connect to uri.*nosuchsock' test-error.err
+grep '^nbdsh: unable to connect to uri.*nosuchsock' $err
+
+# Triggering nbd.Error non-interactively (via -c) prints the error. The
+# output includes the python trace when debugging is enabled (which is
+# the default for our testsuite, when using ./run).
+nbdsh -c 'h.is_read_only()' >$out 2>$err && fail=1
+test ! -s $out
+cat $err
+grep Traceback $err
+grep 'in is_read_only' $err
+grep '^nbd\.Error: nbd_is_read_only: ' $err
+
+# Override ./run's default to show that without debug, the error is succinct.
+nbdsh -c '
+import os
+os.environ["LIBNBD_DEBUG"] = "0"
+h.is_read_only()
+' >$out 2>$err && fail=1
+test ! -s $out
+cat $err
+grep Traceback $err && fail=1
+grep '^nbdsh: command line script failed: nbd_is_read_only: ' $err
exit $fail
--
2.28.0