alexander-bauer/swirlypy

Recording.py handles name errors by printing traceback but exits console interact

WilCrofter opened this issue · 7 comments

Just one question in this lesson:

Swirlypy Test Lessons by Swirlypy Authors
1: Debugging
Selection: 1
Just type a valid command.
>>> x
Traceback (most recent call last):
  File "<console>", line 1, in <module>
NameError: name 'x' is not defined

ast:  Interactive(body=[Expr(value=Name(id='x', ctx=Load()))])

added:  {}
changed:  {}
removed:  {}

values:  []
coursedir:  ../swirlypy_test_lessons

Lesson complete!

InteractiveConsole's push(line) method traps exceptions internally (except SystemExit) and calls showtraceback(). This is OK if you are just emulating a terminal, but we want to know if an exception has occurred in which case we return to >>>, or not in which case we test the response.

There's a hacky fix in branch b.issue34 which messes with sys.last_value hence could have system side effects. It could be made safe with a little more code, but would still be hacky.

My test lessons can be used for illustration.

I started to look into the code here, and got knee deep before I realized that this most likely isn't an issue we should fix at this level. A while ago, I built in a mechanism for test_response to discard invalid responses (not to be confused with wrong ones). We may want shell subclasses to discard these sorts of events more generally. For example, NewVariable might discard any event which doesn't actually create a new variable.

We ran into a similar issue in swirl. The user enters x=10 (or something) but x already has the value 10 for some reason. Nothing changes, so that looks like an error and the user is asked to retry, which he or she does, but again nothing changes, and so on. (I forget how we got around it. It wasn't pretty.)

That particular thing may not be possible in swirlypy, and I understand that test_response can ignore invalid responses, but IMO it would be best to know when the console swallows an exception rather than to guess at the test_response level.

This issue is actually a bit more complex than you might expect at first glance. In particular, consider the following, where the goal is to set x = 10.

x = 1200
for n in [5, 4, 3, 2, 1, 0]:
    print("x = %d / %d" % (x, n))
    x = int(x / n)

This results in the output

x = 1200 / 5
x = 240 / 4
x = 60 / 3
x = 20 / 2
x = 10 / 1
x = 10 / 0
Traceback (most recent call last):
  File "<stdin>", line 3, in <module>
ZeroDivisionError: division by zero

Even though x = 10 at the end, we would fail to process this correctly. Syntax errors are already ignored properly by the console, so I'm uncertain how much intervention is actually necessary, here.

Good point. Just for the record, I think we've decided to detect exceptions as we're now doing and to pass them back as part of the response, but not to act on them for the moment.

Handled and committed,