nucleic / enaml

Declarative User Interfaces for Python
http://enaml.readthedocs.io/en/latest/
Other
1.53k stars 130 forks source link

Show declarative stack if an error occurs during initialization or change event #494

Closed frmdstryr closed 1 year ago

frmdstryr commented 2 years ago

For #493 . It works by saving a reference to the compiler node to each declarative object and copies the filename and line number during compilation.

Unfortunately this breaks any existing caches because of the additional parameters, I'm not sure what to do about that, it could maybe provide empty defaults for the filename and lineno?

Also it currently doesn't catch errors when manually updating an expression, eg so I'm not sure if it should be doing a try/except in the expression engine itself.

MatthieuDartiailh commented 2 years ago

This sounds great. Could you post the resulting traceback for reference ?

I will do my best to review this week.

frmdstryr commented 2 years ago

The error when clicking the button for this

from enaml.core.api import Conditional
from enaml.widgets.api import Window, Container, Label, PushButton

enamldef MyContainer(Container): inside_my_container:
    Label: my_label:
        text = "Hello"
    Conditional: cond:
        condition = True#None
        PushButton: btn:
            text = "Blow up"
            clicked :: cond.condition = None

enamldef Main(Window): main:
    Container: container:
        Label: label1:
            text = "Test"
        MyContainer: my_container:
            pass

Will look like this:

Traceback (most recent call last):
  File "/home/user/projects/enaml/enaml/core/declarative_meta.py", line 77, in declarative_change_handler
    engine.write(owner, change['name'], change)
  File "/home/user/projects/enaml/enaml/core/expression_engine.py", line 210, in write
    pair.writer(owner, name, change)
  File "/home/user/projects/enaml/enaml/core/standard_handlers.py", line 82, in __call__
    call_func(func, (), {}, scope)
  File "tests/example.enaml", line 1, in f
    from enaml.core.api import Conditional
TypeError: The 'condition' member on the 'Conditional' object must be of type 'bool'. Got object of type 'NoneType' instead.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/user/projects/enaml/enaml/qt/qt_abstract_button.py", line 83, in on_clicked
    self.declaration.clicked(checked)
  File "/home/user/projects/enaml/enaml/core/declarative_meta.py", line 81, in declarative_change_handler
    raise DeclarativeError(owner, e) from e
enaml.core.declarative_meta.DeclarativeError: 
  File "tests/example.enaml", line 14, in Main
    enamldef Main(Window): main:
  File "tests/example.enaml", line 15, in Container
    Container: container:
  File "tests/example.enaml", line 18, in MyContainer
    MyContainer: my_container:
  File "tests/example.enaml", line 10, in PushButton
    PushButton: btn:
Aborted

It'd be nice if it could show the source lines all the way up and actual expression but I this at least shows which block. (edit: now does this)

codecov-commenter commented 2 years ago

Codecov Report

Merging #494 (22c6a3e) into main (2aa659b) will increase coverage by 0.03%. The diff coverage is 86.98%.

@@            Coverage Diff             @@
##             main     #494      +/-   ##
==========================================
+ Coverage   74.48%   74.52%   +0.03%     
==========================================
  Files         317      317              
  Lines       24136    24272     +136     
  Branches       55       55              
==========================================
+ Hits        17978    18088     +110     
- Misses       6158     6184      +26     
frmdstryr commented 2 years ago

I suppose the trace should actually be coming from the conditional in this case. I can't see how it's possible to get it to do that because it's coming from atom unless atom would support something like a sender that could be traced back to the event that triggered the update.

MatthieuDartiailh commented 2 years ago

@frmdstryr I am a bit swamped at the moment so please ping me when you want a review.

frmdstryr commented 2 years ago

Unfortunately I don't really like how the DeclarativeError() from e works... Most of the time I have to scroll up to the original error. Any ideas for an alternate approach?

frmdstryr commented 2 years ago

For reasons I don't quite understand yet an error in a default value is not propagated up with this branch. Eg this is simply ignored.

from enaml.widgets.api import Window, Container, Label

enamldef MyWindow(Window):
    Container:
        Label:
            text = undefined_variable

Edit: With the current master branch it returns this.


Traceback (most recent call last):
  File "/home/usr/micromamba/envs/enaml/bin/enaml-run", line 33, in <module>
    sys.exit(load_entry_point('enaml', 'console_scripts', 'enaml-run')())
  File "/home/usr/projects/enaml/enaml/runner.py", line 71, in main
    window.show()
  File "/home/usr/projects/enaml/enaml/widgets/window.py", line 382, in show
    self.activate_proxy()
  File "/home/usr/projects/enaml/enaml/widgets/toolkit_object.py", line 213, in activate_proxy
    child.activate_proxy()
  File "/home/usr/projects/enaml/enaml/widgets/toolkit_object.py", line 213, in activate_proxy
    child.activate_proxy()
  File "/home/usr/projects/enaml/enaml/widgets/toolkit_object.py", line 210, in activate_proxy
    self.activate_top_down()
  File "/home/usr/projects/enaml/enaml/widgets/toolkit_object.py", line 226, in activate_top_down
    self.proxy.activate_top_down()
  File "/home/usr/projects/enaml/enaml/qt/qt_toolkit_object.py", line 74, in activate_top_down
    self.init_widget()
  File "/home/usr/projects/enaml/enaml/qt/qt_label.py", line 55, in init_widget
    self.set_text(d.text)
  File "/home/usr/projects/enaml/enaml/core/declarative_meta.py", line 48, in __call__
    value = engine.read(owner, name)
  File "/home/usr/projects/enaml/enaml/core/expression_engine.py", line 179, in read
    return pair.reader(owner, name)
  File "/home/usr/projects/enaml/enaml/core/standard_handlers.py", line 64, in __call__
    return call_func(func, (), {}, scope)
  File "tests/example.enaml", line 9, in <module>
    text = ThisDoesNotExist
