Repository: giampaolo/psutil
Pull request #1992 (2021-10-03 13:36:55)

Improve custom error tracebacks and messages
Removal of duplicated `psutil.NoSuchProcess` text. Before:

```
psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists (pid=4651, name="python")
psutil.ZombieProcess: psutil.ZombieProcess process no longer exists and it's a zombie (pid=4651, name="python")
psutil.AccessDenied: psutil.AccessDenied (pid=4651, name="python")
psutil.TimeoutExpired: psutil.TimeoutExpired timeout after 5 seconds (pid=4651, name="python")
```

Now:

```
psutil.NoSuchProcess: process no longer exists (pid=4651, name="python")
psutil.ZombieProcess: process no longer exists and it's a zombie (pid=4651, name="python")
psutil.AccessDenied: (pid=4651, name="python")
psutil.TimeoutExpired: timeout after 5 seconds (pid=4651, name="python")

```
---

More info if process PID has been reused: Before:

```
psutil.NoSuchProcess: psutil.NoSuchProcess process no longer exists (pid=465148)
```

Now:

```
psutil.NoSuchProcess: process no longer exists and its PID has been reused (pid=465148)
```

---

Before:

```
psutil.NoSuchProcess: psutil.NoSuchProcess no process found with pid 666
```

Now:

```
psutil.NoSuchProcess: process PID not found (pid=666)
```

--- 

Before:

```
>>> psutil.NoSuchProcess(212, name="python")
psutil.NoSuchProcess process no longer exists (pid=212, name='python')
```

Now:

```
>>> psutil.NoSuchProcess(212, name="python")
psutil.NoSuchProcess(pid=212, name='python', msg='process no longer exists')
```


Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index ee7803d1..db0e4261 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -10,6 +10,9 @@ XXXX-XX-XX
 - 1851_: [Linux] cpu_freq() is slow on systems with many CPUs. Read current
   frequency values for all CPUs from /proc/cpuinfo instead of opening many
   files in /sys fs.  (patch by marxin)
+- 1992_: NoSuchProcess message now specifies if the PID has been reused.
+- 1992_: error classes (NoSuchProcess, AccessDenied, etc.) now have a better
+  formatted and separated `__repr__` and `__str__` implementations.
 
 **Bug fixes**
 
diff --git a/psutil/__init__.py b/psutil/__init__.py
index 41f9bf5a..e52fdcda 100644
--- a/psutil/__init__.py
+++ b/psutil/__init__.py
@@ -269,7 +269,11 @@ def _assert_pid_not_reused(fun):
     @functools.wraps(fun)
     def wrapper(self, *args, **kwargs):
         if not self.is_running():
-            raise NoSuchProcess(self.pid, self._name)
+            if self._pid_reused:
+                msg = "process no longer exists and its PID has been reused"
+            else:
+                msg = None
+            raise NoSuchProcess(self.pid, self._name, msg=msg)
         return fun(self, *args, **kwargs)
     return wrapper
 
@@ -340,6 +344,7 @@ class Process(object):
         self._exe = None
         self._create_time = None
         self._gone = False
+        self._pid_reused = False
         self._hash = None
         self._lock = threading.RLock()
         # used for caching on Windows only (on POSIX ppid may change)
@@ -364,8 +369,7 @@ class Process(object):
             pass
         except NoSuchProcess:
             if not _ignore_nsp:
-                msg = 'no process found with pid %s' % pid
-                raise NoSuchProcess(pid, None, msg)
+                raise NoSuchProcess(pid, msg='process PID not found')
             else:
                 self._gone = True
         # This pair is supposed to indentify a Process instance
