classification
Title: Accidental exception chaining in inspect.Signature.bind()
Type: Stage: resolved
Components: Library (Lib) Versions: Python 3.6, Python 3.5
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: yselivanov Nosy List: doerwalter, larry, ncoghlan, yselivanov
Priority: normal Keywords: needs review, patch

Created on 2015-06-19 16:16 by doerwalter, last changed 2015-06-20 08:50 by ncoghlan. This issue is now closed.

Files
File name Uploaded Description Edit
sig.patch yselivanov, 2015-06-19 17:57 review
Messages (5)
msg245506 - (view) Author: Walter Dörwald (doerwalter) * (Python committer) Date: 2015-06-19 16:16
When an exception is raised by inspect.Signature.bind() in some cases the exception has a StopIteration as its __context__:

import inspect

try:
   inspect.signature(lambda x:None).bind()
except Exception as exc:
   print(repr(exc))
   print(repr(exc.__context__))

This prints:

   TypeError("missing a required argument: 'x'",)
   StopIteration()

I would have expected it to print:

   TypeError("missing a required argument: 'x'",)
   None

This reason for this is that the code in bind() has nested exception handlers. The innermost handler does

   raise TypeError(...) from None

to drop the exception context, but another context exception gets added by the outermost exception handler.
msg245512 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-06-19 17:57
Hi Walter,
Thanks for reporting this. A patch is attached.
msg245543 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-06-20 06:28
This isn't a bug, as "raise exc from None" only hides *display* of the context, it doesn't lose the record of the context entirely.

This means Walter's display code is incorrect, as it isn't checking for __suppress_context__:

>>> import inspect
>>> try:
...     inspect.signature(lambda x:None).bind()
... except Exception as e:
...     exc = e
... 
>>> exc.__suppress_context__
True

Interestingly, neither PEP 409 *nor* PEP 415 capture the rationale for that design: retaining the full context data even when it's hidden by the default exception display routines means that you can still debug code that uses "raise X from None" inappropriately. (I thought there was an open RFE to make it easy to force the traceback module to ignore __suppress_context__ but if there is one, I can't find it)
msg245550 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2015-06-20 07:34
> This isn't a bug, as "raise exc from None" only hides *display* of the context, it doesn't lose the record of the context entirely.

Agree.  My patch, though, is still valid.  I think removing try..except blocks actually simplifies the code, and removes the need of using 'raise .. from None' completely.
msg245555 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2015-06-20 08:50
The sentinel form of next() is nice when there's a natural sentinel value, but needing to *create* a sentinel is generally a sign that the out-of-band exception is a better fit.

The function could likely do with some refactoring (e.g. breaking out some helper functions to make the logic easier to follow), but shifting from idiomatic code to non-idiomatic code isn't a good way of tackling that.
History
Date User Action Args
2015-06-20 08:50:59ncoghlansetmessages: + msg245555
2015-06-20 07:34:41yselivanovsetmessages: + msg245550
2015-06-20 06:28:35ncoghlansetstatus: open -> closed
resolution: not a bug
messages: + msg245543

stage: patch review -> resolved
2015-06-19 17:57:41yselivanovsetfiles: + sig.patch

nosy: + ncoghlan, larry
messages: + msg245512

keywords: + needs review, patch
stage: patch review
2015-06-19 16:20:44yselivanovsetassignee: yselivanov

nosy: + yselivanov
components: + Library (Lib)
versions: + Python 3.5, Python 3.6
2015-06-19 16:16:29doerwaltercreate