summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAllen W. Smith, Ph.D <drallensmith@users.noreply.github.com>2017-08-12 15:37:09 (GMT)
committerAntoine Pitrou <pitrou@free.fr>2017-08-12 15:37:09 (GMT)
commit48d9823a0ebde4dfab8bc154bb6df462fb2ee403 (patch)
tree14fbb6daae62b2c83da9b7af1b309b8c64630ed0
parente664d7f89d2b9960d9049237136396e824795cac (diff)
downloadcpython-48d9823a0ebde4dfab8bc154bb6df462fb2ee403.zip
cpython-48d9823a0ebde4dfab8bc154bb6df462fb2ee403.tar.gz
cpython-48d9823a0ebde4dfab8bc154bb6df462fb2ee403.tar.bz2
bpo-5001, bpo-31169: Fix two uninformative asserts in multiprocessing/managers.py (#3078)
* 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)
-rw-r--r--Lib/multiprocessing/managers.py17
-rw-r--r--Misc/ACKS1
-rw-r--r--Misc/NEWS.d/next/Library/2017-08-12-09-25-55.bpo-5001.huQi2Y.rst9
3 files changed, 20 insertions, 7 deletions
diff --git a/Lib/multiprocessing/managers.py b/Lib/multiprocessing/managers.py
index cae1c10..c672277 100644
--- 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 d6088b3..6fc41b3 100644
--- 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
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).