NameError: name 'ThisDoesNotExist' is not defined

If I wrap the read expression with the try except raise DeclarativeError(...) from e the whole error is just ignored?? But if I print it is getting called...

MatthieuDartiailh commented 2 years ago

That does not make any sense. What happens if you simply reraise the same exception rather than the declarative one.

frmdstryr commented 2 years ago

It does nothing. What is interesting is if I raise the error in the DeclarativeError __init__ then it gives an error. It almost seems like the exception flag somehow needs handled differently by atom in this case?

Edit: My local branch I also wrapped the expresion_engine read handler but the error is being ignored with or without it.

frmdstryr commented 2 years ago

If I raise a ValueError then it works so maybe it's something with validation. nevermind, doesn't matter.

frmdstryr commented 2 years ago

Oh sorry, it's because it is doing 'pass' when DeclarativeError is raised... https://github.com/nucleic/enaml/pull/494/files#diff-68b4f23d01f700d0980fdb56a264c7bb6313fcd987739a0e7e27e9bf467d1882R189

MatthieuDartiailh commented 2 years ago

I do lot have an alternate idea. I am not surprised you need to know what failed to fix the issue bit knowing where to look is really helpful.

Currently you do not use the compiler node you store and because you the the widget parent you can miss element which are not widgets (Include, Looper,...). I believe you could do better by checking if the node of the parent does contain the node of the child and if not look in the children of the parent for an element whose node does contain the child node.

I would also be interested in seeing some tests for templates and template instances.

MatthieuDartiailh commented 2 years ago

Tests for templates are still missing but is this otherwise ready for review ?

frmdstryr commented 2 years ago

It still doesn't work right for templates but feedback is welcome.

frmdstryr commented 2 years ago

This template

from enaml.widgets.api import Window, Container, Html

template Panel(Content):
    Container:
        Content:
            pass

enamldef HtmlContent(Html):
    source = False

enamldef Main(Window): main:
    Panel(HtmlContent):
        pass

Currently gives this

Traceback (most recent call last):
  File "/home/user/projects/enaml/enaml/widgets/toolkit_object.py", line 224, in activate_proxy
    child.activate_proxy()
  File "/home/user/projects/enaml/enaml/widgets/toolkit_object.py", line 220, in activate_proxy
    self.activate_top_down()
  File "/home/user/projects/enaml/enaml/widgets/toolkit_object.py", line 241, in activate_top_down
    self.proxy.activate_top_down()
  File "/home/user/projects/enaml/enaml/qt/qt_toolkit_object.py", line 74, in activate_top_down
    self.init_widget()
  File "/home/user/projects/enaml/enaml/qt/qt_html.py", line 40, in init_widget
    self.set_source(self.declaration.source)
TypeError: The 'source' member on the 'HtmlContent' object must be of type 'str'. Got object of type 'bool' instead.

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "/home/user/micromamba/envs/enaml/bin/enaml-run", line 33, in <module>
    sys.exit(load_entry_point('enaml', 'console_scripts', 'enaml-run')())
  File "/home/user/projects/enaml/enaml/runner.py", line 71, in main
    window.show()
  File "/home/user/projects/enaml/enaml/widgets/window.py", line 382, in show
    self.activate_proxy()
  File "/home/user/projects/enaml/enaml/widgets/toolkit_object.py", line 224, in activate_proxy
    child.activate_proxy()
  File "/home/user/projects/enaml/enaml/widgets/toolkit_object.py", line 228, in activate_proxy
    raise DeclarativeError(child, e) from e
enaml.core.declarative_meta.DeclarativeError: 
  File "tests/example.enaml", line 11, in Main
    enamldef Main(Window): main:
  File "tests/example.enaml", line 12, in template
    Panel(HtmlContent):
  File "tests/example.enaml", line 4, in Container
    Container:
  File "tests/example.enaml", line 5, in HtmlContent
    Content:
TypeError: The 'source' member on the 'HtmlContent' object must be of type 'str'. Got object of type 'bool' instead.

I'm not sure how to get the template class name from line 12. Also it's inserting the HtmlContent node into the template as Content so the trace needs to somehow capture that...

MatthieuDartiailh commented 2 years ago

Looking a bit at the compiler it looks like currently the information we would need is discarded. I will try to dig deeper later. Also I am wondering if we could make it more explicit when we move across different definitions.

frmdstryr commented 2 years ago

It appears that linecache becomes stale if a file is reloaded. Not sure if that needs addressed?

MatthieuDartiailh commented 2 years ago

Can you elaborate about the linecache issue ?

frmdstryr commented 2 years ago

When loading from source that changes (eg the live editor example) the source lines it prints out can become incorrect. I suppose that's something with linecache since I'm not using linecache directly anymore.

MatthieuDartiailh commented 1 year ago

I have not forgotten about this but I am swamped at the moment (with bytecode for 3.11 and hence enaml for 3.11 at the moment).

frmdstryr commented 1 year ago

I don't like this approach.

MatthieuDartiailh commented 1 year ago

I can understand your opinion. Still I wish we could do something to improve tracebacks.