Repository: tomerfiliba/plumbum

Issue:
ProcessExecutionError is misleading when piped commands fail
Relates to #145 

```
Type "help", "copyright", "credits" or "license" for more information.
>>> from plumbum.cmd import cat, head
>>> from plumbum import FG
>>> (cat['/dev/urndom'] | head['-c', '10']) & FG()
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/nail/home/josnyder/consul-backups/env/local/lib/python2.7/site-packages/plumbum/commands/modifiers.py", line 143, in __rand__
    cmd(retcode = self.retcode, stdin = None, stdout = None, stderr = None)
  File "/nail/home/josnyder/consul-backups/env/local/lib/python2.7/site-packages/plumbum/commands/base.py", line 89, in __call__
    return self.run(args, **kwargs)[1]
  File "/nail/home/josnyder/consul-backups/env/local/lib/python2.7/site-packages/plumbum/commands/base.py", line 219, in run
    return p.run()
  File "/nail/home/josnyder/consul-backups/env/local/lib/python2.7/site-packages/plumbum/commands/base.py", line 181, in runner
    return run_proc(p, retcode, timeout)
  File "/nail/home/josnyder/consul-backups/env/local/lib/python2.7/site-packages/plumbum/commands/processes.py", line 217, in run_proc
    return _check_process(proc, retcode, timeout, stdout, stderr)
  File "/nail/home/josnyder/consul-backups/env/local/lib/python2.7/site-packages/plumbum/commands/processes.py", line 34, in _check_process
    stdout, stderr)
plumbum.commands.processes.ProcessExecutionError: Command line: ['/usr/bin/head', '-c', '10']
Exit code: 1
```

In this example, the process which failed is the cat process.  The exception message is misleading, because it implies that the head process failed.

Discussion:

Patch:
diff --git a/plumbum/cli/progress.py b/plumbum/cli/progress.py
index 3ca68b2..080c80a 100644
--- a/plumbum/cli/progress.py
+++ b/plumbum/cli/progress.py
@@ -103,9 +103,9 @@ class ProgressBase(six.ABC):
         pass
 
     @classmethod
-    def range(cls, *value, **kargs):
+    def range(cls, value, **kargs):
         """Fast shortcut to create a range based progress bar, assumes work done in body"""
-        return cls(range(*value), body=True, **kargs)
+        return cls(range(value), value, body=True, **kargs)
 
     @classmethod
     def wrap(cls, iterator, length=None, **kargs):
diff --git a/plumbum/commands/base.py b/plumbum/commands/base.py
index f20ee15..900f0ce 100644
--- a/plumbum/commands/base.py
+++ b/plumbum/commands/base.py
@@ -5,6 +5,7 @@ from plumbum.commands.processes import run_proc, iter_lines
 from plumbum.lib import six
 from tempfile import TemporaryFile
 from subprocess import PIPE, Popen
+from types import MethodType
 
 
 class RedirectionError(Exception):
@@ -296,6 +297,16 @@ class Pipeline(BaseCommand):
             dstproc.returncode = rc_src or rc_dst
             return dstproc.returncode
         dstproc.wait = wait2
+
+        dstproc_verify = dstproc.verify
+        def verify(proc, retcode, timeout, stdout, stderr):
+            #TODO: right now it's impossible to specify different expected
+            # return codes for different stages of the pipeline, but we
+            # should make that possible.
+            proc.srcproc.verify(retcode, timeout, stdout, stderr)
+            dstproc_verify(retcode, timeout, stdout, stderr)
+        dstproc.verify = MethodType(verify, dstproc)
+
         return dstproc
 
 class BaseRedirection(BaseCommand):
diff --git a/plumbum/commands/processes.py b/plumbum/commands/processes.py
index 2129dde..06a5dca 100644
--- a/plumbum/commands/processes.py
+++ b/plumbum/commands/processes.py
@@ -20,18 +20,7 @@ except ImportError:
 # utility functions
 #===================================================================================================
 def _check_process(proc, retcode, timeout, stdout, stderr):
-    if getattr(proc, "_timed_out", False):
-        raise ProcessTimedOut("Process did not terminate within %s seconds" % (timeout,),
-            getattr(proc, "argv", None))
-
-    if retcode is not None:
-        if hasattr(retcode, "__contains__"):
-            if proc.returncode not in retcode:
-                raise ProcessExecutionError(getattr(proc, "argv", None), proc.returncode,
-                    stdout, stderr)
-        elif proc.returncode != retcode:
-            raise ProcessExecutionError(getattr(proc, "argv", None), proc.returncode,
-                stdout, stderr)
+    proc.verify(retcode, timeout, stdout, stderr)
     return proc.returncode, stdout, stderr
 
 def _iter_lines(proc, decode, linesize):
diff --git a/plumbum/machines/base.py b/plumbum/machines/base.py
index 109f4f3..a384e4d 100644
--- a/plumbum/machines/base.py
+++ b/plumbum/machines/base.py
@@ -1,4 +1,21 @@
 from plumbum.commands.processes import CommandNotFound
