[Libguestfs] [libnbd PATCH v2] nbdsh: Catch nbd.Error from -c arguments

[Eric Blake]

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