classification
Title: Problems with recursive automatic exception chaining
Type: behavior Stage:
Components: Interpreter Core Versions: Python 3.5
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: alonho, benjamin.peterson, chris.jerdonek, hniksic, ncoghlan, nikratio, njs
Priority: normal Keywords:

Created on 2013-08-28 02:42 by nikratio, last changed 2017-11-11 09:03 by chris.jerdonek.

Messages (17)
msg196344 - (view) Author: Nikolaus Rath (nikratio) * Date: 2013-08-28 02:42
Consider this:

$ python3 test_exc.py 
Traceback (most recent call last):
  File "test_exc.py", line 14, in <module>
    fail1()
  File "test_exc.py", line 11, in fail1
    fail2()
  File "test_exc.py", line 5, in fail2
    raise RuntimeError('Third') from None
RuntimeError: Third

$ cat test_exc.py 
def fail2():
    try:
        raise RuntimeError('Second')
    except RuntimeError:
        raise RuntimeError('Third') from None

def fail1():
    try:
        raise RuntimeError('First')
    except:
        fail2()
        raise

fail1()


Any exception raised in fail2() is the immediate consequence of the 'First' exception should thus be chained to the 'First' exception.

However, if somewhere in the call stack under fail2() an exception is caught and re-raised from None (to convert between exception types), this also results in a loss of the chain to the initial exception.


The correct stacktrace (in my opinion) would be:
Traceback (most recent call last):
  File "test_exc.py", line 9, in fail1
    raise RuntimeError('First')
RuntimeError: First

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "test_exc.py", line 14, in <module>
    fail1()
  File "test_exc.py", line 11, in fail1
    fail2()
  File "test_exc.py", line 5, in fail2
    raise RuntimeError('Third')
RuntimeError: Third
msg196345 - (view) Author: Nikolaus Rath (nikratio) * Date: 2013-08-28 02:43
The second paragraph should of course read "...consequence of the 'First' exception *and* should thus be chained..."
msg201410 - (view) Author: Nikolaus Rath (nikratio) * Date: 2013-10-27 03:33
*ping* 

No one any comments on this at all?
msg201417 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2013-10-27 06:01
raise ... from None explictly silences the printing of exception context, so I don't see the problem.
msg201460 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-10-27 13:47
Nothing is lost either way, it just isn't displayed by default.

While I can see the appeal of treating an "outer" context differently from an "inner" one, I don't see any way for the exception context suppression machinery to tell the difference.

The traceback *display* machinery might be able to figure it out by looking at the traceback details, but such a search could be quite expensive.
msg201482 - (view) Author: Nikolaus Rath (nikratio) * Date: 2013-10-27 18:22
Benjamin: I think that in most cases the intention of a ".. from None" is to disable printing of a stack trace for a specific exception when it occurs in the try .. except block. (In the example, that would be suppressing the stacktrace for the "Second" exception, because it is explicity converted to "Third"). I do not think that this ought to effect the printing of exceptions that occured previously and higher-up in the call chain.

In other words, I think it is natural to expect that

def foo():
   try:
      do_stuff
   except Something:
      raise SomethingElse from None

disables printing the stack trace for `Something` exceptions when they occur in the try..except block -- but not to hide the exception context for exceptions that occured before foo() was even called.
msg202421 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-11-08 14:34
I actually think Nikratio is right about the way this *should* work (intuitively).

I'm just not sure it's feasible to *implement* those semantics in CPython without significant changes to the way exception handling works - I don't believe the exception chaining code can currently tell the difference between the cases:

  # No context on Exception3 is exactly what we want
  try:
      try:
          raise Exception1
      except Exception1:
          raise Exception2
  except Exception2 as exc
      raise Exception3 from Exception2

  # Not seeing Exception1 as the context for Exception3 is surprising!
  try:
      raise Exception1
  except Exception1:
      try:
          raise Exception2
      except Exception2 as exc
          raise Exception3 from Exception2

In a certain sense, exceptions need to be able to have *multiple* contexts to handle this case properly without losing data. Frames would need to tag exceptions appropriately with the context details as an unhandled exception passed through a frame that was currently running an exception handler.

