#3702 breaks printing an exception twice #3771
Conversation
This reverts commit 0cd951f. It is not a correct solution because it prevents printing the same exception twice.
|
Is the CPython behaviour specified somewhere? |
|
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 |
|
@DavePutz What do you think? |
|
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. |
|
Could we prevent the duplicate traceback in a different way? We could clear the traceback before computing it a second time. |
|
It is "normal" to raise a single exception object multiple times, as in this second raise statement should not clear any exception frames already associated with |
|
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>
.
|
|
It pertains to clearing on 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”. |
|
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. |
|
I've rerun the jobs for this PR too. |
|
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 |
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 theterminalioscreen to be drawn over thestagescreen, haven’t investigated that yet.)The problem observed in micropython#2056 is specific to
adafruit_midiin 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 wayadafruit_mididoes it from one re-raised in anexceptblock.In fact, I’m not sure it should be fixed, since CPython appears to do the same:
So maybe the proper fix is rather to make
adafruit_midistop reusing exceptions. @kevinjwalters, looks like that would be your territory? It comes from adafruit/Adafruit_CircuitPython_MIDI@0f96357.