@@ -571,7 +575,7 @@ class Process(object):
         It also checks if PID has been reused by another process in
         which case return False.
         """
-        if self._gone:
+        if self._gone or self._pid_reused:
             return False
         try:
             # Checking if PID is alive is not enough as the PID might
@@ -579,7 +583,8 @@ class Process(object):
             # verify process identity.
             # Process identity / uniqueness over time is guaranteed by
             # (PID + creation time) and that is verified in __eq__.
-            return self == Process(self.pid)
+            self._pid_reused = self != Process(self.pid)
+            return not self._pid_reused
         except ZombieProcess:
             # We should never get here as it's already handled in
             # Process.__init__; here just for extra safety.
diff --git a/psutil/_common.py b/psutil/_common.py
index 2acab626..306301ed 100644
--- a/psutil/_common.py
+++ b/psutil/_common.py
@@ -9,6 +9,7 @@
 
 from __future__ import division, print_function
 
+import collections
 import contextlib
 import errno
 import functools
@@ -18,7 +19,6 @@ import stat
 import sys
 import threading
 import warnings
-from collections import defaultdict
 from collections import namedtuple
 from socket import AF_INET
 from socket import SOCK_DGRAM
@@ -275,15 +275,32 @@ class Error(Exception):
     """
     __module__ = 'psutil'
 
-    def __init__(self, msg=""):
-        Exception.__init__(self, msg)
-        self.msg = msg
+    def _infodict(self, attrs):
+        try:
+            info = collections.OrderedDict()
+        except AttributeError:  # pragma: no cover
+            info = {}  # Python 2.6
+        for name in attrs:
+            value = getattr(self, name, None)
+            if value:
+                info[name] = value
+        return info
+
+    def __str__(self):
+        # invoked on `raise Error`
+        info = self._infodict(("pid", "ppid", "name"))
+        if info:
+            details = "(%s)" % ", ".join(
+                ["%s=%r" % (k, v) for k, v in info.items()])
+        else:
+            details = None
+        return " ".join([x for x in (self.msg, details) if x])
 
     def __repr__(self):
-        ret = "psutil.%s %s" % (self.__class__.__name__, self.msg)
-        return ret.strip()
-
-    __str__ = __repr__
+        # invoked on `repr(Error)`
+        info = self._infodict(("pid", "ppid", "name", "seconds", "msg"))
+        details = ", ".join(["%s=%r" % (k, v) for k, v in info.items()])
+        return "psutil.%s(%s)" % (self.__class__.__name__, details)
 
 
 class NoSuchProcess(Error):
@@ -293,16 +310,10 @@ class NoSuchProcess(Error):
     __module__ = 'psutil'
 
     def __init__(self, pid, name=None, msg=None):
-        Error.__init__(self, msg)
+        Error.__init__(self)
         self.pid = pid
         self.name = name
-        self.msg = msg
-        if msg is None:
-            if name:
-                details = "(pid=%s, name=%s)" % (self.pid, repr(self.name))
-            else:
-                details = "(pid=%s)" % self.pid
-            self.msg = "process no longer exists " + details
+        self.msg = msg or "process no longer exists"
 
 
 class ZombieProcess(NoSuchProcess):
@@ -315,19 +326,9 @@ class ZombieProcess(NoSuchProcess):
     __module__ = 'psutil'
 
     def __init__(self, pid, name=None, ppid=None, msg=None):
-        NoSuchProcess.__init__(self, msg)
-        self.pid = pid
+        NoSuchProcess.__init__(self, pid, name, msg)
         self.ppid = ppid
-        self.name = name
-        self.msg = msg
-        if msg is None:
-            args = ["pid=%s" % pid]
-            if name:
-                args.append("name=%s" % repr(self.name))
-            if ppid:
-                args.append("ppid=%s" % self.ppid)
-            details = "(%s)" % ", ".join(args)
-            self.msg = "process still exists but it's a zombie " + details
+        self.msg = msg or "PID still exists but it's a zombie"
 
 
 class AccessDenied(Error):
@@ -335,17 +336,10 @@ class AccessDenied(Error):
     __module__ = 'psutil'
 
     def __init__(self, pid=None, name=None, msg=None):
-        Error.__init__(self, msg)
+        Error.__init__(self)
         self.pid = pid
         self.name = name
-        self.msg = msg
-        if msg is None:
-            if (pid is not None) and (name is not None):
-                self.msg = "(pid=%s, name=%s)" % (pid, repr(name))
-            elif (pid is not None):
-                self.msg = "(pid=%s)" % self.pid
-            else:
-                self.msg = ""
+        self.msg = msg or ""
 
 
 class TimeoutExpired(Error):
