bpo-5001, bpo-31169: Fix two uninformative asserts in multiprocessing/managers.py...

* Make error message more informative

Replace assertions in error-reporting code with more-informative version that doesn't cause confusion over where and what the error is.

* Additional clarification + get travis to check

* Change from SystemError to TypeError

As suggested in PR comment by @pitrou, changing from SystemError; TypeError appears appropriate.

* NEWS file installation; ACKS addition (will do my best to justify it by additional work)

diff --git a/Lib/multiprocessing/managers.py b/Lib/multiprocessing/managers.py
index cae1c10734ba5a58cf69ad47a85263eaeeddaea9..c6722771b05c94f97469ed3db14250f99142fd45 100644 (file)
--- a/Lib/multiprocessing/managers.py
+++ b/Lib/multiprocessing/managers.py
@@ -84,14 +84,17 @@ def dispatch(c, id, methodname, args=(), kwds={}):
 def convert_to_error(kind, result):
     if kind == '#ERROR':
         return result
-    elif kind == '#TRACEBACK':
-        assert type(result) is str
-        return  RemoteError(result)
-    elif kind == '#UNSERIALIZABLE':
-        assert type(result) is str
-        return RemoteError('Unserializable message: %s\n' % result)
+    elif kind in ('#TRACEBACK', '#UNSERIALIZABLE'):
+        if not isinstance(result, str):
+            raise TypeError(
+                "Result {0!r} (kind '{1}') type is {2}, not str".format(
+                    result, kind, type(result)))
+        if kind == '#UNSERIALIZABLE':
+            return RemoteError('Unserializable message: %s\n' % result)
+        else:
+            return RemoteError(result)
     else:
-        return ValueError('Unrecognized message type')
+        return ValueError('Unrecognized message type {!r}'.format(kind))
 
 class RemoteError(Exception):
     def __str__(self):

diff --git a/Misc/ACKS b/Misc/ACKS
index d6088b3f5d987b4fd4cbc166fa9767d4054b8fe4..6fc41b36b597106e27c000e8e55f85b9ce5ddb56 100644 (file)
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -1465,6 +1465,7 @@ Ville Skyttä
 Michael Sloan
 Nick Sloan
 Václav Šmilauer
+Allen W. Smith
 Christopher Smith
 Eric V. Smith
 Gregory P. Smith

diff --git a/Misc/NEWS.d/next/Library/2017-08-12-09-25-55.bpo-5001.huQi2Y.rst b/Misc/NEWS.d/next/Library/2017-08-12-09-25-55.bpo-5001.huQi2Y.rst
new file mode 100644 (file)
index 0000000..f157d49
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2017-08-12-09-25-55.bpo-5001.huQi2Y.rst
@@ -0,0 +1,9 @@
+There are a number of uninformative asserts in the `multiprocessing` module,
+as noted in issue 5001. This change fixes two of the most potentially
+problematic ones, since they are in error-reporting code, in the
+`multiprocessing.managers.convert_to_error` function. (It also makes more
+informative a ValueError message.) The only potentially problematic change
+is that the AssertionError is now a TypeError; however, this should also
+help distinguish it from an AssertionError being *reported* by the
+function/its caller (such as in issue 31169). - Patch by Allen W. Smith
+(drallensmith on github).