+from plumbum.commands.processes import ProcessExecutionError
+from plumbum.commands.processes import ProcessTimedOut
+
+class PopenAddons(object):
+    def verify(self, retcode, timeout, stdout, stderr):
+        if getattr(self, "_timed_out", False):
+            raise ProcessTimedOut("Process did not terminate within %s seconds" % (timeout,),
+                getattr(self, "argv", None))
+
+        if retcode is not None:
+            if hasattr(retcode, "__contains__"):
+                if self.returncode not in retcode:
+                    raise ProcessExecutionError(getattr(self, "argv", None), self.returncode,
+                        stdout, stderr)
+            elif self.returncode != retcode:
+                raise ProcessExecutionError(getattr(self, "argv", None), self.returncode,
+                    stdout, stderr)
 
 
 class BaseMachine(object):
diff --git a/plumbum/machines/local.py b/plumbum/machines/local.py
index e821c25..0cd64e5 100644
--- a/plumbum/machines/local.py
+++ b/plumbum/machines/local.py
@@ -16,6 +16,7 @@ from plumbum.lib import ProcInfo, IS_WIN32, six, StaticProperty
 from plumbum.commands.daemons import win32_daemonize, posix_daemonize
 from plumbum.commands.processes import iter_lines
 from plumbum.machines.base import BaseMachine
+from plumbum.machines.base import PopenAddons
 from plumbum.machines.env import BaseEnv
 
 if sys.version_info >= (3, 2):
@@ -31,11 +32,12 @@ else:
         from subprocess import Popen, PIPE
         has_new_subprocess = False
 
-class IterablePopen(Popen):
+class IterablePopen(Popen, PopenAddons):
     iter_lines = iter_lines
     def __iter__(self):
         return self.iter_lines()
 
+
 logger = logging.getLogger("plumbum.local")
 
 
diff --git a/plumbum/machines/paramiko_machine.py b/plumbum/machines/paramiko_machine.py
index 43c5ed1..8f45ad8 100644
--- a/plumbum/machines/paramiko_machine.py
+++ b/plumbum/machines/paramiko_machine.py
@@ -2,6 +2,7 @@ import logging
 import errno
 import stat
 import socket
+from plumbum.machines.base import PopenAddons
 from plumbum.machines.remote import BaseRemoteMachine
 from plumbum.machines.session import ShellSession
 from plumbum.lib import _setdoc, six
@@ -23,7 +24,7 @@ except ImportError:
 
 logger = logging.getLogger("plumbum.paramiko")
 
-class ParamikoPopen(object):
+class ParamikoPopen(PopenAddons):
     def __init__(self, argv, stdin, stdout, stderr, encoding, stdin_file = None,
             stdout_file = None, stderr_file = None):
         self.argv = argv
diff --git a/plumbum/machines/session.py b/plumbum/machines/session.py
index 3767294..43b4b37 100644
--- a/plumbum/machines/session.py
+++ b/plumbum/machines/session.py
@@ -4,6 +4,7 @@ import logging
 import threading
 from plumbum.commands import BaseCommand, run_proc
 from plumbum.lib import six
+from plumbum.machines.base import PopenAddons
 
 
 class ShellSessionError(Exception):
@@ -53,7 +54,7 @@ class MarkedPipe(object):
         return line
 
 
-class SessionPopen(object):
+class SessionPopen(PopenAddons):
     """A shell-session-based ``Popen``-like object (has the following attributes: ``stdin``,
     ``stdout``, ``stderr``, ``returncode``)"""
     def __init__(self, argv, isatty, stdin, stdout, stderr, encoding):


Test patch:
diff --git a/tests/test_local.py b/tests/test_local.py
index d5bd458..a1fadcd 100644
--- a/tests/test_local.py
+++ b/tests/test_local.py
@@ -7,6 +7,7 @@ from plumbum import (local, LocalPath, FG, BG, TF, RETCODE, ERROUT,
                     CommandNotFound, ProcessExecutionError, ProcessTimedOut)
 from plumbum.lib import six, IS_WIN32
 from plumbum.fs.atomic import AtomicFile, AtomicCounterFile, PidFile
+from plumbum.machines.local import LocalCommand
 from plumbum.path import RelativePath
 import plumbum
 from plumbum._testtools import (skipIf, skip_on_windows,
@@ -295,13 +296,26 @@ class LocalMachineTest(unittest.TestCase):
     def test_run(self):
         from plumbum.cmd import ls, grep
 
-        rc, out, err = (ls | grep["non_exist1N9"]).run(retcode = 1)
+        rc, out, err = (ls | grep["non_exist1N9"]).run(retcode = (0, 1))
         self.assertEqual(rc, 1)
 
     def test_timeout(self):
         from plumbum.cmd import sleep
         self.assertRaises(ProcessTimedOut, sleep, 10, timeout = 5)
 
+    @skip_on_windows
+    def test_fair_error_attribution(self):
+        # use LocalCommand directly for predictable argv
+        false = LocalCommand('false')
+        true = LocalCommand('true')
+        try:
+            (false | true) & FG
+        except ProcessExecutionError as e:
+            self.assertEqual(e.argv, ['false'])
+        else:
+            self.fail("Expected a ProcessExecutionError")
+
+
     @skip_on_windows
     def test_iter_lines_timeout(self):
         from plumbum.cmd import ping