#3702 breaks printing an exception twice

[cwalther]

Just so it isn’t forgotten, since I can’t seem to reopen #3702:

I believe the fix proposed there is wrong: It prevents printing an exception twice, which I do in #3696 (once to measure the length of the traceback, once to get the text) and which people may want to do for other reasons, it’s available from Python via sys.print_exception(). (For some reason, it also causes the terminalio screen to be drawn over the stage screen, haven’t investigated that yet.)

The problem observed in micropython#2056 is specific to adafruit_midi in that it raises the same exception instance every time, a usage that apparently wasn’t anticipated in the core. Will have to dig deeper into how exceptions work to figure out how to properly fix that. It may be nontrivial – there would need to be a way to distinguish an exception reused in the way adafruit_midi does it from one re-raised in an except block.

In fact, I’m not sure it should be fixed, since CPython appears to do the same:

Python 3.8.3 (default, Jul  8 2020, 14:25:02) 
[Clang 11.0.0 (clang-1100.0.33.17)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> e = ValueError('X')
>>> raise e
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    <?xml version="1.0" encoding="UTF-8"?>
ValueError: X
>>> raise e
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    <?xml version="1.0" encoding="UTF-8"?>
  File "<stdin>", line 1, in <module>
    <?xml version="1.0" encoding="UTF-8"?>
ValueError: X
>>> raise e
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
    <?xml version="1.0" encoding="UTF-8"?>
  File "<stdin>", line 1, in <module>
    <?xml version="1.0" encoding="UTF-8"?>
  File "<stdin>", line 1, in <module>
    <?xml version="1.0" encoding="UTF-8"?>
ValueError: X

So maybe the proper fix is rather to make adafruit_midi stop reusing exceptions. @kevinjwalters, looks like that would be your territory? It comes from adafruit/Adafruit_CircuitPython_MIDI@0f96357.

[kevinjwalters]

Is the CPython behaviour specified somewhere?

[cwalther]

Good question. As far as I can find, only by absence: https://docs.python.org/3/reference/datamodel.html#traceback-objects says how traceback objects are added, but there is nothing that says that any previously accumulated traceback would be cleared when an exception is raised outside of an except block.

[tannewt]

@DavePutz What do you think?

[DavePutz]

I don't see the value in showing multiple exceptions for a single error. It gives the impression that more than one thing went wrong. i.e. I think CPython is probably not doing it correctly. I do understand, though, the value of #3696 to get a previous traceback. Would it be worth adding a parameter to mp_obj_print_exception() to indicate that we don't want to clear i.e. when calling supervisor.get_previous_traceback() ? That would involve changing the 15 or so places in the code where mp_obj_print_exception() is called. I'm willing to do that if it is a reasonable solution.

[tannewt]

Could we prevent the duplicate traceback in a different way? We could clear the traceback before computing it a second time.

[jepler]

It is "normal" to raise a single exception object multiple times, as in

try:
   1/random.randint(0, 10)
except ZeroDivisionError as e:
   print("unlucky")
   raise e

this second raise statement should not clear any exception frames already associated with e.

[DavePutz]

I am not sure this example pertains, as the output shows up the same with and without the fix in PR #3702: unlucky Traceback (most recent call last): File "<stdin>", line 1, in <module> File "tracetest.py", line 7, in <module> File "tracetest.py", line 7, in <module> File "tracetest.py", line 4, in <module> ZeroDivisionError: division by zero

…

On Wed, Dec 2, 2020 at 9:27 PM Jeff Epler ***@***.***> wrote: It is "normal" to raise a single exception object multiple times, as in try: 1/random.randint(0, 10) except ZeroDivisionError as e: print("unlucky") raise e this second raise statement should not clear any exception frames already associated with e. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3771 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AFNJKEV33NQTCSZB5FMZCMLSS4ASDANCNFSM4UGCJMYQ> .

[cwalther]

It pertains to clearing on raise, as I (perhaps unclearly) proposed in the initial post (and is what I meant when I said it’s nontrivial), not to clearing on print, as you implemented in #3702.

As far as the other proposed solutions go, I could probably live with a flag to restrict the clearing-on-print to a subset of cases, but really the more I think about it the more it feels to me like a case of garbage-in-garbage-out where the proper solution is just “don’t reuse exceptions”.

[tannewt]

I agree with @cwalther. It's weird to reuse an exception object for multiple errors. The better fix for the original issue micropython#2056 is to not reuse ValueError in the MIDI library. I'll transfer the issue.

[tannewt]

I've rerun the jobs for this PR too.

[cwalther]

Context for confused readers (as I was at first): The moved issue is adafruit/Adafruit_CircuitPython_MIDI#28, which is why mentions of its previous number #2056 now erroneously link to micropython#2056.

[jepler]

[CI failure was a network problem]