@@ -355,14 +349,11 @@ class TimeoutExpired(Error):
     __module__ = 'psutil'
 
     def __init__(self, seconds, pid=None, name=None):
-        Error.__init__(self, "timeout after %s seconds" % seconds)
+        Error.__init__(self)
         self.seconds = seconds
         self.pid = pid
         self.name = name
-        if (pid is not None) and (name is not None):
-            self.msg += " (pid=%s, name=%s)" % (pid, repr(name))
-        elif (pid is not None):
-            self.msg += " (pid=%s)" % self.pid
+        self.msg = "timeout after %s seconds" % seconds
 
 
 # ===================================================================
@@ -629,8 +620,8 @@ class _WrapNumbers:
         assert name not in self.reminders
         assert name not in self.reminder_keys
         self.cache[name] = input_dict
-        self.reminders[name] = defaultdict(int)
-        self.reminder_keys[name] = defaultdict(set)
+        self.reminders[name] = collections.defaultdict(int)
+        self.reminder_keys[name] = collections.defaultdict(set)
 
     def _remove_dead_reminders(self, input_dict, name):
         """In case the number of keys changed between calls (e.g. a


Test patch:
diff --git a/psutil/tests/test_contracts.py b/psutil/tests/test_contracts.py
index 32c75fd7..b03477d9 100755
--- a/psutil/tests/test_contracts.py
+++ b/psutil/tests/test_contracts.py
@@ -360,7 +360,6 @@ def proc_info(pid):
         elif isinstance(exc, psutil.NoSuchProcess):
             tcase.assertProcessGone(proc)
         str(exc)
-        assert exc.msg
 
     def do_wait():
         if pid != 0:
diff --git a/psutil/tests/test_misc.py b/psutil/tests/test_misc.py
index 81fa8f39..fc9f5b13 100755
--- a/psutil/tests/test_misc.py
+++ b/psutil/tests/test_misc.py
@@ -97,57 +97,71 @@ class TestMisc(PsutilTestCase):
     def test_process__str__(self):
         self.test_process__repr__(func=str)
 
-    def test_no_such_process__repr__(self, func=repr):
+    def test_no_such_process__repr__(self):
         self.assertEqual(
             repr(psutil.NoSuchProcess(321)),
-            "psutil.NoSuchProcess process no longer exists (pid=321)")
+            "psutil.NoSuchProcess(pid=321, msg='process no longer exists')")
         self.assertEqual(
-            repr(psutil.NoSuchProcess(321, name='foo')),
-            "psutil.NoSuchProcess process no longer exists (pid=321, "
-            "name='foo')")
+            repr(psutil.NoSuchProcess(321, name="name", msg="msg")),
+            "psutil.NoSuchProcess(pid=321, name='name', msg='msg')")
+
+    def test_no_such_process__str__(self):
+        self.assertEqual(
+            str(psutil.NoSuchProcess(321)),
+            "process no longer exists (pid=321)")
         self.assertEqual(
-            repr(psutil.NoSuchProcess(321, msg='foo')),
-            "psutil.NoSuchProcess foo")
+            str(psutil.NoSuchProcess(321, name="name", msg="msg")),
+            "msg (pid=321, name='name')")
 
-    def test_zombie_process__repr__(self, func=repr):
+    def test_zombie_process__repr__(self):
         self.assertEqual(
             repr(psutil.ZombieProcess(321)),
-            "psutil.ZombieProcess process still exists but it's a zombie "
-            "(pid=321)")
+            'psutil.ZombieProcess(pid=321, msg="PID still '
+            'exists but it\'s a zombie")')
         self.assertEqual(
-            repr(psutil.ZombieProcess(321, name='foo')),
-            "psutil.ZombieProcess process still exists but it's a zombie "
-            "(pid=321, name='foo')")
+            repr(psutil.ZombieProcess(321, name="name", ppid=320, msg="foo")),
+            "psutil.ZombieProcess(pid=321, ppid=320, name='name', msg='foo')")
+
+    def test_zombie_process__str__(self):
         self.assertEqual(
-            repr(psutil.ZombieProcess(321, name='foo', ppid=1)),
-            "psutil.ZombieProcess process still exists but it's a zombie "
-            "(pid=321, name='foo', ppid=1)")
+            str(psutil.ZombieProcess(321)),
+            "PID still exists but it's a zombie (pid=321)")
         self.assertEqual(
-            repr(psutil.ZombieProcess(321, msg='foo')),
-            "psutil.ZombieProcess foo")
+            str(psutil.ZombieProcess(321, name="name", ppid=320, msg="foo")),
+            "foo (pid=321, ppid=320, name='name')")
 
-    def test_access_denied__repr__(self, func=repr):
+    def test_access_denied__repr__(self):
         self.assertEqual(
             repr(psutil.AccessDenied(321)),
-            "psutil.AccessDenied (pid=321)")
+            "psutil.AccessDenied(pid=321)")
         self.assertEqual(
-            repr(psutil.AccessDenied(321, name='foo')),
-            "psutil.AccessDenied (pid=321, name='foo')")
+            repr(psutil.AccessDenied(321, name="name", msg="msg")),
+            "psutil.AccessDenied(pid=321, name='name', msg='msg')")
+
+    def test_access_denied__str__(self):
         self.assertEqual(
-            repr(psutil.AccessDenied(321, msg='foo')),
-            "psutil.AccessDenied foo")
+            str(psutil.AccessDenied(321)),
+            "(pid=321)")
+        self.assertEqual(
+            str(psutil.AccessDenied(321, name="name", msg="msg")),
+            "msg (pid=321, name='name')")
 
-    def test_timeout_expired__repr__(self, func=repr):
+    def test_timeout_expired__repr__(self):
         self.assertEqual(
-            repr(psutil.TimeoutExpired(321)),
-            "psutil.TimeoutExpired timeout after 321 seconds")
+            repr(psutil.TimeoutExpired(5)),
+            "psutil.TimeoutExpired(seconds=5, msg='timeout after 5 seconds')")
+        self.assertEqual(
+            repr(psutil.TimeoutExpired(5, pid=321, name="name")),
+            "psutil.TimeoutExpired(pid=321, name='name', seconds=5, "
+            "msg='timeout after 5 seconds')")
+
+    def test_timeout_expired__str__(self):
         self.assertEqual(
-            repr(psutil.TimeoutExpired(321, pid=111)),
-            "psutil.TimeoutExpired timeout after 321 seconds (pid=111)")
+            str(psutil.TimeoutExpired(5)),
+            "timeout after 5 seconds")
         self.assertEqual(
-            repr(psutil.TimeoutExpired(321, pid=111, name='foo')),
-            "psutil.TimeoutExpired timeout after 321 seconds "
-            "(pid=111, name='foo')")
+            str(psutil.TimeoutExpired(5, pid=321, name="name")),
+            "timeout after 5 seconds (pid=321, name='name')")
 
     def test_process__eq__(self):
         p1 = psutil.Process()
diff --git a/psutil/tests/test_process.py b/psutil/tests/test_process.py
index dfe88547..002c58de 100755
--- a/psutil/tests/test_process.py
+++ b/psutil/tests/test_process.py
@@ -1332,6 +1332,20 @@ class TestProcess(PsutilTestCase):
             self.assertEqual(p.status(), psutil.STATUS_ZOMBIE)
             assert m.called
 
+    def test_reused_pid(self):
+        # Emulate a case where PID has been reused by another process.
+        subp = self.spawn_testproc()
+        p = psutil.Process(subp.pid)
+        p._ident = (p.pid, p.create_time() + 100)
+        assert not p.is_running()
+        assert p != psutil.Process(subp.pid)
+        msg = "process no longer exists and its PID has been reused"
+        self.assertRaisesRegex(psutil.NoSuchProcess, msg, p.suspend)
+        self.assertRaisesRegex(psutil.NoSuchProcess, msg, p.resume)
+        self.assertRaisesRegex(psutil.NoSuchProcess, msg, p.terminate)
+        self.assertRaisesRegex(psutil.NoSuchProcess, msg, p.kill)
+        self.assertRaisesRegex(psutil.NoSuchProcess, msg, p.children)
+
     def test_pid_0(self):
         # Process(0) is supposed to work on all platforms except Linux
         if 0 not in psutil.pids():