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)
This commit is contained in:
Allen W. Smith, Ph.D 2017-08-12 10:37:09 -05:00 committed by Antoine Pitrou
parent e664d7f89d
commit 48d9823a0e
3 changed files with 20 additions and 7 deletions

View File

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

View File

@ -1465,6 +1465,7 @@ Ville Skyttä
Michael Sloan Michael Sloan
Nick Sloan Nick Sloan
Václav Šmilauer Václav Šmilauer
Allen W. Smith
Christopher Smith Christopher Smith
Eric V. Smith Eric V. Smith
Gregory P. Smith Gregory P. Smith

View File

@ -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).