bpo-45711: Change exc_info related APIs to derive type and traceback from the exception instance

[iritkatriel]

https://bugs.python.org/issue45711

[gvanrossum]

Nice and straightforward. I have a few nits but really, this could go as-is.

[iritkatriel]

There is one more place where the move to using traceback from the exception can be visible from a program - in the eval loop when we reraise. See iritkatriel#32 . I think maybe we should add that change to this PR.

[gvanrossum]

There is one more place where the move to using traceback from the exception can be visible from a program - in the eval loop when we reraise. See iritkatriel#32 . I think maybe we should add that change to this PR.

It comes down to how certain we are that that's not going to break anything. I don't see an assertion in there that checks that exc_info->exc_traceback equals whatever PyException_GetTraceback() returns.

[iritkatriel]

There is one more place where the move to using traceback from the exception can be visible from a program - in the eval loop when we reraise. See iritkatriel#32 . I think maybe we should add that change to this PR.

It comes down to how certain we are that that's not going to break anything. I don't see an assertion in there that checks that exc_info->exc_traceback equals whatever PyException_GetTraceback() returns.

It will break the same edge case as the other changes in this pr - when you change the traceback on the handled exception in an except clause and then do ‘raise’.

[gvanrossum]

It will break the same edge case as the other changes in this pr - when you change the traceback on the handled exception in an except clause and then do ‘raise’.

Okay, that's fine then. I was worried that it might in some cases insert or leave out a frame, but it sounds like that would only happen if you mess around with the exception (e.g. raising and catching it, like you showed earlier). That edge case is acceptable to me as long as we announce it clearly in what's new.

(I am still concerned about something else -- sometimes "raise e" and "raise" don't do the same thing even if "e" is the exception being handled. But that is not affected by these changes -- that difference will persist, and arguably it is correct (though it surprises me occasionally). Even if we wanted to change it, we couldn't, because it is not an obscure corner case -- the semantics are clearly described. Please ignore my mumbling here.)

[iritkatriel]

FTR - an example of what will change for raise:

import sys

def f():
  raise TypeError(42)

try:
  f()
except Exception as e:
  exc = e
  print('1--', e.__traceback__, sys.exc_info()[2])
  try:
    raise e
  except:
    pass
  print('2--', e.__traceback__, sys.exc_info()[2])
  raise

Old:

iritkatriel@Irits-MBP cpython-1 % ../cpython/python.exe xx.py
1-- <traceback object at 0x10dc13980> <traceback object at 0x10dc13980>
2-- <traceback object at 0x10dc13840> <traceback object at 0x10dc13980>
Traceback (most recent call last):
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 7, in <module>
    f()
    ^^^
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 4, in f
    raise TypeError(42)
    ^^^^^^^^^^^^^^^^^^^
TypeError: 42

New:

iritkatriel@Irits-MBP cpython-1 % ./python.exe xx.py
1-- <traceback object at 0x105748180> <traceback object at 0x105748180>
2-- <traceback object at 0x1057498c0> <traceback object at 0x1057498c0>
Traceback (most recent call last):
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 12, in <module>
    raise e
    ^^^^^^^
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 7, in <module>
    f()
    ^^^
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 4, in f
    raise TypeError(42)
    ^^^^^^^^^^^^^^^^^^^
TypeError: 42

[iritkatriel]

(I am still concerned about something else -- sometimes "raise e" and "raise" don't do the same thing even if "e" is the exception being handled. But that is not affected by these changes -- that difference will persist, and arguably it is correct (though it surprises me occasionally). Even if we wanted to change it, we couldn't, because it is not an obscure corner case -- the semantics are clearly described. Please ignore my mumbling here.)

A related story: After we moved our system from python 2 to python 3 we got bug reports about nonsensical tracebacks. Turned out we had something like this:

import sys
import traceback

class Calc:
  def __init__(self):
     self.value = None
     self.exc_info = None

  def calc(self):
     raise TypeError(12)

  def calc_and_cache(self):
    try:
      self.value = self.calc()
      return self.value
    except Exception as e:
      self.exc_info = sys.exc_info()
      raise

  def get_cached_value(self):
    if self.exc_info is not None:
      raise self.exc_info[1]
    return self.value

c = Calc()

print('------ 1 ------')
try:
   c.calc_and_cache()
except Exception as e:
   traceback.print_exception(e)

print('------ 2 ------')
try:
   c.get_cached_value()
except Exception as e:
   traceback.print_exception(e)

The two tracebacks were the same in python 2, but in python 3 you get:

------ 1 ------
Traceback (most recent call last):
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 29, in <module>
    c.calc_and_cache()
    ^^^^^^^^^^^^^^^^^^
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 14, in calc_and_cache
    self.value = self.calc()
                 ^^^^^^^^^^^
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 10, in calc
    raise TypeError(12)
    ^^^^^^^^^^^^^^^^^^^
TypeError: 12
------ 2 ------
Traceback (most recent call last):
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 35, in <module>
    c.get_cached_value()
    ^^^^^^^^^^^^^^^^^^^^
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 22, in get_cached_value
    raise self.exc_info[1]
    ^^^^^^^^^^^^^^^^^^^^^^
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 29, in <module>
    c.calc_and_cache()
    ^^^^^^^^^^^^^^^^^^
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 14, in calc_and_cache
    self.value = self.calc()
                 ^^^^^^^^^^^
  File "/Users/iritkatriel/src/cpython-1/xx.py", line 10, in calc
    raise TypeError(12)
    ^^^^^^^^^^^^^^^^^^^
TypeError: 12

The fix was to change the raise to raise self.exc_info[1].with_traceback(self.exc_info[2]).

[gvanrossum]

The use of sys.exc_info()[1] always puts me off -- it's just too cryptic. Maybe "the traceback attached to this exception is now always its __traceback__ attribute? Or use the phrasing from the reference manual above?

[gvanrossum]

A related story [...]

Hm, this makes me realize that there may be a bug in asyncio. When you call a Future's result() method, if the Future represents an error, it raises self.exception. But that means that each time you attempt to get the result, you'll get an extra line (or more) added to the traceback. This seems quite unintended -- the author of the code (me) apparently wasn't aware of this behavior. Curiously, nobody's ever complained about nonsensical tracebacks. To fix it, we'd have to add a separate attribute to hold the traceback.

Oh, and the same bug exists in concurrent.futures. (Which I did not write.) In fact now I suspect it exists all over the stdlib where exceptions are stored and raised later.

[iritkatriel]

Curiously, nobody's ever complained about nonsensical tracebacks.

What made our tracebacks obviously nonsensical is that we raise the same exception from two different places. There is no way to get calc_and_cache and get_cached_value in the same stack.

I've created bpo-45924 to track this.

[bedevere-bot]

🤖 New build scheduled with the buildbot fleet by @iritkatriel for commit 05af230 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

[gvanrossum]

LGTM. Land any time!

[finefoot]

Previously, the exception was re-raised with the traceback it had when it was caught.

Isn't this a bug and that particular change should therefore be backported? I recently got surprised by this behavior of raise when using BaseException.with_traceback in Python 3.10: I was able to set e.__traceback__ to a custom traceback with e.with_traceback(my_custom_tb), even verified with e.__traceback__ == my_custom_tb, yet a bare raise raised the exception like no changes had been made. (Also adding to this confusion: Changes to the original traceback like e.__traceback__.tb_next = None did successfully show up with bare raise.)

[finefoot]

So I originally wanted to post this as an issue but discovered that the behavior seemingly disappeared when using Python 3.11. With some help I found out about this PR where one of the changes fixes the behavior of bare raise. So I thought it would be the best idea to ask here via comment since @iritkatriel is also listed as the maintainer for the traceback module anyway? I hope that's okay.

[finefoot]

Here's a short example to explain what I mean:

Python 3.10.11 (main, May  4 2023, 06:08:16) [GCC 10.2.1 20210110] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> def f(): g()
... 
>>> def g(): h()
... 
>>> def h(): raise Exception
... 
>>> # Trying to show only the "most recent" step of the traceback; doesn't work:
>>> try:
...     f()
... except Exception as e:
...     tb = e.__traceback__
...     while tb.tb_next is not None:
...         tb = tb.tb_next
...     e.with_traceback(tb)
...     raise
... 
Exception()
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
  File "<stdin>", line 1, in f
  File "<stdin>", line 1, in g
  File "<stdin>", line 1, in h
Exception
>>> # However, modifying the traceback is possible in general. For example,
>>> # reducing the traceback to the "earliest" step of the original traceback:
>>> try:
...     f()
... except Exception as e:
...     e.__traceback__.tb_next = None
...     raise
... 
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
Exception
>>> 

[iritkatriel]

We can't backport this change, it's way too invasive for that. This behaviour existed since 3.0, unfortunately.

[iritkatriel]

If you "raise e" instead of "raise" then you will see the edited traceback (but with the current frame added).