django-unicorn icon indicating copy to clipboard operation
django-unicorn copied to clipboard

loading modules with coding errors is masked ModuleNotFoundError

Open nerdoc opened this issue 3 years ago • 6 comments

This is a follow-up to #374.

There is still a problem when loading components, and the current code hides some things away, making it extremely complicated to debug and find the culprit.

I have a component that did some attribute errors within the code, it asked for forms.field - which is a nonexistent attribute - instead of forms.BooleanField. So that component was not loaded, and in unicorn_view, line 845ff, it checks first for ModuleNotFoundError (no), and then AttributeError (yes) - BUT: you don't raise the error then, you just save it in last_exception, and the module is NOT loaded, because of the error. So it runs all iterations for all modules, and comes to the last one, where it does not find the module (because it already had found it before, but did not load it).

Here The ModuleNotFoundError is raised, and this overrides the last_exception. This error then is presented to the user - but it tells you nothing about the real cause.

It would be a possibility to remove the catching of AttributeError as a whole - so it is risen and the application stops - which is correct IMHO.

But I don't know what AttributeError you wanted to catch in the first place when you wrote these lines... I don't have the bigger picture here.

nerdoc avatar Mar 12 '22 22:03 nerdoc

@nerdoc I think you are right that I should just raise the error right away, but I'll dig into this and see if I can find any edge cases that might fail.

adamghill avatar Mar 25 '22 12:03 adamghill

Same situation here. I can't debug the cause of a component not work on staging environment but work on my local notebook.

nielsonsantana avatar Apr 03 '22 10:04 nielsonsantana

Sorry about this! I'm going to make the error more clear in the next release, but I won't be able to fix it for the next week or so. But, I'll try to make it better as soon as I can. Thanks for your patience.

adamghill avatar Apr 03 '22 15:04 adamghill

This also is true for components that simply raises errors, e.g. in mount().

class FooView(UnicornView):
    namespace=""

    def mount(self):
        """Check if component was called with a namespace attribute:
        {% unicorn "foo" namespace="foo" %}
        """
        if not self.namespace:
            raise AttributeError(
                "Foo component needs to have a 'namespace' attribute"
        )

At least Unicorn should propagate that error.

nerdoc avatar Apr 04 '22 09:04 nerdoc

in https://github.com/adamghill/django-unicorn/blob/6cb0e137dadf64916376147db09854fae87b0f05/django_unicorn/components/unicorn_view.py#L898 and the following lines there is the culprit code IMHO. You correctly save the AttributeError in the lines before, but do not print it out with the ComponentClassLoadError afterwords.

You could just add the AttributeError in the message, and everything is clearer...

nerdoc avatar Oct 26 '22 07:10 nerdoc

This PR partly fixes the bug. I don't know if other edge cases occur, as there is the second exception raise at the bottom of that code, which still just tells the user a common error without details. I for myself never ran into that. So for me, my PR fixes the issue or at least vastly improves the feedback to the developer...

nerdoc avatar Oct 26 '22 07:10 nerdoc