[tor-commits] [stem/master] Additional information when stem.util.system.call() fails

[atagar at torproject.org]

commit 5b8b5ca5dc3cd2c0797c10910d6826e274651ff5
Author: Damian Johnson <atagar at torproject.org>
Date:   Thu Jan 28 08:32:15 2016 -0800

    Additional information when stem.util.system.call() fails
    
    Commonly callers at least want the stderr. I ran into this because our
    cache_manual.py error output sucked...
    
      IOError: Unable to run 'a2x -f manpage /tmp/tmpU36UMJ/tor.1.txt': a2x -f
      manpage /tmp/tmpU36UMJ/tor.1.txt returned exit status 1
    
    This says the command twice, and gives no useful informaiton about what the
    error even is. Now it's...
    
      IOError: Unable to run 'a2x -f manpage /tmp/tmpfq6cVA/tor.1.txt': a2x: ERROR:
      /usr/bin/asciidoc --backend docbook -a a2x-format=manpage  --doctype manpage
      --out-file /tmp/tmpfq6cVA/tor.1.xml /tmp/tmpfq6cVA/tor.1.txt returned
      non-zero exit status 1
---
 docs/change_log.rst |    5 +++--
 stem/manual.py      |    4 ++--
 stem/util/system.py |   43 ++++++++++++++++++++++++++++++++++++++-----
 3 files changed, 43 insertions(+), 9 deletions(-)

diff --git a/docs/change_log.rst b/docs/change_log.rst
index b386670..74159e2 100644
--- a/docs/change_log.rst
+++ b/docs/change_log.rst
@@ -72,11 +72,12 @@ The following are only available within Stem's `git repository
  * **Utilities**
 
   * IPv6 support in :func:`~stem.util.connection.get_connections` when resolving with proc, netstat, lsof, or ss (:trac:`18079`)
-  * Added :func:`~stem.util.__init__.datetime_to_unix`
   * The 'ss' connection resolver didn't work on Gentoo (:trac:`18079`)
   * Recognize IPv4-mapped IPv6 addresses in our utils (:trac:`18079`)
-  * Added an **is_ipv6** value to :class:`~stem.util.connection.Connection` instances
   * Allow :func:`stem.util.conf.Config.set` to remove values when provided with a **None** value
+  * Additional information when :func:`~stem.util.system.call` fails through a :class:`~stem.util.system.CallError`
+  * Added an **is_ipv6** value to :class:`~stem.util.connection.Connection` instances
+  * Added :func:`~stem.util.__init__.datetime_to_unix`
 
  * **Interpreter**
 
diff --git a/stem/manual.py b/stem/manual.py
index 55c94a2..31f9ed1 100644
--- a/stem/manual.py
+++ b/stem/manual.py
@@ -245,8 +245,8 @@ def download_man_page(path = None, file_handle = None, url = GITWEB_MANUAL_URL,
 
       if not os.path.exists(manual_path):
         raise OSError('no man page was generated')
-    except OSError as exc:
-      raise IOError("Unable to run 'a2x -f manpage %s': %s" % (asciidoc_path, exc))
+    except stem.util.system.CallError as exc:
+      raise IOError("Unable to run '%s': %s" % (exc.command, exc.stderr))
 
     if path:
       try:
diff --git a/stem/util/system.py b/stem/util/system.py
index 318b7e4..14516b2 100644
--- a/stem/util/system.py
+++ b/stem/util/system.py
@@ -126,6 +126,32 @@ _PROCESS_NAME = None
 _MAX_NAME_LENGTH = -1
 
 
+class CallError(OSError):
+  """
+  Error response when making a system call. This is an **OSError** subclass
+  with additional information about the process. Depending on the nature of the
+  error not all of these attributes will be available.
+
+  :var str msg: exception string
+  :var str command: command that was ran
+  :var int exit_status: exit code of the process
+  :var float runtime: time the command took to run
+  :var str stdout: stdout of the process
+  :var str stderr: stderr of the process
+  """
+
+  def __init__(self, msg, command, exit_status, runtime, stdout, stderr):
+    self.msg = msg
+    self.command = command
+    self.exit_status = exit_status
+    self.runtime = runtime
+    self.stdout = stdout
+    self.stderr = stderr
+
+  def __str__(self):
+    return self.msg
+
+
 def is_windows():
   """
   Checks if we are running on Windows.
@@ -960,6 +986,10 @@ def call(command, default = UNDEFINED, ignore_exit_status = False, env = None):
   are not permitted.
 
   .. versionchanged:: 1.5.0
+     Providing additional information upon failure by raising a CallError. This
+     is a subclass of OSError, providing backward compatibility.
+
+  .. versionchanged:: 1.5.0
      Added env argument.
 
   :param str,list command: command to be issued
@@ -970,7 +1000,8 @@ def call(command, default = UNDEFINED, ignore_exit_status = False, env = None):
 
   :returns: **list** with the lines of output from the command
 
-  :raises: **OSError** if this fails and no default was provided
+  :raises: **CallError** if this fails and no default was provided, this is an
+    **OSError** subclass
   """
 
   if isinstance(command, str):
@@ -978,6 +1009,8 @@ def call(command, default = UNDEFINED, ignore_exit_status = False, env = None):
   else:
     command_list = command
 
+  exit_status, runtime, stdout, stderr = None, None, None, None
+
   try:
     is_shell_command = command_list[0] in SHELL_COMMANDS
 
@@ -998,10 +1031,10 @@ def call(command, default = UNDEFINED, ignore_exit_status = False, env = None):
     elif stderr:
       log.trace(trace_prefix + ', stderr:\n%s' % stderr)
 
-    exit_code = process.poll()
+    exit_status = process.poll()
 
-    if not ignore_exit_status and exit_code != 0:
-      raise OSError('%s returned exit status %i' % (command, exit_code))
+    if not ignore_exit_status and exit_status != 0:
+      raise OSError('%s returned exit status %i' % (command, exit_status))
 
     if stdout:
       return stdout.decode('utf-8', 'replace').splitlines()
@@ -1013,7 +1046,7 @@ def call(command, default = UNDEFINED, ignore_exit_status = False, env = None):
     if default != UNDEFINED:
       return default
     else:
-      raise
+      raise CallError(str(exc), ' '.join(command_list), exit_status, runtime, stdout, stderr)
 
 
 def get_process_name():