So even though it doesn't require new syntax, I think it *does* require a PEP if we're going to change this (and we still haven't fully dealt with the consequence of the last one - the display options for tracebacks are still a bit limited)
msg202523 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-11-10 13:12
It may not immediately look like it, but I think issue 17828 offers an example of a related problem. In that issue, I didn't want to *change* the exception raised, I wanted to annotate it to say "Hey, something I called raised an exception, here's some relevant local state to help you figure out what is going on" (in that case, the information to be added is the specific codec being invoked and whether it is an encoding or decoding operation).

Setting the context is somewhat similar - you don't just want to know which specific exception happened to be active when the eventually caught exception was first raised - you also want to know which already active exception handlers you passed through while unwinding the stack.

So really, what may be desirable here is almost an "annotated traceback", where the interpreter can decide to hang additional information off the frame currently being unwound in the exceptions traceback, while leaving the exception itself alone.

That's definitely PEP territory, but there are two distinct problems with the current exception chaining mechanism to help drive a prospective design for 3.5 (the status quo doesn't bother me enough for me to work on it myself, but if you're interested Nikolaus...)
msg202525 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-11-10 13:22
Unrelated to my previous comment, I'm also wondering if this may actually represent a behavioural difference between contextlib.ExitStack and the interpreter's own exception handling machinery.

ExitStack uses a recursive->iterative transformation for its stack unwinding (see issue 14963), and it needs to do a bit of fiddling to get the context right (see issue 19092).

While the contextlib test suite goes to great lengths to try to ensure the semantics of normal stack unwinding are preserved, that definitely doesn't currently cover this case, and I'm thinking the way it works may actually be more like the behaviour Nikolaus expected in the original post (i.e. setting the context as the stack is unwound rather than when the replacement exception is raised).
msg202562 - (view) Author: Nikolaus Rath (nikratio) * Date: 2013-11-10 19:53
Hi Nick,

I am interested in working on this, but I have never worked on the C parts of cpython before. Do you think this is a feasible project to start with? To me it looks a bit daunting, I'd certainly need some mentoring to even know where to start with this.
msg202564 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2013-11-10 20:01
The first thing to do is to carefully specificy what the behavior should be.

2013/11/10 Nikolaus Rath <report@bugs.python.org>:
>
> Nikolaus Rath added the comment:
>
> Hi Nick,
>
> I am interested in working on this, but I have never worked on the C parts of cpython before. Do you think this is a feasible project to start with? To me it looks a bit daunting, I'd certainly need some mentoring to even know where to start with this.
>
> ----------
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <http://bugs.python.org/issue18861>
> _______________________________________
msg202579 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-11-10 22:16
Yes, I suggest using ExitStack to figure out the behaviour we *want* first,
before diving into the messy practical details of how to make that a
reality in CPython. We somehow have to get the state of the exception
object and its traceback to represent an appropriate stack *tree*, rather
than the traditionally assumed linear stack.

It also occurred to me there's another potentially related issue: frame
hiding, where we want to avoid showing infrastructure code in end user
tracebacks. importlib currently has a very hacky version of that. The
Jinja2 template library uses a different approach.

The reason I bring these other problems up is because I think they
illustrate a theme around altering how a traceback is displayed that may be
amenable to a common solution (preferably one that is contextlib and
asyncio friendly).
msg202754 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-11-13 14:46
So, I've been pondering the idea of traceback/frame annotations and exception trees a bit.

And what I'm wondering is if it may make sense to have the ability to annotate *frames* at runtime, and these annotations can be qualified by module names. So, for example, you might be able to write things like:

    sys.annotate_frame("codecs", "encoding", the_encoding)
    sys.annotate_frame("codecs", "decoding", the_encoding)
    sys.annotate_frame("traceback", "hide", True)
    sys.annotate_frame("traceback", "context", exc)

And then the traceback display machinery would be updated to do something useful with the annotations.

I'm not sure how ExitStack would cope with that (or other code that fakes tracebacks) but it's something to ponder.
msg202759 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-11-13 16:14
Walter suggested annotating the exceptions directly might work: https://mail.python.org/pipermail/python-dev/2013-November/130155.html

However, that potentially runs into nesting problems (e.g. the idna codec invokes the ascii codec), although Walter suggested a simpler mechanism that just involved appending to a list.
msg203015 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-11-16 09:01
Adjusting the target version, since it isn't feasible to move away from the current behaviour any earlier than Python 3.5.
msg203017 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-11-16 09:34
Something else that this might make simpler: the unittest.TestCase.subTest API is currently kind of ugly. If the subtest details could be stored as a frame annotation instead...
msg203370 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2013-11-19 12:47
Actually, this won't the subTest example because that actually *suppresses* the errors, and reports them later. Annotations only help when the exception is allowed to escape.
History
Date User Action Args
2017-11-11 09:03:05chris.jerdoneksetnosy: + chris.jerdonek
2017-02-18 03:54:28njssetnosy: + njs
2013-11-19 12:47:58ncoghlansetmessages: + msg203370
2013-11-16 09:34:43ncoghlansetmessages: + msg203017
2013-11-16 09:01:59ncoghlansetmessages: + msg203015
versions: + Python 3.5, - Python 3.3
2013-11-13 16:14:58ncoghlansetmessages: + msg202759
2013-11-13 14:46:47ncoghlansetmessages: + msg202754
2013-11-10 22:16:33ncoghlansetmessages: + msg202579
2013-11-10 20:01:09benjamin.petersonsetmessages: + msg202564
2013-11-10 19:53:29nikratiosetmessages: + msg202562
2013-11-10 13:22:23ncoghlansetnosy: + hniksic, alonho
messages: + msg202525
2013-11-10 13:12:45ncoghlansetmessages: + msg202523
2013-11-08 14:34:12ncoghlansetmessages: + msg202421
2013-10-27 18:22:46nikratiosetmessages: + msg201482
2013-10-27 13:47:47ncoghlansetnosy: + ncoghlan
messages: + msg201460
2013-10-27 06:01:12benjamin.petersonsetmessages: + msg201417
2013-10-27 03:33:41nikratiosetmessages: + msg201410
2013-08-28 07:58:16pitrousetnosy: + benjamin.peterson
2013-08-28 02:43:24nikratiosetmessages: + msg196345
2013-08-28 02:42:07nikratiocreate