From 48d9823a0ebde4dfab8bc154bb6df462fb2ee403 Mon Sep 17 00:00:00 2001 From: "Allen W. Smith, Ph.D" Date: Sat, 12 Aug 2017 10:37:09 -0500 Subject: 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) --- Lib/multiprocessing/managers.py | 17 ++++++++++------- Misc/ACKS | 1 + .../Library/2017-08-12-09-25-55.bpo-5001.huQi2Y.rst | 9 +++++++++ 3 files changed, 20 insertions(+), 7 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2017-08-12-09-25-55.bpo-5001.huQi2Y.rst 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). -- cgit v0.12